Skip to content

ref: Cleanup browser profiling integration #10766

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

Merged
merged 2 commits into from
Feb 21, 2024
Merged

Conversation

mydea
Copy link
Member

@mydea mydea commented Feb 21, 2024

This does two things:

  1. Remove the deprecated class-based BrowserProfilingIntegration
  2. Remove the onProfilingStartRouteTransaction method - this shouldn't be needed anymore, as now the execution order of browser tracing is ensured to be after browser profiling, ensuring that we had the chance to register the startTransaction hook before browser tracing started a transaction (as that now runs in afterAllSetup)

@mydea mydea requested review from JonasBa and AbhiPrasad February 21, 2024 12:57
@mydea mydea self-assigned this Feb 21, 2024
This does two things:
1. Remove the deprecated class-based `BrowserProfilingIntegration`
2. Remove the `onProfilingStartRouteTransaction` method - this shouldn't be needed anymore, as now the execution order of browser tracing is ensured to be after browser profiling, ensuring that we had the chance to register the `startTransaction` hook before browser tracing started a transaction (as that now runs in `afterAllSetup`)
@mydea mydea force-pushed the fn/browser-profiling branch from 81b82b1 to 16baa7b Compare February 21, 2024 14:40
Copy link
Contributor

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay, Feedback) - Webpack (gzipped) 77.79 KB (-0.03% 🔽)
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 69.04 KB (-0.05% 🔽)
@sentry/browser (incl. Tracing, Replay with Canvas) - Webpack (gzipped) 72.98 KB (-0.05% 🔽)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped) 62.56 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 33.25 KB (-0.08% 🔽)
@sentry/browser (incl. browserTracingIntegration) - Webpack (gzipped) 33.15 KB (-0.08% 🔽)
@sentry/browser (incl. Feedback) - Webpack (gzipped) 31.13 KB (-0.03% 🔽)
@sentry/browser (incl. sendFeedback) - Webpack (gzipped) 31.13 KB (-0.04% 🔽)
@sentry/browser - Webpack (gzipped) 22.41 KB (+0.04% 🔺)
@sentry/browser (incl. Tracing, Replay, Feedback) - ES6 CDN Bundle (gzipped) 76.03 KB (-0.08% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 67.54 KB (-0.07% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 33.5 KB (-0.16% 🔽)
@sentry/browser - ES6 CDN Bundle (gzipped) 24.69 KB (-0.1% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 212.31 KB (-0.06% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 101.03 KB (-0.12% 🔽)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 73.87 KB (-0.05% 🔽)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 36.8 KB (-0.12% 🔽)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 69.35 KB (-0.04% 🔽)
@sentry/react - Webpack (gzipped) 22.44 KB (+0.02% 🔺)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 86.83 KB (-0.02% 🔽)
@sentry/nextjs Client - Webpack (gzipped) 50.06 KB (-0.04% 🔽)
@sentry-internal/feedback - Webpack (gzipped) 17.16 KB (0%)

@AbhiPrasad
Copy link
Member

I'm going to merge this in because it changes stuff with span.op, and I'll fix the merge conflict in my PR (#10767)

@AbhiPrasad AbhiPrasad merged commit deb41bc into develop Feb 21, 2024
@AbhiPrasad AbhiPrasad deleted the fn/browser-profiling branch February 21, 2024 16:35
AbhiPrasad pushed a commit that referenced this pull request Feb 21, 2024
This does two things:
1. Remove the deprecated class-based `BrowserProfilingIntegration`
2. Remove the `onProfilingStartRouteTransaction` method - this shouldn't
be needed anymore, as now the execution order of browser tracing is
ensured to be after browser profiling, ensuring that we had the chance
to register the `startTransaction` hook before browser tracing started a
transaction (as that now runs in `afterAllSetup`)
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.

2 participants