opentelemetry-specification icon indicating copy to clipboard operation
opentelemetry-specification copied to clipboard

Remove process.runtime semconv for browser resource

Open scheler opened this issue 3 years ago • 6 comments

What are you trying to achieve?

I am trying to remove the use of process.runtime.* attributes for browser resource. In the Client Instrumentation SIG we agreed on using only browser. namespace in the browser resource for identification purpose. The process.runtime.* predates the browser namespace and is added in BrowserDetector.ts. Checking with few members in the SIG, it's not clear if anyone is using this detector currently.

We discussed this in the 9/14 JS SIG and here - the plan is to implement a new detector for browser in a separate package and mark the current BrowserDetector in opentelemetry-resources package as deprecated.

So for the purpose of marking this detector as deprecated, we want to remove references to Web Browser from JavaScript Runtimes in process.md first. Note that the process semconv is still in Experimental state.

I will submit a PR for the removal, but creating this issue first to see if there are any concerns from the spec members.

@legendecas @martinkuba @MSNev

scheler avatar Sep 15 '22 17:09 scheler

I support replacing the process.runtime.* for web browsers with the new browser.* attributes. It would be more meaningful to have detailed browser attributes than the plain UA strings. Deprecating the legacy attributes and introducing a new detector helps us avoid making breaking changes as the legacy detector has been shipped since the beginning.

legendecas avatar Sep 16 '22 14:09 legendecas

I'm in favor of this change.

jsuereth avatar Sep 19 '22 14:09 jsuereth

I oppose this change.

Why? Because I think there should be a single resource attribute that tells you from which kind of process/runtime any telemetry is emitted from. Of course, adding additional information in a browser.* namespace is totally fine, but having basic process.runtime.* information there is valuable as it allows generic handling of resource information.

With this change you are requiring handling such as

if (resourceAttributes.contains("browser.whatever") {
 // Handle process type identification based on browser
} else {
  // Handle process type identifaction based on process.runtime
}

Instead I'd like to be able to do handling that I only need to look into browser.* if I am actually interested in browser-specific attributes.

Maybe the current definitions for process.runtime for browser are bad, then they should be changed. But I don't think we should start making this "deprecated" for any technology.

Oberon00 avatar Sep 23 '22 14:09 Oberon00

Why? Because I think there should be a single resource attribute that tells you from which kind of process/runtime any telemetry is emitted from

The attribute telemetry.sdk.language represents the general kind of runtime and should be used first, and process.runtime.* can be used for further identification within each. For browsers there is only one runtime identity.

scheler avatar Sep 23 '22 15:09 scheler

I see telemetry.sdk.language has been repurposed to also distinguish nodejs and webjs even though both use javascript. I did not know that. Still, that doesn't really change my argument. If you want to show a textual description of the runtime, you could use process.runtime.description || process.runtime.name + process.runtime.version everywhere. Why not also for browser-based runtimes?

Oberon00 avatar Oct 04 '22 09:10 Oberon00

I don't know if any vendor wants to show a textual description of the runtime for browser apps. We had discussed this in the Client Instrumentation SIG and this didn't come up as a requirement.

scheler avatar Oct 07 '22 19:10 scheler