Skip to content

ref: Remove usage of span tags#10808

Merged
mydea merged 3 commits intodevelopfrom
fn/stop-using-span-tags
Feb 27, 2024
Merged

ref: Remove usage of span tags#10808
mydea merged 3 commits intodevelopfrom
fn/stop-using-span-tags

Conversation

@mydea
Copy link
Copy Markdown
Member

@mydea mydea commented Feb 26, 2024 •
edited
Loading

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 self-assigned this Feb 26, 2024
@mydea mydea mentioned this pull request Feb 26, 2024
Merged
@github-actions
Copy link
Copy Markdown
Contributor

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

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay, Feedback) - Webpack (gzipped) 77.29 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 68.55 KB (+0.02% 🔺)
@sentry/browser (incl. Tracing, Replay with Canvas) - Webpack (gzipped) 72.46 KB (+0.02% 🔺)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped) 62.05 KB (+0.01% 🔺)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 32.73 KB (+0.03% 🔺)
@sentry/browser (incl. browserTracingIntegration) - Webpack (gzipped) 32.73 KB (+0.03% 🔺)
@sentry/browser (incl. Feedback) - Webpack (gzipped) 30.91 KB (0%)
@sentry/browser (incl. sendFeedback) - Webpack (gzipped) 30.91 KB (0%)
@sentry/browser - Webpack (gzipped) 22.19 KB (0%)
@sentry/browser (incl. Tracing, Replay, Feedback) - ES6 CDN Bundle (gzipped) 75.69 KB (-0.08% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 67.46 KB (-0.06% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 33.24 KB (-0.15% 🔽)
@sentry/browser - ES6 CDN Bundle (gzipped) 24.74 KB (+0.06% 🔺)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 211.04 KB (-0.12% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 99.76 KB (-0.25% 🔽)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 73.96 KB (+0.25% 🔺)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 36.29 KB (-0.2% 🔽)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 68.83 KB (+0.02% 🔺)
@sentry/react - Webpack (gzipped) 22.22 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 85.24 KB (-0.02% 🔽)
@sentry/nextjs Client - Webpack (gzipped) 49.62 KB (-0.08% 🔽)
@sentry-internal/feedback - Webpack (gzipped) 17.14 KB (0%)

@mydea mydea force-pushed the fn/stop-using-span-tags branch from 060bcf8 to 83b1506 Compare February 26, 2024 15:05
Lms24
Lms24 approved these changes Feb 26, 2024
View reviewed changes
@mydea mydea force-pushed the fn/stop-using-span-tags branch from 83b1506 to 077a19e Compare February 26, 2024 16:28
@mydea
Copy link
Copy Markdown
Member Author

mydea commented Feb 27, 2024

Actually, I need to double check with performance folks if we are good to migrate the browser metrics from tags to attributes, not sure if something relies on these...!

@mydea
ref: Refactor most usage of tags on spans
deab246 
Only missing are browser metrics, where we need to verify this can be refactored already.
@mydea mydea mentioned this pull request Feb 27, 2024
Merged
@mydea mydea force-pushed the fn/stop-using-span-tags branch from c237db8 to deab246 Compare February 27, 2024 12:28
@mydea
Copy link
Copy Markdown
Member Author

mydea commented Feb 27, 2024

I ended up extracting the browser metrics changes out into #10823, so we can already merge the "clearer" stuff.

mydea
mydea commented Feb 27, 2024
View reviewed changes
packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts
from: currPathname,
},
attributes: {
from: currPathname,
Copy link
Copy Markdown
Member Author

@mydea mydea Feb 27, 2024

Choose a reason for hiding this comment

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

@lforst Just a reminder for ourselves during the v8 alpha: we need to review all our auto instrumentation for what attribute we set, what we actually need/want, etc.

@mydea mydea merged commit a6a4e53 into develop Feb 27, 2024
@mydea mydea deleted the fn/stop-using-span-tags branch February 27, 2024 15:08
mydea added a commit that referenced this pull request Mar 7, 2024
@mydea
ref(browser): Store browser metrics as attributes instead of tags (#1…
c7c6bb2 
…0823)

Extracted this out from
#10808, as this is a
bit more impactful, possibly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Lms24 Lms24 Lms24 approved these changes

@JonasBa JonasBa Awaiting requested review from JonasBa

+2 more reviewers

@onurtemizkan onurtemizkan onurtemizkan approved these changes

@AbhiPrasad AbhiPrasad AbhiPrasad approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@mydea mydea

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mydea @onurtemizkan @Lms24 @AbhiPrasad