Skip to content

Comments

[Monitoring] APM mapping update#46244

Merged
cachedout merged 7 commits intoelastic:masterfrom
cachedout:apm_mapping_update
Sep 25, 2019
Merged

[Monitoring] APM mapping update#46244
cachedout merged 7 commits intoelastic:masterfrom
cachedout:apm_mapping_update

Conversation

@cachedout
Copy link
Contributor

@cachedout cachedout commented Sep 2, 2019 •
edited
Loading

This creates needed mappings for additional APM metrics which were introduced by elastic/apm-server#2142

@cachedout cachedout changed the title Apm mapping update [Monitoring] APM mapping update Sep 2, 2019
@cbuescher cbuescher added :Core/Infra/Monitoring DEPRECATED, DO NOT USE WIP labels Sep 4, 2019
@elasticmachine
Copy link
Collaborator

elasticmachine commented Sep 4, 2019

Pinging @elastic/es-core-features

@cachedout cachedout marked this pull request as ready for review September 24, 2019 12:53
@cachedout cachedout mentioned this pull request Sep 24, 2019
Closed
7 tasks
@ycombinator
Copy link
Contributor

ycombinator commented Sep 24, 2019

I assume these mappings are needed for elastic/kibana#43998? If so, I'd suggest only mapping the fields we need to index for the new UI queries/aggs introduced in that PR to work correctly.

Reading through elastic/kibana#43998 that looks like beats_stats.metrics.apm-server.acm.* are the new fields being introduced. So I think we should only map those for now. I don't think we need to map beats_stats.metrics.apm-server.root.* or beats_stats.metrics.apm-server.sourcemap.* at this time; we can add mappings for those when we need to use those fields in queries/aggs from the Stack Monitoring UI.

@cachedout
Copy link
Contributor Author

cachedout commented Sep 24, 2019

Thanks, @ycombinator. I agree and have removed those two sections.

ycombinator
ycombinator approved these changes Sep 24, 2019
View reviewed changes
Copy link
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

chrisronline
chrisronline approved these changes Sep 24, 2019
View reviewed changes
Copy link
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. These seem to work fine alongside the Kibana changes

@cachedout cachedout merged commit 254655e into elastic:master Sep 25, 2019
@cachedout cachedout deleted the apm_mapping_update branch September 25, 2019 09:31
@cachedout cachedout removed the WIP label Sep 27, 2019
This was referenced Jun 24, 2020
Merged
Merged
Merged
Merged
chrisronline pushed a commit to chrisronline/elasticsearch that referenced this pull request Jun 24, 2020
@cachedout @chrisronline
[Monitoring] APM mapping update (elastic#46244)
b3f3686 
* Add acm mapping to APM for beats

* Add root mapping for APM

* Add sourcemap mapping to APM

* Fix missing properties

* Fix a second missing properties

* Add request property to acm

* Remove root and sourcemap per review
chrisronline added a commit that referenced this pull request Jun 24, 2020
@chrisronline @cachedout
[Monitoring] APM mapping update (#46244) (#58499)
4c010a9 
* Add acm mapping to APM for beats

* Add root mapping for APM

* Add sourcemap mapping to APM

* Fix missing properties

* Fix a second missing properties

* Add request property to acm

* Remove root and sourcemap per review

Co-authored-by: Mike Place <mike.place@elastic.co>
chrisronline added a commit that referenced this pull request Jun 24, 2020
@chrisronline @cachedout @elasticmachine
[Monitoring] APM mapping update (#46244) (#58498)
d5899d1 
* Add acm mapping to APM for beats

* Add root mapping for APM

* Add sourcemap mapping to APM

* Fix missing properties

* Fix a second missing properties

* Add request property to acm

* Remove root and sourcemap per review

Co-authored-by: Mike Place <mike.place@elastic.co>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ycombinator ycombinator ycombinator approved these changes

+1 more reviewer

@chrisronline chrisronline chrisronline approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

:Core/Infra/Monitoring DEPRECATED, DO NOT USE

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@cachedout @elasticmachine @ycombinator @chrisronline @cbuescher