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

Add java.vm.name to process.runtime.description & more examples. #1242

Merged
merged 3 commits into from
Nov 24, 2020

Conversation

Oberon00
Copy link
Member

Add java.vm.name to process.runtime.description. I think (except for Eclipse J9), the result reads cleaner and is more informative.

Related: open-telemetry/opentelemetry-java#1908 (comment)

@Oberon00 Oberon00 added area:semantic-conventions Related to semantic conventions spec:resource Related to the specification/resource directory labels Nov 19, 2020
@Oberon00 Oberon00 requested review from a team as code owners November 19, 2020 15:29
| Zulu OpenJDK | OpenJDK Runtime Environment | 11.0.8+10-LTS | Azul Systems, Inc OpenJDK 64-Bit Server VM Zulu11.41+23-CA |
| Oracle Hotspot 8 (32 bit) | Java(TM) SE Runtime Environment | 1.8.0_221-b11 | Oracle Corporation Java HotSpot(TM) Client VM 25.221-b11 |
| IBM J9 8 | Java(TM) SE Runtime Environment | 8.0.5.25 - pwa6480sr5fp25-20181030_01(SR5 FP25) | IBM Corporation IBM J9 VM 2.9 |
| Android 11 | Android Runtime | 0.9 | The Android Project Dalvik 2.1.0 |
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for even finding the name for Android :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Caveat: I did not actually try this on on Android, but just googled around. The most official source I found was https://android.googlesource.com/platform/art/+/master/openjdkjvmti/ti_properties.cc but Stackoverflow concurs.

Copy link
Member

@arminru arminru left a comment

Choose a reason for hiding this comment

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

Technically this is a breaking change of existing semantics but I would not expect any back end to actually parse this value but rather just display it as-is. If there are no objections, I'm fine with it.

Please add a changelog to make people aware of it, however.

@Oberon00
Copy link
Member Author

Oberon00 commented Nov 20, 2020

Technically this is a breaking change of existing semantics.

Hmm, in the strictest sense it is, but if you just look at the attribute description, it says Java instrumentation should fill in and the description for process.runtime still says "An additional description about the runtime of the process, for example a specific vendor customization of the runtime environment".

I think this description is something that we should encourage people not to parse, and we should add runtime-specific attributes if desired e.g. jvm.prop.java.vm.name to contain the system property java.vm.name.

Please add a changelog to make people aware of it, however.

This is not yet implemented in opentelemetry-java and I made them aware on the respective issue open-telemetry/opentelemetry-java#1908. A changelog entry still makes sense though, will do. EDIT: Done in 6e87835. Also rebased to get the checks in order.

@arminru arminru merged commit 51145ad into open-telemetry:master Nov 24, 2020
@arminru arminru deleted the java-vm-name branch November 24, 2020 13:10
arminru added a commit to dynatrace-oss-contrib/opentelemetry-specification that referenced this pull request Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions spec:resource Related to the specification/resource directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants