Skip to content
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

Change default trace protocol to OpenTelemetry #3469

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sfleen
Copy link
Collaborator

@sfleen sfleen commented Dec 17, 2024

Now that OpenTelemetry has been available for a few releases, we should enable it as the default to begin to phase out OpenCensus.

This sets the default protocol for the proxy, we'll need to make the corresponding changes to the control plane as well.

Now that OpenTelemetry has been available for a few releases, we should enable it as the default to begin to phase out OpenCensus.

Signed-off-by: Scott Fleener <[email protected]>
@sfleen sfleen requested a review from a team as a code owner December 17, 2024 15:47
Copy link

codecov bot commented Dec 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.78%. Comparing base (96124bc) to head (a472c26).
Report is 664 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3469      +/-   ##
==========================================
- Coverage   67.68%   66.78%   -0.90%     
==========================================
  Files         332      388      +56     
  Lines       15158    18139    +2981     
==========================================
+ Hits        10259    12114    +1855     
- Misses       4899     6025    +1126     
Files with missing lines Coverage Δ
linkerd/app/core/src/http_tracing.rs 17.39% <ø> (+6.07%) ⬆️

... and 168 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca50d6b...a472c26. Read the comment docs.

@olix0r
Copy link
Member

olix0r commented Jan 6, 2025

Doesn't the control plane always have to tell the proxy to use the opentelemetry protocol?

This change, aiui, would cause proxies to default to opentelemetry when running with an older control plane that does not know to set this default.

I think we do not want to change the proxy's defaulting behavior with this in mind: the decision should be driven/controlled by the injector.

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