-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat(angular): Export custom browserTracingIntegration()
#10353
Conversation
size-limit report 📦
|
ed19b05
to
af7fc5c
Compare
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.
Nice! I think this is cleaner than the initial approach.
6bb58b7
to
30db58f
Compare
This is now "ready" - @Lms24 could you maybe try this with an actual angular app, just to verify if it really works 😅 |
@mydea yup we should do that. Actually, this is a good opportunity for adding an e2e test app. Will spend some time on this today. |
30db58f
to
4a52c27
Compare
packages/angular/src/tracing.ts
Outdated
): Integration { | ||
// If the user opts out to set this up, we just don't initialize this. | ||
// That way, the TraceService will not actually do anything, functionally disabling this. | ||
if (options.instrumentNavigation === false) { |
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.
I figured this is good enough to allow users to disable this. It means that if users do this:
browserTracingIntegration({ instrumentNavigation: false })
- add the TraceService, they will get the warning "Angular integration has tracing enabled, but Tracing integration is not configured". Id' say that's good enough?
4a52c27
to
8367a00
Compare
8909607
to
b290582
Compare
Also deprecate the routing Instrumentation.
fe7d870
to
4d3828b
Compare
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.
Let's go!
Also deprecate the routing Instrumentation.
This is WIP on top of #10351, to show how that would work.
No idea how to get the tests working for this, though...