Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

ref: Add span options aliases (WithSpanSampled, WithOpName, WithTransactionName) #624

Merged
merged 6 commits into from
Apr 13, 2023

Conversation

tonyo
Copy link
Contributor

@tonyo tonyo commented Apr 13, 2023

As a follow-up to #611, let's unify the (applicable) span options by prefixing them with "With":

SpanSampled -> WithSpanSampled
OpName -> WithOpName
TransactionName -> WithTransactionName

The original names are deprecated: not removed right away, but there's a task to do it in a version or two: #625

Not touching ContinueFromRequest, ContinueFromHeaders, and ContinueFromTrace since they have slightly different semantics.

Why?

  1. Without the prefix some newly introduced span option names collide with existing types names and/or Span fields.
  2. This is a common pattern e.g. in OpenTelemetry: https://github.com/open-telemetry/opentelemetry-go/blob/1b55281859bfaf4b03b7968a059c5239cf8c7a20/trace/config.go#L278-L333

Note: some docs will have to be updated after this change is released.

@codecov
Copy link

codecov bot commented Apr 13, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.08 🎉

Comparison is base (e70db81) 79.87% compared to head (57fb4d2) 79.95%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #624      +/-   ##
==========================================
+ Coverage   79.87%   79.95%   +0.08%     
==========================================
  Files          38       38              
  Lines        3875     3881       +6     
==========================================
+ Hits         3095     3103       +8     
+ Misses        673      671       -2     
  Partials      107      107              
Impacted Files Coverage Δ
http/sentryhttp.go 87.27% <100.00%> (ø)
otel/span_processor.go 94.33% <100.00%> (ø)
tracing.go 89.25% <100.00%> (+0.55%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tonyo tonyo merged commit eb22fb5 into master Apr 13, 2023
@tonyo tonyo deleted the tonyo/feat-rename-spanoptions branch April 13, 2023 11:41
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants