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

[service] implement a noop tracer provider #11026

Merged
merged 3 commits into from
Sep 5, 2024

Conversation

codeboten
Copy link
Contributor

This is to address some of the memory concerns around the SDK's noop tracer provider. Instead of using the published noop tracer provider, we implement our own to avoid the cost of context propagation, which has been proven to be high enough to cause issues for end users.

Part of #10858

Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

Is there any upstream issue about the

memory concerns around the SDK's noop tracer provider

?

@codeboten
Copy link
Contributor Author

Is there any upstream issue about the

I attended the go SIG on 28-Aug-2024, and the end result of the discussion was that because the no-op tracer still has to propagate context, the proposed solution if we need an alternative was to implement our own no-op tracer provider.

This is to address some of the memory concerns around the SDK's noop tracer provider. Instead of using the
published noop tracer provider, we implement our own to avoid the cost of context propagation, which has
been proven to be high enough to cause issues for end users.

Part of open-telemetry#10858

Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
@codeboten codeboten force-pushed the codeboten/noop-providers branch from 94b236d to c7197a8 Compare September 5, 2024 17:57
@codeboten codeboten marked this pull request as ready for review September 5, 2024 17:57
@codeboten codeboten requested review from a team and songy23 September 5, 2024 17:57
Signed-off-by: Alex Boten <[email protected]>
Copy link

codecov bot commented Sep 5, 2024

Codecov Report

Attention: Patch coverage is 20.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 92.20%. Comparing base (3612194) to head (c60c2d9).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
service/telemetry/tracer.go 20.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11026      +/-   ##
==========================================
- Coverage   92.21%   92.20%   -0.02%     
==========================================
  Files         405      405              
  Lines       19240    19244       +4     
==========================================
  Hits        17743    17743              
- Misses       1130     1134       +4     
  Partials      367      367              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@codeboten codeboten merged commit e99074d into open-telemetry:main Sep 5, 2024
48 of 49 checks passed
@codeboten codeboten deleted the codeboten/noop-providers branch September 5, 2024 18:38
@github-actions github-actions bot added this to the next release milestone Sep 5, 2024
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.

3 participants