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.

Sign up for GitHub

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

Jump to bottom

Add process.runtime.name / version describing executing runtime #882

Merged
merged 22 commits into from
Sep 22, 2020
Merged

Add process.runtime.name / version describing executing runtime #882

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
49592a3
Add process.runtime.name / version to capture information about execu…
Aug 26, 2020
2a3feba
.NET core probably considered reference implementation going forward.
Aug 26, 2020
9f441cd
Changelog
Aug 26, 2020
ce631d5
Cleanups
Aug 26, 2020
0826034
Fix go compiler
Aug 26, 2020
0b19aa9
Compiler
Aug 26, 2020
a0482e0
Update specification/resource/semantic_conventions/process.md
Aug 27, 2020
84f3423
Merge branch 'master' into runtime-name
arminru Sep 8, 2020
c201764
Merge branch 'master' into runtime-name
bogdandrutu Sep 9, 2020
10df2c7
Java not constants
Sep 16, 2020
afedfac
Merge branch 'runtime-name' of github.com:anuraaga/opentelemetry-spec…
Sep 16, 2020
31e2ffe
Space
Sep 16, 2020
e5596f2
Ecilpse:
Sep 16, 2020
faf5381
Add TODOs
Sep 17, 2020
7906c26
Avoid direct links
Sep 17, 2020
9eec3e5
Revert "Avoid direct links"
Sep 17, 2020
6a3aca1
Try direct link
Sep 17, 2020
090e92e
Update Ruby
Sep 18, 2020
25fe6d2
Update specification/resource/semantic_conventions/process.md
Sep 18, 2020
9544066
Consistency
Sep 18, 2020
054fc9e
lint
Sep 18, 2020
accbb0a
Android
Sep 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ Updates:
([#611](https://github.com/open-telemetry/opentelemetry-specification/pull/611))
- Version attributes no longer have a prefix such as semver:
([#873](https://github.com/open-telemetry/opentelemetry-specification/pull/873))
- Add semantic conventions for process runtime
([#882](https://github.com/open-telemetry/opentelemetry-specification/pull/882))

## v0.6.0 (07-01-2020)

Expand Down
80 changes: 80 additions & 0 deletions specification/resource/semantic_conventions/process.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,85 @@
| process.command | The command used to launch the process (i.e. the command name). On Linux based systems, can be set to the zeroth string in `proc/[pid]/cmdline`. On Windows, can be set to the first parameter extracted from `GetCommandLineW`. | `cmd/otelcol` | See below |
| process.command_line | The full command used to launch the process. The value can be either a list of strings representing the ordered list of arguments, or a single string representing the full command. On Linux based systems, can be set to the list of null-delimited strings extracted from `proc/[pid]/cmdline`. On Windows, can be set to the result of `GetCommandLineW`. | Linux: `[ cmd/otecol, --config=config.yaml ]`, Windows: `cmd/otecol --config=config.yaml` | See below |
| process.owner | The username of the user that owns the process. | `root` | No |
| process.runtime.name | The name of the runtime of this process. For compiled native binaries, this SHOULD be the name of the compiler. | `openjdk` | No |
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid confusion: is Java a "compiled" in the sense of this line?

Copy link
Contributor

Choose a reason for hiding this comment

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

@anuraaga ping :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Finally updating this PR :) I think no, I use compiled native to try to give that impression. Is there a more clear term?

| process.runtime.version | The version of the runtime of this process, as returned by the runtime without modification. | `14.0.2` | No |

At least one of `process.executable.name`, `process.executable.path`, `process.command`, or `process.command_line` is required.

`process.runtime.name` SHOULD be set to one of the values listed below.
arminru marked this conversation as resolved.
Show resolved Hide resolved
If none of the listed values apply, a custom value best describing the runtime CAN be used.

***Erlang Runtimes:***

| Value | Description |
| --- | --- |
| `beam` | BEAM |
| `jam` | JAM |
anuraaga marked this conversation as resolved.
Show resolved Hide resolved

***Go Runtimes:***

| Value | Description |
| --- | --- |
| `gc` | Go compiler |
| `gccgo` | GCC Go frontend |
Comment on lines +42 to +45
Copy link
Member

Choose a reason for hiding this comment

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

These seem to cover the Go language implementations that I'm aware of. I'm not sure we've ever tested with gccgo, but as an example it is fine.

Version information can be obtained from the runtime.Version() function and should indicate either a semver identifier or a commit hash and date in the case of a non-release build of the toolchain.


***Java runtimes:***

| Value | Description |
| --- | --- |
| `openjdk` | Oracle OpenJDK |
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

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.vendor system property? I suspect that will be the most common source of this information.

Copy link
Contributor Author

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.

Copy link
Contributor

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.name to be mean "which java distribution is in use", then I think we should use java.vendor verbatim (not parsing) as part of process.runtime.name. I don't know if this is enough. E.g. in case of Zulu and Zing, do they both have Azul Systems as vendor? I would expect so. Then we need something else in addition to vendor name.

Copy link
Contributor Author

@anuraaga anuraaga Sep 14, 2020 •
edited
Loading

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.vendor and java.vendor.version. But this doesn't really work because adoptopenjdk is AdoptOpenJDK AdoptOpenJDK in that case.

I'm starting to think I also need to add runtime.description attribute 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. so

runtime.name = java.runtime.name
runtime.version = java.runtime.version
runtime.description = java.vendor + java.vendor.version

It'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-CA
OpenJDK 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?

Copy link
Contributor

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 :(

Copy link
Contributor Author

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.vendor concatenated with java.vm.version? It seems it will remove AdoptOpenJDK from their OpenJ9 JVM but it's probably a better handling of the corner case.

| `adoptopenjdk` | AdoptOpenJDK |
| `amazon-corretto` | Amazon Corretto |
| `dragonwell` | Alibaba Dragonwell |
| `graalvm` | GraalVM |
| `liberica-openjdk` | Liberica OpenJDK |
| `ojdkbuild` | ojdkbuild |
| `oraclejdk` | Oracle JDK |
| `redhat-openjdk` | Red Hat build of OpenJDK |
| `sapmachine` | SapMachine |
| `zulu-openjdk` | Zulu OpenJDK |

***JavaScript runtimes:***

| Value | Description |
| --- | --- |
| `nodejs` | NodeJS |
| `browser` | Web Browser |
Copy link
Member

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?

Copy link
Contributor Author

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 the process.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!

Copy link
Member

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 browser the 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)

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Member

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:

  • name - should probably be the name of the browser (Chrome/Firefox/etc)
  • version - should probably the version of that browser

Or maybe completely exclude browser from this list and have a dedicated section for browser, there may be other things we need.

I don't have big experience with browsers so I may be wrong.

Copy link
Contributor Author

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?

Copy link
Member

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.

Copy link
Contributor Author

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.

| `iojs` | io.js |
| `graalvm` | GraalVM |

When the value is `browser`, `process.runtime.version` SHOULD be set to the User-Agent header.

***.NET Runtimes:***

| Value | Description |
| --- | --- |
| `dotnet-core` | .NET Core, .NET 5+ |
| `dotnet-framework` | .NET Framework |
| `mono` | Mono |

***Python Runtimes:***

| Value | Description |
| --- | --- |
| `cpython` | CPython |
| `graalvm` | GraalVM |
| `ironpython` | IronPython |
| `jython` | Jython |
| `pypy` | PyPy|
| `pythonnet` | PythonNet |

***Ruby Runtimes:***

| Value | Description |
| --- | --- |
| `rubymri` | Ruby MRI |
| `yarv` | YARV |
Copy link
Contributor

Choose a reason for hiding this comment

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

Practically speaking, these are the same thing. YARV is the bytecode interpreter for MRI.

| `graalvm` | GraalVM |
Copy link
Contributor

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.

| `ironruby` | IronRuby |
Copy link
Contributor

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.

Suggested change
| `ironruby` | IronRuby |

Copy link
Member

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?

Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Member

Choose a reason for hiding this comment

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

Although JRuby and TruffleRuby can both be considered "addons" for the JVM, it is important to distinguish them

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"]

| `jruby` | JRuby |
| `macruby` | MacRuby |
| `maglev` | MagLev |
| `mruby` | Mruby |
| `rubinius` | Rubinius |
| `rubymotion` | RubyMotion |
Copy link
Contributor

Choose a reason for hiding this comment

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

None of these are supported by OTel Ruby.

Suggested change
| `macruby` | MacRuby |
| `maglev` | MagLev |
| `mruby` | Mruby |
| `rubinius` | Rubinius |
| `rubymotion` | RubyMotion |

Copy link
Member

Choose a reason for hiding this comment

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

That should not matter, we are not writing these semantic conventions solely for our current implementations 😃