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.

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

Inconsistency on non_heap vs. nonheap value for memory metrics #7876

Closed
AlexanderWert opened this issue Feb 22, 2023 · 4 comments · Fixed by open-telemetry/opentelemetry-specification#3250
Labels
bug Something isn't working

Comments

@AlexanderWert
Copy link
Member

AlexanderWert commented Feb 22, 2023

Description

Describe the bug
In the semantic conventions for JVM runtime metrics (on the process.runtime.jvm.memory.* metrics) the attribute values for the type attribute are defined as heap and nonheap.

Here (in the Java instrumentation) the values are heap and non_heap (with underscore!) which is not what the semantic conventions define:

Steps to reproduce
Run the Java agent and observe the JVM memory metrics.

What did you expect to see?
The attribute values for type to be aligned with the semantic conventions.

What did you see instead?
non_heap instead of nonheap

What version are you using?
1.23.0

Environment
not relevant

Solution alternatives

  1. Either fix it in the Java instrumentation / agent to comply with the semantic conventions
  2. OR fix the semantic conventions to comply with the Java implementation.

Since this aspect of the semantic conventions only affects Java, Option 2 seems to be the simpler and less intrusive approach.

@AlexanderWert AlexanderWert added the bug Something isn't working label Feb 22, 2023
@laurit
Copy link
Contributor

laurit commented Feb 22, 2023

@jack-berg should we change semantic conventions or the code?

@trask
Copy link
Member

trask commented Feb 22, 2023

+1 on using non_heap (I think that's most consistent)

i'll send a spec PR to float the proposal

@jack-berg
Copy link
Member

Agreed on non_heap.

carlosalberto added a commit to open-telemetry/opentelemetry-specification that referenced this issue Feb 24, 2023
Fixes
open-telemetry/opentelemetry-java-instrumentation#7876
(the implementation used `non_heap`, which seems better)

## Changes

Renames JVM metric attribute value from `nonheap` to `non_heap`.

---------

Co-authored-by: Carlos Alberto Cortez <[email protected]>
@trask
Copy link
Member

trask commented Feb 24, 2023

done, thx for noticing this @AlexanderWert!

jsuereth pushed a commit to jsuereth/opentelemetry-specification that referenced this issue Feb 28, 2023
Fixes
open-telemetry/opentelemetry-java-instrumentation#7876
(the implementation used `non_heap`, which seems better)

## Changes

Renames JVM metric attribute value from `nonheap` to `non_heap`.

---------

Co-authored-by: Carlos Alberto Cortez <[email protected]>
jsuereth pushed a commit to jsuereth/otel-semconv-test that referenced this issue Apr 19, 2023
Fixes
open-telemetry/opentelemetry-java-instrumentation#7876
(the implementation used `non_heap`, which seems better)

## Changes

Renames JVM metric attribute value from `nonheap` to `non_heap`.

---------

Co-authored-by: Carlos Alberto Cortez <[email protected]>
jsuereth pushed a commit to open-telemetry/semantic-conventions that referenced this issue May 11, 2023
Fixes
open-telemetry/opentelemetry-java-instrumentation#7876
(the implementation used `non_heap`, which seems better)

## Changes

Renames JVM metric attribute value from `nonheap` to `non_heap`.

---------

Co-authored-by: Carlos Alberto Cortez <[email protected]>
carlosalberto added a commit to carlosalberto/opentelemetry-specification that referenced this issue Oct 31, 2024
Fixes
open-telemetry/opentelemetry-java-instrumentation#7876
(the implementation used `non_heap`, which seems better)

## Changes

Renames JVM metric attribute value from `nonheap` to `non_heap`.

---------

Co-authored-by: Carlos Alberto Cortez <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants