feat(browser): introduce browser.document.url.full#3633
feat(browser): introduce browser.document.url.full#3633mquentin wants to merge 5 commits intoopen-telemetry:mainfrom
Conversation
mquentin
force-pushed
the
feat_browser-sig-introduce_browser.document.url.full
branch
from
f884866 to
372aa5d
Compare
|
I am wondering if the document should be a separate entity from |
Indeed let's go for a dedicated entity I will also add a note to the entity to reflect you point Martin |
| note: > | ||
| `entity.browser.document` is defined as a separate entity from `entity.browser` because | ||
| the `entity.browser` attributes (e.g. brands, platform, language) represent the user agent | ||
| and runtime environment, which are immutable for the lifetime of the SDK. | ||
| In contrast, the document URL changes on every navigation and represents a distinct | ||
| concept tied to the current browser document rather than the browser itself. |
There was a problem hiding this comment.
The note is currently not shown in any of the docs.
Is it meant to be included in the brief?
There was a problem hiding this comment.
So I wasn't sure if it would make sens to have it in the doc at all. Or just as an extra bit of information you have if you look for more details in the entities.yaml like the above user_agent.original does.
All thing considered, you're right it make sens to document this context and explain the two entity differences in a more visible way in the doc. I added it back to the brief. Let me know if it works for you.
There was a problem hiding this comment.
I think this note makes sense in the PR description, but not in the semconv itself - it's part of how we define entities in otel - they are granular. having document and browser as separate entities totally makes sense from entity design perspective, no explanation needed.
What belongs in the conventions: somewhat actionable guidance for instrumentation authors on how to write them and guidance to end users on how to interpret data. The rest (design choices, justification) usually belongs in the PR description / comments. Will leave a comment suggesting to remove it.
| `entity.browser.document` is defined as a separate entity from `entity.browser` because | ||
| the `entity.browser` attributes (e.g. brands, platform, language) represent the user agent | ||
| and runtime environment, which are immutable for the lifetime of the SDK. | ||
| In contrast, the document URL changes on every navigation and represents a distinct | ||
| concept tied to the current browser document rather than the browser itself. |
There was a problem hiding this comment.
| `entity.browser.document` is defined as a separate entity from `entity.browser` because | |
| the `entity.browser` attributes (e.g. brands, platform, language) represent the user agent | |
| and runtime environment, which are immutable for the lifetime of the SDK. | |
| In contrast, the document URL changes on every navigation and represents a distinct | |
| concept tied to the current browser document rather than the browser itself. |
| In contrast, the document URL changes on every navigation and represents a distinct | ||
| concept tied to the current browser document rather than the browser itself. | ||
| attributes: | ||
| - ref: browser.document.url.full |
There was a problem hiding this comment.
| - ref: browser.document.url.full | |
| - ref: browser.document.url.full | |
| role: identifying |
We don't enforce it yet, but all entities should have at least one identifying attribute and all attributes should either be identifying or descriptive. This one looks like identifying - let's document it.
There was a problem hiding this comment.
We do enforce it prior to stabilization - +1 to making this one identifying
lmolkova
left a comment
lmolkova
left a comment
There was a problem hiding this comment.
Looks great, just a couple of comments.
lmolkova
moved this from Untriaged
to Needs More Approval
in Semantic Conventions Triage
9 participants
Fixes open-telemetry/opentelemetry-browser#174
Changes
Following the first PR that has been closed due to Browser SIG semantic discussion: #3519
Following the 5th of March Browser SIG we identified the need instrumenting the on which location url any of the browser OTel signal are generated.
There is an existing url.full attribute holds network urls cf. official description:
Which does not match with a browser page location Url. As a result a RUM signal reporting a click interaction cannot rely on the url.full to report the location url on which the click occurred.
open-telemetry/opentelemetry-browser#174 (comment)
Merge requirement checklist
CONTRIBUTING.md guidelines followed.