-
Notifications
You must be signed in to change notification settings - Fork 174
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
Adding a few browser related attribute conventions #559
Conversation
@@ -54,3 +54,19 @@ groups: | |||
to retrieve brands and platform individually from the User-Agent Client Hints API. | |||
To retrieve the value, the legacy `navigator.userAgent` API can be used. | |||
examples: ['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'] | |||
- id: page.url |
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.
Is page.*
really an attribute of the resource browser
? Shouldn't that be rather a signal-level attribute?
In particular: if a user navigates to a different page would that also imply that there's a new instance of the OTel SDK in the browser? If not, the page.*
must not be a resource attribute, because resource attributes must not change during the lifetime of a resource / SDK instance.
Apart from the above, can we use browser.page.url.full
instead? To be consistent with the url.*
namespace. Once we have a solution for #339, we will be able to simply replace it with a reuse of the url.full
attribute under the browser.page.*
namespace
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.
@AlexanderWert yes, in my intended usage of these attributes they are not at the resource level - I added them here so that all attributes under browser
namespace are at one place. I realize this one place is in registry, which didn't exist when this namespace was introduced. I can create the registry entry for browser now, but have one question - these attribute are intended to be added by sdk level processors into both spans and events. Do you want me to create both model/trace/browser.yaml and model/logs/browser.yaml, duplicated?
cc: @martinkuba
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.
@scheler Unfortunately, this is currently blocked by that build-tools issue: open-telemetry/build-tools#242 (We are working on resolving it). |
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Introducing a few attribute conventions under browser namespace.
Changes
Note: if the PR is touching an area that is not listed in the existing areas, or the area does not have sufficient domain experts coverage, the PR might be tagged as experts needed and move slowly until experts are identified.
Merge requirement checklist