-
Notifications
You must be signed in to change notification settings - Fork 13
Description
What happened?
Steps to Reproduce
Set a HTML page that does soft navigations using history.pushState or history.replaceState APIs
OTEL setup
- Setup a tracer provider and a logger provider with console exporters
- register
@opentelemetry/instrumentation-browser-navigation - register
@opentelemetry/instrumentation-user-interaction
Expected Result
when I click on a button that does a soft navigation (with pushState) I get the click span from @opentelemetry/instrumentation-user-interaction and a log from @opentelemetry/instrumentation-browser-navigation
Actual Result
I only get the span from @opentelemetry/instrumentation-user-interaction
Additional Details
After investigating a little but I identified the root cause as a combination of
@opentelemetry/instrumentation-browser-navigationand@opentelemetry/instrumentation-user-interactionpatching the same history APIs (pushState, replaceState ...)@opentelemetry/instrumentation-user-interactionunwrapping APIs when being enabled see@opentelemetry/instrumentation-user-interactionbeing enabled after@opentelemetry/instrumentation-browser-navigation
I've created https://github.com/david-luna/otel-browser-patch-issue/ to reproduce the issue
If @opentelemetry/instrumentation-browser-navigation the issue does not happen because the instrumentation does not unwrap. The result is the API becomes a composition of both patches.
We could use this workaround and move on but IMHO leaving the wrap functionality as is it will become a problem because:
- usually instrumentations unwrap APIs when disabled. This means disabling one of them implies disabling others (or at least the parts that rely on the APIs being patched)
- if we start accepting/implementing more instrumentations the probability of overlapping and wrapping the same APIs increases
I've played a bit with the idea of keeping track of the wrappers applied to a function and created a small utility as a PoC. You can have a look at https://github.com/david-luna/patcharan. Obviously this is just an improvement of the current functionality so maybe there is a better way (nodejs is interested in tracing channels, although is not implemented in browser I think it could be pollyfiled)
I guess sooner or later we need to review the APIs we're using for instrumentations. In the case of wrapping I think the time has come or is close. Let's use this issue to start a conversation about it.