Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 process.runtime.name / version describing executing runtime #882
Add process.runtime.name / version describing executing runtime #882
Changes from 3 commits
49592a32a3feba9f441cdce631d508260340b19aa9a0482e084f3423c20176410df2c7afedfac31e2ffee5596f2faf53817906c269eec3e56a3aca1090e92e25fe6d29544066054fc9eaccbb0aFile filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you checked this list against values returned by
java.vendorsystem property? I suspect that will be the most common source of this information.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found that e.g. zulu jdk has a vendor like
Azul Systems(sorry on phone right now so don't remember the exact string but it was something like this and very different than what I wrote in this table either way). So I'm wavering between whether these are descriptive or not. We could define this as "the value of the java.vendor` property and be done with it. Or we can expect instrumentation to parse these strings into listed constants like listed here. I'm leaning towards the latter since I don't think there is any way to avoid a vendor string from changing despite being effectively the same JDK imementation, but don't have a strong preference.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If in case of Java we want
process.runtime.nameto be mean "which java distribution is in use", then I think we should usejava.vendorverbatim (not parsing) as part ofprocess.runtime.name. I don't know if this is enough. E.g. in case of Zulu and Zing, do they both haveAzul Systemsas vendor? I would expect so. Then we need something else in addition to vendor name.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for calling this out. I collected a bunch of properties.
https://gist.github.com/anuraaga/c79219a7e4401515c3de998f9d077be0
It looks like the only way to properly express the vendor distribution (which is important, some distributions have totally different runtime components like corretto, openj9) is a concatenation of
java.vendorandjava.vendor.version. But this doesn't really work because adoptopenjdk isAdoptOpenJDK AdoptOpenJDKin that case.I'm starting to think I also need to add
runtime.descriptionattribute which contains a probably language-specific, descriptive name of a runtime, for cases where they still all fork off of a very large common component like the openjdk distributions. soruntime.name=java.runtime.nameruntime.version=java.runtime.versionruntime.description=java.vendor+java.vendor.versionIt's ok - but still not ideal because version strings aren't consistent either :/ But probably best we can do, backends can probably deal with this well enough.
OpenJDK Runtime Environment,11.0.8+10,N/A 18.9(18.9 I think is release date)OpenJDK Runtime Environment,11.0.8+10-LTS,Azul Systems, Inc. Zulu11.41+23-CAOpenJDK Runtime Environment,11.0.8+10,AdoptOpenJDK AdoptOpenJDK(sigh)Android Runtime,0.9,Android Project 1.2.3(guessing based on https://android.googlesource.com/platform/dalvik/+/gingerbread/vm/Properties.c#206)How does this sound?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is much better, but still misses the info that IBM J9 VM was used :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah good point - how about
java.vm.vendorconcatenated withjava.vm.version? It seems it will removeAdoptOpenJDKfrom their OpenJ9 JVM but it's probably a better handling of the corner case.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the user agent string be reported in this case? Or some other browser name/version identification? Or would that name be part of the version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User agent parsing is very hairy, so my instinct is to not make the SDK fill in anything smarter here than
browser, and we probably should put the user agent string as theprocess.runtime.version(unless any browser instrumentation developers know of a better one :) ). User agent will also be present in HTTP spans, but if we don't include it in Resource as well we'd miss it on internal spans. I'm not that familiar with browser instrumentation though so happy to hear more thoughts!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think explicitly saying that in case of
browserthe version SHOULD/MUST be set to the user agent (although https://en.wikipedia.org/wiki/User_agent#Deprecation_of_User-Agent_header; JS may have better APIs to get browser version information than the user agent string)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, let me know if it's clear, I don't know how well the new API is supported right now so I'm leaving it out for now but if it seems worth mentioning let me know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry had forgotten to git push >< Now it has my new line about browser.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should not use the "user-agent" in the version. The runtime:
Or maybe completely exclude
browserfrom this list and have a dedicated section forbrowser, there may be other things we need.I don't have big experience with browsers so I may be wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As browser is a drastically different runtime (even the opentelemetry-js distribution is different for server vs browser) I definitely want to include it somewhere. I feel as if the current recommendation provides good information to backends, and is
SHOULD- is it ok to iterate on the details per language? The only language I'm deeply familiar with here is Java, everything else is me fumbling through the dark to provide a starting point for others, but I think a need for polishing by each language SIG is a feature of every language except for Java here, not just this one. Should I go ahead and file an issue for each language to check this table after merging this?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should be good, or maybe just leave TODOs instead of providing things for all the other languages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added TODOs with linked issues. I hope what's here can be helpful for the language owners, though might not be but lean towards not clearing it out since it's at least something I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be TruffleRuby rather than GraalVM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't support IronRuby in OTel Ruby.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should that matter? I guess we don't support many of the things here (yet). But maybe some 3rd party vendor will at some point.
However, I'm wondering if ironruby can even be considered a runtime in it's own right. It is more like an addon for a .NET CLR, isn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Practically speaking, I don't know of anyone using it, or the other implementations below. There's a lot of abandoned alternative Ruby implementations out there. It doesn't seem valuable to catalog them all here.
The active implementations that I know of are JRuby, TruffleRuby and MRI. Although JRuby and TruffleRuby can both be considered "addons" for the JVM, it is important to distinguish them - the implementation technologies and functionality are quite different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH, it feels more useful to have a few well known examples here rather than an exhaustive list of all possible values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM, but in that case, information about the JVM would still be interesting, right? I wonder how we could represent that in semantic conventions. Using an array for each value? E.g. process.runtime.name=["OpenJDK Runtime Environment", "JRuby"]