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

feat(angular): Export custom browserTracingIntegration() #10353

Merged
merged 7 commits into from
Jan 31, 2024

Conversation

mydea
Copy link
Member

@mydea mydea commented Jan 25, 2024

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...

@mydea mydea requested a review from Lms24 January 25, 2024 20:16
@mydea mydea self-assigned this Jan 25, 2024
Copy link
Contributor

github-actions bot commented Jan 25, 2024

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay, Feedback) - Webpack (gzipped) 78.11 KB (0%)
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 69.35 KB (0%)
@sentry/browser (incl. Tracing, Replay with Canvas) - Webpack (gzipped) 73.21 KB (0%)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped) 62.96 KB (0%)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 33.36 KB (0%)
@sentry/browser (incl. browserTracingIntegration) - Webpack (gzipped) 33.23 KB (-0.01% 🔽)
@sentry/browser (incl. Feedback) - Webpack (gzipped) 31.33 KB (0%)
@sentry/browser (incl. sendFeedback) - Webpack (gzipped) 31.34 KB (0%)
@sentry/browser - Webpack (gzipped) 22.6 KB (0%)
@sentry/browser (incl. Tracing, Replay, Feedback) - ES6 CDN Bundle (gzipped) 76.07 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 67.63 KB (0%)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 33.46 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped) 24.66 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 213.38 KB (0%)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 101.39 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 74 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 36.6 KB (0%)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 69.71 KB (0%)
@sentry/react - Webpack (gzipped) 22.63 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 86.48 KB (0%)
@sentry/nextjs Client - Webpack (gzipped) 50.78 KB (0%)
@sentry-internal/feedback - Webpack (gzipped) 17.21 KB (0%)

@mydea mydea force-pushed the fn/newBrowserTracingIntegrationOnly branch from ed19b05 to af7fc5c Compare January 25, 2024 21:48
Copy link
Member

@Lms24 Lms24 left a 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.

Base automatically changed from fn/newBrowserTracingIntegrationOnly to develop January 26, 2024 15:54
@mydea mydea marked this pull request as ready for review January 26, 2024 16:12
@mydea
Copy link
Member Author

mydea commented Jan 26, 2024

This is now "ready" - @Lms24 could you maybe try this with an actual angular app, just to verify if it really works 😅

@Lms24
Copy link
Member

Lms24 commented Jan 29, 2024

@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.

): 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) {
Copy link
Member Author

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?

@mydea mydea force-pushed the fn/browserTracingAngular branch 2 times, most recently from 8909607 to b290582 Compare January 31, 2024 09:13
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go!

@mydea mydea merged commit 5fd5c5d into develop Jan 31, 2024
98 checks passed
@mydea mydea deleted the fn/browserTracingAngular branch January 31, 2024 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants