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

Make SuppressInstrumentation an IDisposable #988

Merged
48 changes: 2 additions & 46 deletions src/OpenTelemetry/Sdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,53 +34,9 @@ namespace OpenTelemetry
/// </summary>
public static class Sdk
{
private static readonly TimeSpan DefaultPushInterval = TimeSpan.FromSeconds(60);

private static readonly RuntimeContextSlot<bool> SuppressInstrumentationRuntimeContextSlot = RuntimeContext.RegisterSlot<bool>("otel.suppress_instrumentation");

/// <summary>
/// Gets or sets a value indicating whether automatic telemetry
/// collection in the current context should be suppressed (disabled).
/// Default value: False.
/// </summary>
/// <remarks>
/// Set <see cref="SuppressInstrumentation"/> to <see langword="true"/>
/// when you want to turn off automatic telemetry collection.
/// This is typically used to prevent infinite loops created by
/// collection of internal operations, such as exporting traces over HTTP.
/// <code>
/// public override async Task&lt;ExportResult&gt; ExportAsync(
/// IEnumerable&lt;Activity&gt; batch,
/// CancellationToken cancellationToken)
/// {
/// var currentSuppressionPolicy = Sdk.SuppressInstrumentation;
/// Sdk.SuppressInstrumentation = true;
/// try
/// {
/// await this.SendBatchActivityAsync(batch, cancellationToken).ConfigureAwait(false);
/// return ExportResult.Success;
/// }
/// finally
/// {
/// Sdk.SuppressInstrumentation = currentSuppressionPolicy;
/// }
/// }
/// </code>
/// </remarks>
public static bool SuppressInstrumentation
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
get
{
return SuppressInstrumentationRuntimeContextSlot.Get();
}
public static readonly SuppressInstrumentationScope SuppressInstrumentation = new SuppressInstrumentationScope(false);

[MethodImpl(MethodImplOptions.AggressiveInlining)]
set
{
SuppressInstrumentationRuntimeContextSlot.Set(value);
}
}
private static readonly TimeSpan DefaultPushInterval = TimeSpan.FromSeconds(60);

/// <summary>
/// Creates MeterProvider with the configuration provided.
Expand Down
74 changes: 74 additions & 0 deletions src/OpenTelemetry/SuppressInstrumentationScope.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// <copyright file="SuppressInstrumentationScope.cs" company="OpenTelemetry Authors">
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// </copyright>

using System;
using OpenTelemetry.Context;

namespace OpenTelemetry
{
public sealed class SuppressInstrumentationScope : IDisposable
{
private static readonly RuntimeContextSlot<bool> SuppressInstrumentationRuntimeContextSlot = RuntimeContext.RegisterSlot<bool>("otel.suppress_instrumentation");
Copy link
Member

Choose a reason for hiding this comment

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

Given this is inside a class SuppressInstrumentationScope, we can probably simplify the name to ContextSlot or even Slot.


private readonly bool previousValue;
private bool disposed;

internal SuppressInstrumentationScope(bool value = true)
{
this.previousValue = SuppressInstrumentationRuntimeContextSlot.Get();
SuppressInstrumentationRuntimeContextSlot.Set(value);
}

public static implicit operator bool(SuppressInstrumentationScope x) => SuppressInstrumentationRuntimeContextSlot.Get();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public static implicit operator bool(SuppressInstrumentationScope x) => SuppressInstrumentationRuntimeContextSlot.Get();
public static implicit operator bool(SuppressInstrumentationScope unused) => SuppressInstrumentationRuntimeContextSlot.Get();

Copy link
Member

Choose a reason for hiding this comment

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

Or other name that explains the intention.

Copy link
Member Author

Choose a reason for hiding this comment

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

😄 agree unused is better than x. I changed it. Though this did leave me wondering if people would prefer to be able to use _ in this case. I think it's pretty common practice, though I imagine allowing it means allowing any variable to start with a _ which I'm guessing we don't want. No biggie unused is great.

Copy link
Member

Choose a reason for hiding this comment

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

I like _ and initially I was trying to use it, then caught by a style cop 😄


/// <summary>
/// Begins a new scope in which automatic telemetry is suppressed (disabled).
reyang marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
/// <param name="value">Value indicating whether to suppress automatic telemetry.</param>
reyang marked this conversation as resolved.
Show resolved Hide resolved
/// <returns>Object to dispose to end the scope.</returns>
/// <remarks>
/// This is typically used to prevent infinite loops created by
/// collection of internal operations, such as exporting traces over HTTP.
/// <code>
/// public override async Task&lt;ExportResult&gt; ExportAsync(
/// IEnumerable&lt;Activity&gt; batch,
/// CancellationToken cancellationToken)
/// {
/// using (Sdk.SuppressInstrumentation.Begin())
/// {
/// // Instrumentation is suppressed (i.e., Sdk.SuppressInstrumentation == true)
/// }
///
/// // Instrumentation is not suppressed (i.e., Sdk.SuppressInstrumentation == false)
/// }
/// </code>
/// </remarks>
public IDisposable Begin(bool value = true)
{
return new SuppressInstrumentationScope(value);
}

/// <inheritdoc/>
public void Dispose()
{
if (!this.disposed)
{
SuppressInstrumentationRuntimeContextSlot.Set(this.previousValue);
this.disposed = true;
}
}
}
}
45 changes: 45 additions & 0 deletions test/OpenTelemetry.Tests/SuppressInstrumentationTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// <copyright file="SuppressInstrumentationTest.cs" company="OpenTelemetry Authors">
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// </copyright>

using Xunit;

namespace OpenTelemetry.Tests
{
public class SuppressInstrumentationTest
{
[Fact]
public static void UsingSuppressInstrumentation()
{
using (var scope = Sdk.SuppressInstrumentation.Begin())
{
Assert.True(Sdk.SuppressInstrumentation);

using (var innerScope = Sdk.SuppressInstrumentation.Begin())
{
innerScope.Dispose();

Assert.True(Sdk.SuppressInstrumentation);

scope.Dispose();
}

Assert.False(Sdk.SuppressInstrumentation);
}

Assert.False(Sdk.SuppressInstrumentation);
}
}
}