Skip to content

stats/opentelemetry: use TextMapProvider and TracerProvider from TraceOptions instead of otel global #8166

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 13, 2025
Merged

stats/opentelemetry: use TextMapProvider and TracerProvider from TraceOptions instead of otel global #8166

merged 2 commits into from
Mar 13, 2025

Conversation

@vinothkumarr227
Copy link
Contributor

@vinothkumarr227 vinothkumarr227 commented Mar 12, 2025 •
edited by dfawley
Loading

RELEASE NOTES:

  • stats/opentelemetry: fix tracing to use TextMapPropagator and TracerProvider from TraceOptions instead of OpenTelemetry globals.
@codecov
Copy link

codecov bot commented Mar 12, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.38%. Comparing base (5ac9042) to head (cb90395).
Report is 3 commits behind head on master.

Additional details and impacted files 
@@ Coverage Diff @@ ## master #8166 +/- ## ========================================== + Coverage 82.33% 82.38% +0.04%  ========================================== Files 392 393 +1 Lines 39119 39143 +24 ========================================== + Hits 32210 32247 +37  + Misses 5593 5579 -14  - Partials 1316 1317 +1
Files with missing lines Coverage Δ
stats/opentelemetry/client_tracing.go 76.47% <100.00%> (ø)
stats/opentelemetry/server_tracing.go 100.00% <100.00%> (ø)

... and 20 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
@purnesh42H purnesh42H changed the title Refactor: Use TraceOptions TextMapPropagator stats/opentelemetry: use TextMapProvider and Tracer from TraceOptions instead of otel global Mar 12, 2025
@purnesh42H purnesh42H changed the title stats/opentelemetry: use TextMapProvider and Tracer from TraceOptions instead of otel global stats/opentelemetry: use TextMapProvider and TracerProvider from TraceOptions instead of otel global Mar 12, 2025
@purnesh42H purnesh42H self-requested a review March 12, 2025 18:32
purnesh42H
purnesh42H reviewed Mar 12, 2025
View reviewed changes
Copy link
Contributor

@purnesh42H purnesh42H left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for quick fix.
stats/opentelemetry/client_tracing.go Outdated
otelinternaltracing "google.golang.org/grpc/stats/opentelemetry/internal/tracing"
)

const traceName = "grpc-go"
Copy link
Contributor

@purnesh42H purnesh42H Mar 12, 2025

Choose a reason for hiding this comment

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

nit: tracerName?
Copy link
Contributor Author

@vinothkumarr227 vinothkumarr227 Mar 12, 2025

Choose a reason for hiding this comment

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

Done
@purnesh42H purnesh42H self-requested a review March 12, 2025 18:35
@purnesh42H purnesh42H added Area: Observability Includes Stats, Tracing, Channelz, Healthz, Binlog, Reflection, Admin, GCP Observability Type: Bug labels Mar 12, 2025
@purnesh42H purnesh42H added this to the 1.72 Release milestone Mar 12, 2025
@purnesh42H
Copy link
Contributor

purnesh42H commented Mar 12, 2025

@dfawley for second review. Not sure why dependency check is failing. Is it because we removed usages of otel?
@purnesh42H purnesh42H mentioned this pull request Mar 12, 2025
Closed
dfawley
dfawley approved these changes Mar 12, 2025
View reviewed changes
stats/opentelemetry/client_tracing.go
otelinternaltracing "google.golang.org/grpc/stats/opentelemetry/internal/tracing"
)

const tracerName = "grpc-go"
Copy link
Member

@dfawley dfawley Mar 12, 2025

Choose a reason for hiding this comment

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

I don't see this mentioned in the PR description - why is this changed?
Copy link
Contributor

@purnesh42H purnesh42H Mar 13, 2025

Choose a reason for hiding this comment

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

to keep same as metrics https://github.com/grpc/grpc-go/blob/master/stats/opentelemetry/client_metrics.go#L51
@dfawley dfawley assigned purnesh42H and unassigned dfawley Mar 12, 2025
@purnesh42H purnesh42H merged commit 775150f into grpc:master Mar 13, 2025
14 of 15 checks passed
purnesh42H pushed a commit to purnesh42H/grpc-go that referenced this pull request Mar 19, 2025
@vinothkumarr227 @purnesh42H
stats/opentelemetry: use TextMapProvider and TracerProvider from Trac…
daeca53 
…eOptions instead of otel global (grpc#8166)
@purnesh42H purnesh42H mentioned this pull request Mar 19, 2025
Merged
purnesh42H added a commit that referenced this pull request Mar 20, 2025
@purnesh42H
Cherry-pick #8164 #8166 to v1.71.x (#8182)
208e03b
janardhanvissa pushed a commit to janardhanvissa/grpc-go that referenced this pull request Mar 24, 2025
@vinothkumarr227 @janardhankrishna-sai
stats/opentelemetry: use TextMapProvider and TracerProvider from Trac…
b510f30 
…eOptions instead of otel global (grpc#8166)
@arjan-bal arjan-bal modified the milestones: 1.72 Release, 1.71 Release Apr 17, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Area: Observability Includes Stats, Tracing, Channelz, Healthz, Binlog, Reflection, Admin, GCP Observability Type: Bug

4 participants

@vinothkumarr227 @purnesh42H @dfawley @arjan-bal