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

Improvements to the tracing instrumentation #162

Open
bfmatei opened this issue Apr 19, 2023 · 1 comment · May be fixed by #928
Open

Improvements to the tracing instrumentation #162

bfmatei opened this issue Apr 19, 2023 · 1 comment · May be fixed by #928
Assignees
Labels
improvement Request a change of an existing feature

Comments

@bfmatei
Copy link
Collaborator

bfmatei commented Apr 19, 2023

Description

The tracing instrumentation brings the OpenTelemetry User Interaction instrumentation by default. Because of this, the tracing instrumentation also loads the OpenTelemetry Zone Context Manager.

The problem is that the User Interaction instrumentation is not bringing enough value to justify enabling it by default, especially because we also allow the users to load their own instrumentation and pass their own context manager.

Even if one passes a "hardcoded" array of instrumentation when initializing the tracing instrumentation and another context manager, zone.js is still loaded.

Proposed solution

There are a couple of solutions I have in mind:

  • do not embed the user interaction instrumentation in the tracing instrumentation package. Doing so would allow us to remove the zone context manager from the default configuration.
  • make the user interaction instrumentation optional along with zone.js context manager. If you choose this path, be sure to load the due using dynamic imports rather than a sync import in order to avoid zone.js from being initialized.

Context

@bfmatei bfmatei added the improvement Request a change of an existing feature label Apr 19, 2023
@codecapitano
Copy link
Collaborator

Thank you Bodan,

I like the idea. Currently I'm more into your first proposed solution since dynamic import() is a relatively new feature and my not work on older browsers.

@codecapitano codecapitano linked a pull request Jan 31, 2025 that will close this issue
3 tasks
@codecapitano codecapitano self-assigned this Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Request a change of an existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants