Fix jvm.thread.count semconv.

[breedx-splk]

The new scraper code is reporting the wrong name for jvm.thread.count (semconv here).

Even if this was done to keep parity with the old module, this new name is wrong and will have a negative impact on systems that rely on the correct name.

[robsunday]

If this change is going to be merged it would be good to make an update also in JMX Metrics Gatherer

[SylvainJuge]

For enhancing the JMX metrics, I have opened open-telemetry/opentelemetry-java-instrumentation#13238 to discuss what are the options we can use to make the transition as smooth as possible.

I think we could argue for this particular case that if "old" jmx gatherer definitions are inconsistent with semconv they should be fixed anyway because we have a very clear definition of the metrics definitions provided by semconv. However, this does not prevent introducing a breaking change for the consumers of those metrics, stable semconv definitions help to ensure it's the last. For the same reason we did not change the metric attributes that now need to be properly namespaced.

So, we could do this change here, but it's quite unlikely we can easily do the same for all the metrics that are not part of semconv, for those we need to discuss it in open-telemetry/opentelemetry-java-instrumentation#13238. While ideally I would like to have all JMX metrics part of semconv, I don't think this is something easily achievable but I'd be happy to be proven wrong on this.

[SylvainJuge]

After discussing this with @robsunday today, we think it would be best to do the following:

• JVM metrics are part of semconv, so there is no or less discussion to have on their definitions which hopefully makes it easier.
• do the migration of the whole "target system" at once, which means we need to cover all the JVM metrics that are part of semconv
• keep the gatherer definitions aligned for now to reduce the gap between gatherer and scraper.

For the particular case of JVM metrics, it means we keep 3 distinct implementations, but once they are all aligned with semconv they won't drastically change nor require maintenance:

• in instrumentation it's implemented with runtime-metrics instrumentation with code
• for jmx scraper, it's implemented with a dedicated jvm.yaml
• for jmx gatherer, it's implemented with a dedicated jvm.groovy

@breedx-splk given this widens the scope of the change you initially wanted to make, I am volunteering to open a PR on your PR, or alternatively if that's simpler we can close this PR and I can take over and create a new one.

[breedx-splk]

I think it's just better overall to use the correct names everywhere...but that's just me.

[SylvainJuge]

I think it's just better overall to use the correct names everywhere...but that's just me.

I could not agree more, especially when the "correct name" has been defined in semconv.

Does it works for you if I open another PR to fix all those definitions mismatch at once for the JVM metrics or do you prefer to merge this one first then do all the other ones separately ? While it's more work I think it's simpler for end-users to have to deal with a single one-time change for all the JVM metrics rather than potentially spreading it over many iterations.

[breedx-splk]

Does it works for you if I open another PR to fix all those definitions mismatch at once for the JVM metrics or do you prefer to merge this one first then do all the other ones separately ? While it's more work I think it's simpler for end-users to have to deal with a single one-time change for all the JVM metrics rather than potentially spreading it over many iterations.

Yeah that's fine, especially if you have some main list or know of all the gaps right now. Happy to close this one....I just tend to favor smaller incremental work over mega PRs. But if it's mostly name changes should be fine.

[SylvainJuge]

I opened open-telemetry/opentelemetry-java-instrumentation#13392 PR in instrumentation to allow to adding semconv-compliant metrics in YAML format, those can then later be reused in jmx-scraper as-is.