Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Monitoring] Added ability to possibly distinguish between Agent type metrics in APM #95129

Merged
merged 19 commits into from
Apr 26, 2021
Merged

[Monitoring] Added ability to possibly distinguish between Agent type metrics in APM #95129

merged 19 commits into from
Apr 26, 2021

Conversation

igoristic
Copy link
Contributor

@igoristic igoristic commented Mar 23, 2021 •
edited by jasonrhodes
Loading

Resolves #97879

In this PR we're changing some headers from "APM Server" to "APM and Fleet Server" to help users understand that data from other Agent subprocesses might also be included if the monitoring cluster is in cloud and has (at least one) node with version 7.13 (or greater). More information in #97879

@igoristic igoristic added this to the Stack Monitoring UI 7.13 milestone Mar 23, 2021
@igoristic igoristic requested review from simitt and a team March 23, 2021 01:06
@igoristic igoristic self-assigned this Mar 23, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/stack-monitoring (Team:Monitoring)

@igoristic igoristic requested a review from simitt April 6, 2021 00:58
simitt
simitt reviewed Apr 6, 2021
View reviewed changes
Copy link
Contributor

@simitt simitt left a comment

Choose a reason for hiding this comment

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

I didn't actually run it yet, but LGTM apart from the version check comment I left.

x-pack/plugins/monitoring/public/lib/apm_agent.ts Outdated Show resolved Hide resolved
simitt
simitt approved these changes Apr 12, 2021
View reviewed changes
Copy link
Contributor

@simitt simitt left a comment

Choose a reason for hiding this comment

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

wasn't able to run it, but changes LGTM

@igoristic igoristic changed the title [Monitoring] Added agent_mode config option to APM [Monitoring] Added ability to possibly distinguish between Agent type metrics in APM Apr 13, 2021
@simitt simitt mentioned this pull request Apr 19, 2021
Closed
@jasonrhodes
Copy link
Member

I talked to @igoristic and I am not sure we fully understand this change yet, so I'm going to reach out to some folks out of band and get clarification. If we still need to land this for 7.13, I'm comfortable with talking about landing it as a text/visual bug fix, or we can push it to 7.13.1/7.14 if that's okay also.

@simitt
Copy link
Contributor

simitt commented Apr 20, 2021

Can you please share your concerns? We are actually relying on this change. The APM & Fleet Server - Resource Usage section will be the only indicator for cloud users regarding the cpu and memory usage of the APM & Fleet Server container; no other metrics will be reported. I fear when we are not consistent with the naming, it will be very confusing for users.

@spalger spalger added v7.14.0 and removed v7.13.0 labels Apr 21, 2021
@jasonrhodes jasonrhodes added bug Fixes for quality problems that affect the customer experience v7.13.0 and removed v7.14.0 labels Apr 23, 2021
@simitt
Copy link
Contributor

simitt commented Apr 26, 2021

@igoristic could you please re-request a review once this is finalized?

@igoristic
Copy link
Contributor Author

@simitt

could you please re-request a review...

Will do 👍 Though, nothing has changed since the last approval. Just some test fix and conflict resolves

@igoristic igoristic requested a review from simitt April 26, 2021 07:31
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
monitoring 561 562 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
monitoring 728.8KB 730.8KB +2.0KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @igoristic

jasonrhodes
jasonrhodes approved these changes Apr 26, 2021
View reviewed changes
Copy link
Member

@jasonrhodes jasonrhodes left a comment

Choose a reason for hiding this comment

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

LGTM

@igoristic igoristic merged commit 27f2d4a into elastic:master Apr 26, 2021
@igoristic igoristic deleted the apm_es_agent branch April 26, 2021 19:43
This was referenced Apr 26, 2021
Merged
Merged
igoristic added a commit to igoristic/kibana that referenced this pull request Apr 26, 2021
@igoristic
[Monitoring] Added ability to possibly distinguish between Agent type…
a90f4ea 
… metrics in APM (elastic#95129)

* Added version and cloud check

* Fixed tests

* Code review feedback

* Fixed tests

* added versions

* added version

* ...

* Removed default version

* ..

* Fixed test

* fixed tests
# Conflicts:
#	x-pack/plugins/monitoring/server/lib/apm/_apm_stats.js
#	x-pack/test/api_integration/apis/monitoring/apm/fixtures/cluster.json
igoristic added a commit that referenced this pull request Apr 28, 2021
@igoristic
[7.13] [Monitoring] Added ability to possibly distinguish between Age…
d9d2fba 
…nt type metrics in APM (#95129) (#98388)

* [Monitoring] Added ability to possibly distinguish between Agent type metrics in APM (#95129)

* Added version and cloud check

* Fixed tests

* Code review feedback

* Fixed tests

* added versions

* added version

* ...

* Removed default version

* ..

* Fixed test

* fixed tests
# Conflicts:
#	x-pack/plugins/monitoring/server/lib/apm/_apm_stats.js
#	x-pack/test/api_integration/apis/monitoring/apm/fixtures/cluster.json

* Update cluster.json

* Update _apm_stats.js
igoristic added a commit that referenced this pull request Apr 28, 2021
@igoristic
[7.x] [Monitoring] Added ability to possibly distinguish between Agen…
9f4dbd7 
…t type metrics in APM (#95129) (#98387)

* [Monitoring] Added ability to possibly distinguish between Agent type metrics in APM (#95129)

* Added version and cloud check

* Fixed tests

* Code review feedback

* Fixed tests

* added versions

* added version

* ...

* Removed default version

* ..

* Fixed test

* fixed tests
# Conflicts:
#	x-pack/plugins/monitoring/server/lib/apm/_apm_stats.js
#	x-pack/test/api_integration/apis/monitoring/apm/fixtures/cluster.json

* Update cluster.json

* Update _apm_stats.js
@simitt
Copy link
Contributor

simitt commented Apr 29, 2021

@igoristic I wanted to test this on cloud QA but I see APM Server - Resource Usage. From my understanding the title should be changed when running a cloud deployment. Could you please verify that this is actually working as expected?

@simitt simitt mentioned this pull request May 3, 2021
Closed
@igoristic igoristic mentioned this pull request May 4, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonrhodes jasonrhodes jasonrhodes approved these changes

@simitt simitt Awaiting requested review from simitt

Assignees

@igoristic igoristic

Labels
bug Fixes for quality problems that affect the customer experience release_note:enhancement Team:Monitoring Stack Monitoring team v7.13.0 v8.0.0
Projects
None yet
Milestone
Stack Monitoring UI 7.13
Development

Successfully merging this pull request may close these issues.

[Stack Monitoring] Adjust APM server headings when we detect a container + agent env
6 participants
@igoristic @elasticmachine @jasonrhodes @simitt @kibanamachine @spalger