Skip to content

ref(browser): Store browser metrics as attributes instead of tags #10823

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Mar 7, 2024
Merged

ref(browser): Store browser metrics as attributes instead of tags #10823

merged 2 commits into from
Mar 7, 2024

Conversation

mydea
Copy link
Member

@mydea mydea commented Feb 27, 2024

Extracted this out from #10808, as this is a bit more impactful, possibly.

@mydea mydea requested review from lforst, Lms24 and AbhiPrasad February 27, 2024 12:27
@mydea mydea self-assigned this Feb 27, 2024
@mydea mydea mentioned this pull request Feb 27, 2024
Merged
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 27, 2024 •
edited
Loading

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay, Feedback) - Webpack (gzipped) 77.56 KB (+0.23% 🔺)
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 68.8 KB (+0.33% 🔺)
@sentry/browser (incl. Tracing, Replay with Canvas) - Webpack (gzipped) 72.7 KB (+0.28% 🔺)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped) 62.35 KB (+0.34% 🔺)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 33.03 KB (+0.59% 🔺)
@sentry/browser (incl. browserTracingIntegration) - Webpack (gzipped) 33.03 KB (+0.59% 🔺)
@sentry/browser (incl. Feedback) - Webpack (gzipped) 31.36 KB (+0.06% 🔺)
@sentry/browser (incl. sendFeedback) - Webpack (gzipped) 31.37 KB (+0.05% 🔺)
@sentry/browser - Webpack (gzipped) 22.57 KB (+0.05% 🔺)
@sentry/browser (incl. Tracing, Replay, Feedback) - ES6 CDN Bundle (gzipped) 0 B (removed)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 0 B (removed)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 0 B (removed)
@sentry/browser - ES6 CDN Bundle (gzipped) 0 B (removed)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 0 B (removed)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 0 B (removed)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 0 B (removed)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 0 B (removed)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 69.05 KB (+0.26% 🔺)
@sentry/react - Webpack (gzipped) 22.6 KB (+0.05% 🔺)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 85.49 KB (+0.24% 🔺)
@sentry/nextjs Client - Webpack (gzipped) 49.9 KB (+0.4% 🔺)
@sentry-internal/feedback - Webpack (gzipped) 17.41 KB (0%)
@sentry/browser (incl. Tracing, Replay, Feedback) - CDN Bundle (gzipped) 75.63 KB (added)
@sentry/browser (incl. Tracing, Replay) - CDN Bundle (gzipped) 67.24 KB (added)
@sentry/browser (incl. Tracing) - CDN Bundle (gzipped) 33.09 KB (added)
@sentry/browser - CDN Bundle (gzipped) 24.01 KB (added)
@sentry/browser (incl. Tracing, Replay) - CDN Bundle (minified & uncompressed) 211.07 KB (added)
@sentry/browser (incl. Tracing) - CDN Bundle (minified & uncompressed) 99.87 KB (added)
@sentry/browser - CDN Bundle (minified & uncompressed) 71.76 KB (added)

mydea added a commit that referenced this pull request Feb 27, 2024
@mydea
ref: Remove usage of span tags (#10808)
a6a4e53 
Mostly, we rewrite this to just use attributes.

This affects esp.:
* browser profiling
* angular tracing
* http status (only set as attribute now)
* remix tracing
* remove all `routing.instrumentation` type tags completely (this is
covered by origin now anyhow)

Not covered by this are browser metrics, which is a dedicated PR here:
#10823
@mydea mydea force-pushed the fn/browser-metrics-tags branch from b80c63d to da074f0 Compare February 27, 2024 15:11
@mydea
Copy link
Member Author

mydea commented Mar 4, 2024

@AbhiPrasad are we actually good to merge this, or do we need to do any more work for this API side?

@mydea mydea force-pushed the fn/browser-metrics-tags branch from da074f0 to c8d5d1a Compare March 4, 2024 08:37
@AbhiPrasad
Copy link
Member

are we actually good to merge this, or do we need to do any more work for this API side

As long as we've documented this change and performance team is aware we are good to merge this!

@mydea mydea force-pushed the fn/browser-metrics-tags branch from c8d5d1a to b098c71 Compare March 6, 2024 13:40
@mydea mydea mentioned this pull request Mar 6, 2024
Merged
mydea added a commit that referenced this pull request Mar 7, 2024
@mydea
ref(browser): Update browser metrics to avoid deprecations (#10943)
b636963 
This rewrites browser metrics to:

1. Accept spans instead of transactions
2. Use the new performance APIs instead of the old ones

The only thing I left for now is the tags, which we can get rid of
separately: #10823

Part of #10900
@mydea mydea force-pushed the fn/browser-metrics-tags branch 2 times, most recently from ce1a098 to 49d668f Compare March 7, 2024 10:06
@mydea mydea force-pushed the fn/browser-metrics-tags branch from 49d668f to 0952fed Compare March 7, 2024 10:41
@mydea mydea merged commit c7c6bb2 into develop Mar 7, 2024
@mydea mydea deleted the fn/browser-metrics-tags branch March 7, 2024 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lforst lforst lforst approved these changes

@AbhiPrasad AbhiPrasad AbhiPrasad approved these changes

@Lms24 Lms24 Awaiting requested review from Lms24

Assignees

@mydea mydea

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mydea @AbhiPrasad @lforst