feat: Add Browser click event#2992
Conversation
There was a problem hiding this comment.
This seems too generic and instead I feel we should aim for tailored click, key etc events. Ideally at a mimum following https://www.w3schools.com/jsref/obj_events.asp but also potentially splitting a couple ie mouse & click. That way we can easily tailor attributes based on the trigger.
Also for click events Why not extend https://opentelemetry.io/docs/specs/semconv/app/app/#event-appwidgetclick but keep the same name?
| - id: browser.page.x | ||
| type: int | ||
| stability: development | ||
| brief: 'Click x (horizontal) coordinates (in pixels) relative to the entire document.' | ||
| examples: [10] | ||
| - id: browser.page.y | ||
| type: int | ||
| stability: development | ||
| brief: 'Click y (vertical) coordinates (in pixels) relative to the entire document.' | ||
| examples: [10] |
There was a problem hiding this comment.
Have we considered using https://opentelemetry.io/docs/specs/semconv/registry/attributes/app/#app-screen-coordinate-x etc
| - id: browser.tag_name | ||
| type: string | ||
| stability: development | ||
| brief: 'Target element tag name obtained via event.target.tagName.' | ||
| examples: ['BUTTON'] |
There was a problem hiding this comment.
Yeah.. I think having multiple events makes sense. It would make it easier to know which attributes to expect vs a bunch of mixed possible attributes depending on type
I wasn't part of the original discussion but it seems they preferred to use browser terminology instead of translating e.g. Some of these
Component is a loaded term in web, it can be confused for example with a React component that can be composed of multiple dom elements.
IMO I would like to have shared events between client SDKs (web and mobile) but they work for both cases, |
So I have seen at https://www.w3schools.com/jsref/obj_mouseevent.asp and page/app are in effect different measurements so perhaps we go with app.page.coordinate.x.
I agree but I think we can improve the description to resolve those issues and use the attribute notes to define how to source it. For widget id = html element id and fall back to xpath when not defined. |
Co-authored-by: James Thompson <thompson.tomo@outlook.com>
|
Could someone from the browser sig or a maintainer add the triage accepted label so that the hw.mouse.button can be added to hardware namespace and not have this PR auto closed? |
| - id: hw.mouse.button | ||
| type: | ||
| members: | ||
| - id: left | ||
| value: 'left' | ||
| brief: Left button | ||
| stability: development | ||
| - id: middle | ||
| value: 'middle' | ||
| brief: Middle button | ||
| stability: development | ||
| - id: right | ||
| value: 'right' | ||
| brief: Right button | ||
| stability: development | ||
| stability: development | ||
| brief: 'User friendly name of the mouse button pressed. See [MouseEvent.buttons](https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/buttons#value).' | ||
| examples: ["left"] |
There was a problem hiding this comment.
These should be in the registry file in the hardware folder. But only make the change once the triage:accepted label has been added.
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
@joaquin-diaz could you merge in main/rebase this branch followed by a regeneration of the docs? |
|
This PR has been labeled as stale due to lack of activity. It will be automatically closed if there is no further activity over the next 14 days. |
|
@joaquin-diaz @martinkuba the current span based instrumentation for click allows for linking the click to any xhr/fetch calls made from the click handler, which is helpful to know. The approach used there is to propagate trace context and use a common trace across click and all the xhr/fetch calls. This approach poses a problem that the same API calls are modeled different with and without RUM instrumentation, as these API calls get their own traceId when RUM is not involved. I prefer modeling them using independent traces. However, I do find it helpful to have the xhr/fetch calls linked to the click event. a) is there a span.links equivalent for events where we can add related spans, events, etc? |
An event can only be linked to 1 span but a span can have many events.
My suggestion is that we should have an app interaction span which acts as the root span. This root span could then contain the navigation, http etc child spans. |
This is what the current user-interaction instrumentation does today; however, it has the issue I described above. The trace context ends up propagating to the backend instrumentation, whose behavior then varies depending on whether the user interaction instrumentation is set up or not, and this is not ideal. |
|
Are we not solving that by having the attributes on the root span for the span initiator hence removing the need for inference based on the parent span? |
|
This PR has been labeled as stale due to lack of activity. It will be automatically closed if there is no further activity over the next 7 days. |
|
Thanks, this PR is very useful context for my proposal in open-telemetry/opentelemetry-specification#4975. I agree the right direction is not to introduce a generic new “user action” family, but to build from existing app events where possible. For native mobile, my intent is:
So I’m treating #2992 as prior art rather than trying to redefine click from scratch. |
Changes
Added a new event for browsers named user_action.click. This event captures the "click" action in browsers
Prototype instrumentation:
open-telemetry/opentelemetry-browser#35
Important
Pull requests acceptance are subject to the triage process as described in Issue and PR Triage Management.
PRs that do not follow the guidance above, may be automatically rejected and closed.
Merge requirement checklist
[chore]