Skip to content

jmx add jvm metrics yaml #13392

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.

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

Merged
merged 30 commits into from
Apr 15, 2025
Merged

Conversation

SylvainJuge
Copy link
Contributor

@SylvainJuge SylvainJuge commented Feb 25, 2025

  • adds jvm.yaml in the JMX library so it can be reused by jmx-scraper in contrib.
  • adds documentation on the captured metrics and links to semconv, also include limitations.

Part of #13238 with a scope limited to JVM metrics.

~75% of this PR is for adding test infrastructure similar to contrib repository, it has been moved to #13597 separate PR to make review easier.

PR opened as draft, not ready for review nor being merged yet.

UPDATE: Now ready to be merged, the test infrastructure code that is included could be reviewed and merged separately with #13597 .

TODOs

@SylvainJuge
Copy link
Contributor Author

I tried to refactor the test assertions to only rely on the ones provided by the SDK (using io.opentelemetry.sdk.testing.assertj.MetricAssert), but this requires quite significant work which is probably not worth the investment time for now.

  • the protobuf parsing logic can be reused from io.opentelemetry.javaagent.testing.common.AgentTestingExporterAccess but needs to be moved to a separate class to prevent classloading issues.
  • the metric type checks like isCounter and isGauge needs to be adapted to fit both long and double variants, for example with isLongCounter and isDoubleCounter

Overall I think it would be more beneficial if we extend the SDK MetricAssert with similar "high level" assertions that are closer to the metric data model in order to help making test code more readable. For example, currently writing test code for an updowncounter requires to know that it is stored as a "non monotonic sum" and that a counter is translated to a "monotonic sum", both of which require to read the actual values as the monoticity is not preserved when parsing it from protobuf to MetricData.

@SylvainJuge
Copy link
Contributor Author

All the test infrastructure code changes have been copied to #13597 to make review easier and allow reusing it.

@SylvainJuge SylvainJuge self-assigned this Apr 9, 2025
@SylvainJuge SylvainJuge marked this pull request as ready for review April 9, 2025 08:31
@SylvainJuge SylvainJuge requested a review from a team as a code owner April 9, 2025 08:31
…etry/instrumentation/jmx/rules/JvmTargetSystemTest.java

Co-authored-by: Jay DeLuca <[email protected]>
@trask trask merged commit 60abbf4 into open-telemetry:main Apr 15, 2025
86 checks passed
@SylvainJuge SylvainJuge deleted the jvm-jmx-metrics branch April 16, 2025 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants