Skip to content

draft: add jvm.fd.open and jvm.runtime.uptime to jmx configs #1665

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

Conversation

schmikei
Copy link

Description:

WIP:

  • adding jvm.runtime.uptime and jvm.fd.open to the jvm collectors

Existing Issue(s):

< Link any applicable issues. >

Testing:

< Describe what testing was performed and any tests were added. >

Documentation:

< Describe the documentation added.
Please be sure to modify relevant existing documentation if needed. >

Outstanding items:

< Anything that these changes are intentionally missing
that will be needed or can be helpful in the future. >

@SylvainJuge
Copy link
Contributor

This PR adds metrics to the "JMX Gatherer", which should be replaced in a near/medium future with the new "JMX scraper".

When adding new metrics, you should also add them to the new YAML definitions https://github.com/open-telemetry/opentelemetry-java-contrib/blob/main/jmx-scraper/src/main/resources/jvm.yaml and also modify the related tests.

Also, adding those metrics to the "JMX Insight" feature of instrumentation would be nice to capture consistent metrics when using either of those implementations (Contrib JMX Gatherer, Contrib JMX Scraper or Instrumentation JMX Insights).

Last but not least, I haven't seen those metrics being defined in the Java runtime semconv. While the JMX metrics are currently not defined in semconv and are not strictly required to be, having at least some alignment with the current ones would help to provide a consistent experience and could help making them part of semconv later.

@trask
Copy link
Member

trask commented Jan 23, 2025

adding jvm.runtime.uptime and jvm.fd.open to the jvm collectors

can you open an issue in https://github.com/open-telemetry/semantic-conventions to suggest these? it could probably use some discussion over there to align with naming conventions

@schmikei
Copy link
Author

Apologies coming from mostly the golang Otel contribution side so still getting my bearings here.

Seems like there was already some pre-discussion/consolidation on the approach for cpu time of the jvm process already
open-telemetry/opentelemetry-specification#3490

I'll probably do some issue creating for https://docs.oracle.com/en/java/javase/17/docs/api/jdk.management/com/sun/management/UnixOperatingSystemMXBean.html#getOpenFileDescriptorCount() as that seems like a new use case but want to do my due diligence and see if anything was created in similar veins/familiarize myself a tad bit more.

Thanks for taking a look at the draft and giving some guidance though 😄

otel.instrument(runtime, "jvm.runtime.uptime", "uptime",
"ms", "Uptime", otel.&longValueCallback)

def os = otel.mbean("java.lang:type=OperatingSystem")
Copy link
Member

Choose a reason for hiding this comment

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

I'm not very familiar with how this works, what happens on Windows?

@SylvainJuge
Copy link
Contributor

Hi !

We are currently migrating from jmx gatherer groovy implementation to use instead jmx-scraper that relies on YAML files to define new metrics, so adding a metric to something that is getting deprecated is probably not a great timing.

Also, the JVM metrics definitions that are captured by jmx-scraper are moving to instrumentation repository once open-telemetry/opentelemetry-java-instrumentation#13392 is merged. Doing this allows to have consistent metrics that fit the semantic conventions definitions for JVM metrics.

I would suggest to open at least a draft PR on the semantic conventions repository to add any new metric. (1)

In addition to that, because those metrics seem quite simple to implement with the new YAML syntax, I would suggest to provide their YAML definitions as a draft implementation. We should be able to test those with either instrumentation or jmx scraper by using the otel.jmx.config configuration option. (2)

The idea here is that uou can use (2) as a way to provide an implementation to ensure the semconv proposal (1) is correct. Doing those two tasks together helps to solve the chicken/egg problem we have between semconv metrics definitions which need to fit the reality of what we can capture with JMX.

Once the semconv definition gets merged, the next steps should be to:

  • add those new metrics to the jvm.yaml in instrumentation, those will then get automatically available in jmx-scraper
  • add those new metrics to the instrumentation/runtime-telemetry implementations as those do not reuse the yaml files for JVM metrics but explicit code (which allows to use advanced features of JMX and get more details).

@schmikei schmikei closed this Mar 19, 2025
@schmikei
Copy link
Author

@SylvainJuge sweet thank you so much for the guidance!

@schmikei schmikei deleted the feat/add-uptime-and-openfd-jvm-target-metrics branch March 19, 2025 19:31
@SylvainJuge
Copy link
Contributor

Hi !

The jvm.yaml is now part of instrumentation thanks to the open-telemetry/opentelemetry-java-instrumentation#13392 PR that has been merged, so it's now easier to:

  • create a draft PR to modify the jvm.yaml in instrumentation
  • open another draft PR to modify the JVM metrics semantic conventions to add those as experimental metrics.

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.

6 participants