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 Resource browser and app attributes, clarify device attributes as client-side only #2115

Closed

Conversation

martinkuba
Copy link
Contributor

In order to identify client-side telemetry, this PR specifies that device attributes should be used only for client-side devices. It also adds a section for browser attributes. The presence of either a device or browser attribute would be used to distinguish that the signal comes from a client-side instrumentation.

This was discussed in the Client-side SIG as well as Spec SIG on 11/2/21

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 10, 2021

CLA Signed

The committers are authorized under a signed CLA.

@martinkuba martinkuba changed the title added section for browsers, clarified device as client-side only Add Resource browser attributes, clarify device attributes as client-side only Nov 10, 2021
@@ -4,7 +4,7 @@

**type:** `device`

**Description**: The device on which the process represented by this resource is running.
**Description**: The device on which the process represented by this resource is running. The `device.*` attributes MUST be used only for resources that represent client-side devices - the presence of these attributes can be used to identify client-side telemetry.
Copy link
Contributor

Choose a reason for hiding this comment

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

I know we had some discussion at the SIG about whether IOT devices would also fall into this category. If they didn't fall into this resource category, where would they end up?

@@ -91,6 +90,23 @@ namespace = Company
service.name = Shop.shoppingcart
```

## App
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this different from existing service.* attributes?

Copy link
Contributor

Choose a reason for hiding this comment

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

An app is not a service. It has different qualitative properties and behaviors. Separating the namespaces will allow backends to account for these differences and treat RUM signals differently.

Copy link
Member

@mtwo mtwo left a comment

Choose a reason for hiding this comment

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

We discussed one or two pending changes during today's call, but LGTM otherwise

| `browser.mobile` | boolean | Flag indicating a mobile device | | No |
| `browser.userAgent` | string | Full user-agent string provided by the browser [1] | `'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/95.0.4638.54 Safari/537.36'` | No |

**[1]:** The user-agent value should be provided only from browsers that do not have a mechanism to retrieve name, version, and platform individually (for example from the User-Agent Client Hints API).
Copy link
Contributor

Choose a reason for hiding this comment

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

Snake case is recommended for words in attribute names, so browser.userAgent should be browser.user_agent.
https://opentelemetry.io/docs/reference/specification/common/attribute-naming/

@martinkuba martinkuba force-pushed the client-side-attributes branch from 7f7a6bb to 9085bce Compare November 23, 2021 20:06
@martinkuba martinkuba marked this pull request as ready for review November 23, 2021 20:07
@martinkuba martinkuba requested review from a team November 23, 2021 20:07
@martinkuba martinkuba changed the title Add Resource browser attributes, clarify device attributes as client-side only Add Resource browser and app attributes, clarify device attributes as client-side only Nov 23, 2021
@svrnm svrnm mentioned this pull request Nov 24, 2021
**type:** `browser`

**Description**: The web browser in which the application represented by the resource is running. The `browser.*` attributes MUST be used only for resources that represent applications running in a web browser. The presence of these attributes is intended to identify client-side telemetry.

Choose a reason for hiding this comment

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

Should spec have semconv for things that are currently mostly added during ingest? I don't think anyone parses useragent client side and there is currently no other way of getting the browser version etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe indeed worth calling out the specific case for the browser telemetry, where the information in browser.* might be represented in User-Agent header in the exported telemetry and not in the payload itself?

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 can make the change. However, I wonder if that will make it unclear whether an SDK should implement collecting these values. I think if we assume that all browsers will send these headers, then the SDKs will not be sending any of these attributes, and we might as well not define them?


**[1]:** MUST be the same for all instances of the application running on individual client devices. If the value was not specified, SDKs MUST fallback to `unknown_application`.

**[2]:** A string value having a meaning that helps to distinguish a group of client applications, for example the team name that owns a group of applications or web site name that consists of multiple components. `app.name` is expected to be unique within the same namespace. If `app.namespace` is not specified in the Resource then `app.name` is expected to be unique for all applications that have no explicit namespace defined (so the empty/unspecified namespace is simply one more valid namespace). Zero-length namespace string is assumed equal to unspecified namespace.

Choose a reason for hiding this comment

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

Is it correct to say that one app can belong to only one app.namespace? In order words, a single app.namespace can have multiple apps, or app.name(s) but an app.name can only belong to one namespace?

Copy link
Contributor

Choose a reason for hiding this comment

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

What about the situation (real world asks from customers) who are capturing telemetry from browser applications with large surface area (hundreds of different screens), built and maintained by different teams but all packaged in one user-facing UI. These customers want to have different app.name to be set on different parts of the application, for example: everything under /users belongs into app.name=User management and everything under /invoice belongs into app.name=Billing

Considering that resources are immutable by the spec, we cannot change the app.name dynamically during runtime. Initializing multiple tracers could be an option, but different issues (such as SPAs in browser applications initializing only on pageloads and not on route changes) as well as issues with global method instrumentations (XHR/fetch in browser) where isolation is not guaranteed so in practice this also does not seem to work.

Wonder what is the recommended path forward to support such cases? Relaxing spec requirements on resource immutability maybe?

Copy link

Choose a reason for hiding this comment

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

Adding onto above, another case where updating resource would be useful when adding user identifiying attributes like enduser.* (while on server request model having them added per span is more useful as generally every request may be served to different users, in a stateful application it'd be simpler to put it into resource (as you'd generally have one user using it at a time) and update when application state changes (user logs in / out) as it happens less frequently)

If not mutating the resource then at least swapping out the currently attached Resource on TracerProvider / MeterProvider / ... so any new spans afterwards would use the new state / easily differentiated based on different Resource instance

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ivomagi @t2t2 Good point, I added this as a topic to the Client-side SIG agenda on 12/1.

@github-actions
Copy link

github-actions bot commented Dec 8, 2021

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions
Copy link

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@MSNev
Copy link
Contributor

MSNev commented Oct 3, 2023

@open-telemetry/specs-approvers @open-telemetry/technical-committee Can we please have this PR reopened for discussion or would you prefer a new PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.