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

Disable flag for TracerProvider / MeterProvider #3583

Conversation

rajkumar-rangaraj
Copy link
Contributor

Fixes #3455 .

Changes

Added disable flag in MeterProviderBuilderBase and TracerProviderBuilderBase. When disable flag is set, builder will create a default MeterProvider or TracerProvider. From OpenTelemetry Auto-Instrumentation this could be controlled through reflection.

For significant contributions please make sure you have completed the following items:

  • Appropriate CHANGELOG.md updated for non-trivial changes
  • Design discussion issue #
  • Changes in public API reviewed

@open-telemetry/dotnet-instrumentation-approvers , FYI.

@rajkumar-rangaraj rajkumar-rangaraj requested a review from a team August 17, 2022 20:42
@rajkumar-rangaraj rajkumar-rangaraj changed the title flag to disable sdk Disable flag for TracerProvider / MeterProvider Aug 17, 2022
@codecov
Copy link

codecov bot commented Aug 17, 2022

Codecov Report

Merging #3583 (55db43f) into main (5208ec0) will decrease coverage by 0.12%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3583      +/-   ##
==========================================
- Coverage   87.35%   87.23%   -0.13%     
==========================================
  Files         282      282              
  Lines       10132    10137       +5     
==========================================
- Hits         8851     8843       -8     
- Misses       1281     1294      +13     
Impacted Files Coverage Δ
src/OpenTelemetry.Api/Metrics/MeterProvider.cs 100.00% <100.00%> (ø)
.../OpenTelemetry/Metrics/MeterProviderBuilderBase.cs 88.13% <100.00%> (+0.41%) ⬆️
...lemetry/Trace/Builder/TracerProviderBuilderBase.cs 91.96% <100.00%> (+0.14%) ⬆️
src/OpenTelemetry/Logs/Pool/LogRecordSharedPool.cs 78.94% <0.00%> (-21.06%) ⬇️
...tation/OpenTelemetryProtocolExporterEventSource.cs 75.00% <0.00%> (-10.00%) ⬇️
...emetry.Api/Internal/OpenTelemetryApiEventSource.cs 73.52% <0.00%> (-5.89%) ⬇️
...tpListener/Internal/PrometheusCollectionManager.cs 78.04% <0.00%> (-2.44%) ⬇️
src/OpenTelemetry/Logs/OpenTelemetryLogger.cs 86.66% <0.00%> (-2.23%) ⬇️
....Prometheus.HttpListener/PrometheusHttpListener.cs 81.33% <0.00%> (-1.34%) ⬇️
...Telemetry/Internal/SelfDiagnosticsEventListener.cs 97.65% <0.00%> (+0.78%) ⬆️
... and 2 more

@CodeBlanch
Copy link
Member

@rajkumar-rangaraj Would you be open to working on this after #3533 is done? I'm in the middle of changing this stuff not totally sure how this will fit in.

@rajkumar-rangaraj
Copy link
Contributor Author

@rajkumar-rangaraj Would you be open to working on this after #3533 is done? I'm in the middle of changing this stuff not totally sure how this will fit in.

@CodeBlanch sure, I will wait for #3533 to complete.

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or Pushing will instruct the bot to automatically remove the label. This bot runs once per day.

@github-actions github-actions bot added the Stale Issues and pull requests which have been flagged for closing due to inactivity label Aug 25, 2022
@github-actions github-actions bot removed the Stale Issues and pull requests which have been flagged for closing due to inactivity label Sep 1, 2022
@pellared
Copy link
Member

pellared commented Sep 6, 2022

@rajkumar-rangaraj I see that #3533 is merged. Can you please double-check and clarify if it is ready for review?


// Builds noop MeterProvider when this flag is set.
// This flag is controlled through reflection.
private static bool disableMeterProviderBuilder = false;
Copy link
Member

Choose a reason for hiding this comment

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

Is there a scenario where metrics/traces need to be disabled independently?
What is the contract and how to maintain the contract?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe consider something similar to

// An integer value which controls whether instrumentation should be suppressed (disabled).
// * 0: instrumentation is not suppressed
// * [int.MinValue, -1]: instrumentation is always suppressed
// * [1, int.MaxValue]: instrumentation is suppressed in a reference-counting mode
private static readonly RuntimeContextSlot<int> Slot = RuntimeContext.RegisterSlot<int>("otel.suppress_instrumentation");
.

@rajkumar-rangaraj
Copy link
Contributor Author

Will be addressed through #3639

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.

OpenTelemetry .NET Auto-Instrumentation/SDK Side-by-Side Support
4 participants