From 98b73277442ff635b4be6bd4e88a6758b46d9c78 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Thu, 13 Aug 2020 16:52:51 -0700 Subject: [PATCH 1/4] Add SuppressInstrumentation check in the ActivityListener --- src/OpenTelemetry/Trace/TracerProviderSdk.cs | 34 +++++++++++-------- .../Trace/TracerProvideSdkTest.cs | 18 ++++++++++ 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/OpenTelemetry/Trace/TracerProviderSdk.cs b/src/OpenTelemetry/Trace/TracerProviderSdk.cs index 30c563d718c..41f43b8f43b 100644 --- a/src/OpenTelemetry/Trace/TracerProviderSdk.cs +++ b/src/OpenTelemetry/Trace/TracerProviderSdk.cs @@ -89,21 +89,8 @@ internal TracerProviderSdk( AutoGenerateRootContextTraceId = true, }; - if (sampler is AlwaysOnSampler) - { - listener.GetRequestedDataUsingContext = (ref ActivityCreationOptions options) => ActivityDataRequest.AllDataAndRecorded; - } - else if (sampler is AlwaysOffSampler) - { - /*TODO: Change options.Parent.SpanId to options.Parent.TraceId - once AutoGenerateRootContextTraceId is removed.*/ - listener.GetRequestedDataUsingContext = (ref ActivityCreationOptions options) => PropagateOrIgnoreData(options.Parent.SpanId); - } - else - { - // This delegate informs ActivitySource about sampling decision when the parent context is an ActivityContext. - listener.GetRequestedDataUsingContext = (ref ActivityCreationOptions options) => ComputeActivityDataRequest(options, sampler); - } + // This delegate informs ActivitySource about sampling decision when the parent context is an ActivityContext. + listener.GetRequestedDataUsingContext = (ref ActivityCreationOptions options) => ComputeActivityDataRequest(options, sampler); if (sources.Any()) { @@ -204,6 +191,23 @@ private static ActivityDataRequest ComputeActivityDataRequest( in ActivityCreationOptions options, Sampler sampler) { + if (Sdk.SuppressInstrumentation) + { + return ActivityDataRequest.None; + } + + if (sampler is AlwaysOnSampler) + { + return ActivityDataRequest.AllDataAndRecorded; + } + + if (sampler is AlwaysOffSampler) + { + /*TODO: Change options.Parent.SpanId to options.Parent.TraceId + once AutoGenerateRootContextTraceId is removed.*/ + return PropagateOrIgnoreData(options.Parent.SpanId); + } + // As we set ActivityListener.AutoGenerateRootContextTraceId = true, // Parent.TraceId will always be the TraceId of the to-be-created Activity, // if it get created. diff --git a/test/OpenTelemetry.Tests/Trace/TracerProvideSdkTest.cs b/test/OpenTelemetry.Tests/Trace/TracerProvideSdkTest.cs index 35c37e4022a..e13cb94740d 100644 --- a/test/OpenTelemetry.Tests/Trace/TracerProvideSdkTest.cs +++ b/test/OpenTelemetry.Tests/Trace/TracerProvideSdkTest.cs @@ -147,6 +147,24 @@ public void TracerSdkSetsActivityDataRequestBasedOnSamplingDecision() } } + [Fact] + public void TracerSdkSetsActivityDataRequestToNoneWhenSuppressInstrumentationIsTrue() + { + using var scope = Sdk.SuppressInstrumentation.Begin(); + + var testSampler = new TestSampler(); + using var activitySource = new ActivitySource(ActivitySourceName); + using var sdk = Sdk.CreateTracerProviderBuilder() + .AddSource(ActivitySourceName) + .SetSampler(testSampler) + .Build(); + + using (var activity = activitySource.StartActivity("root")) + { + Assert.Null(activity); + } + } + [Fact] public void ProcessorDoesNotReceiveNotRecordDecisionSpan() { From 048e064b8aef9c0d89112aa98965e1260bdf800f Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Thu, 13 Aug 2020 19:45:39 -0700 Subject: [PATCH 2/4] Add SuppressInstrumentation check in the DiagnosticSourceListner --- .../DiagnosticSourceListener.cs | 5 ++ .../DiagnosticSourceListenerTest.cs | 60 +++++++++++++++++++ .../Instrumentation/TestListenerHandler.cs | 53 ++++++++++++++++ 3 files changed, 118 insertions(+) create mode 100644 test/OpenTelemetry.Tests/Instrumentation/DiagnosticSourceListenerTest.cs create mode 100644 test/OpenTelemetry.Tests/Instrumentation/TestListenerHandler.cs diff --git a/src/OpenTelemetry/Instrumentation/DiagnosticSourceListener.cs b/src/OpenTelemetry/Instrumentation/DiagnosticSourceListener.cs index 2ebb330b3ac..d7112b9365f 100644 --- a/src/OpenTelemetry/Instrumentation/DiagnosticSourceListener.cs +++ b/src/OpenTelemetry/Instrumentation/DiagnosticSourceListener.cs @@ -39,6 +39,11 @@ public void OnError(Exception error) public void OnNext(KeyValuePair value) { + if (Sdk.SuppressInstrumentation) + { + return; + } + if (!this.handler.SupportsNullActivity && Activity.Current == null) { InstrumentationEventSource.Log.NullActivity(value.Key); diff --git a/test/OpenTelemetry.Tests/Instrumentation/DiagnosticSourceListenerTest.cs b/test/OpenTelemetry.Tests/Instrumentation/DiagnosticSourceListenerTest.cs new file mode 100644 index 00000000000..30c243d9be2 --- /dev/null +++ b/test/OpenTelemetry.Tests/Instrumentation/DiagnosticSourceListenerTest.cs @@ -0,0 +1,60 @@ +// +// 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. +// + +using System.Diagnostics; +using Xunit; + +namespace OpenTelemetry.Instrumentation.Tests +{ + public class DiagnosticSourceListenerTest + { + private const string TestSourceName = "TestSourceName"; + private DiagnosticSource diagnosticSource; + private TestListenerHandler testListenerHandler; + private DiagnosticSourceSubscriber testDiagnosticSourceSubscriber; + + public DiagnosticSourceListenerTest() + { + this.diagnosticSource = new DiagnosticListener(TestSourceName); + this.testListenerHandler = new TestListenerHandler(TestSourceName); + this.testDiagnosticSourceSubscriber = new DiagnosticSourceSubscriber(this.testListenerHandler, null); + this.testDiagnosticSourceSubscriber.Subscribe(); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ListenerHandlerIsNotInvokedWhenSuppressInstrumentationTrue(bool suppressInstrumentation) + { + using var scope = Sdk.SuppressInstrumentation.Begin(suppressInstrumentation); + + var activity = new Activity("Main"); + this.diagnosticSource.StartActivity(activity, null); + this.diagnosticSource.StopActivity(activity, null); + + if (suppressInstrumentation) + { + Assert.Equal(0, this.testListenerHandler.OnStartInvokedCount); + Assert.Equal(0, this.testListenerHandler.OnStopInvokedCount); + } + else + { + Assert.Equal(1, this.testListenerHandler.OnStartInvokedCount); + Assert.Equal(1, this.testListenerHandler.OnStopInvokedCount); + } + } + } +} diff --git a/test/OpenTelemetry.Tests/Instrumentation/TestListenerHandler.cs b/test/OpenTelemetry.Tests/Instrumentation/TestListenerHandler.cs new file mode 100644 index 00000000000..aee4188ef3f --- /dev/null +++ b/test/OpenTelemetry.Tests/Instrumentation/TestListenerHandler.cs @@ -0,0 +1,53 @@ +// +// 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. +// + +using System.Diagnostics; + +namespace OpenTelemetry.Instrumentation.Tests +{ + public class TestListenerHandler : ListenerHandler + { + public int OnStartInvokedCount = 0; + public int OnStopInvokedCount = 0; + public int OnExceptionInvokedCount = 0; + public int OnCustomInvokedCount = 0; + + public TestListenerHandler(string sourceName) + : base(sourceName) + { + } + + public override void OnStartActivity(Activity activity, object payload) + { + this.OnStartInvokedCount++; + } + + public override void OnStopActivity(Activity activity, object payload) + { + this.OnStopInvokedCount++; + } + + public override void OnException(Activity activity, object payload) + { + this.OnExceptionInvokedCount++; + } + + public override void OnCustom(string name, Activity activity, object payload) + { + this.OnCustomInvokedCount++; + } + } +} From 70469740cebd7fdae6b830123e27810f29eb1f3a Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Thu, 13 Aug 2020 21:51:16 -0700 Subject: [PATCH 3/4] Fix things post merge --- .../Instrumentation/DiagnosticSourceListenerTest.cs | 2 +- test/OpenTelemetry.Tests/Trace/TracerProvideSdkTest.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/OpenTelemetry.Tests/Instrumentation/DiagnosticSourceListenerTest.cs b/test/OpenTelemetry.Tests/Instrumentation/DiagnosticSourceListenerTest.cs index 30c243d9be2..424c0f50dbb 100644 --- a/test/OpenTelemetry.Tests/Instrumentation/DiagnosticSourceListenerTest.cs +++ b/test/OpenTelemetry.Tests/Instrumentation/DiagnosticSourceListenerTest.cs @@ -39,7 +39,7 @@ public DiagnosticSourceListenerTest() [InlineData(false)] public void ListenerHandlerIsNotInvokedWhenSuppressInstrumentationTrue(bool suppressInstrumentation) { - using var scope = Sdk.SuppressInstrumentation.Begin(suppressInstrumentation); + using var scope = SuppressInstrumentationScope.Begin(suppressInstrumentation); var activity = new Activity("Main"); this.diagnosticSource.StartActivity(activity, null); diff --git a/test/OpenTelemetry.Tests/Trace/TracerProvideSdkTest.cs b/test/OpenTelemetry.Tests/Trace/TracerProvideSdkTest.cs index e13cb94740d..808832c1f2b 100644 --- a/test/OpenTelemetry.Tests/Trace/TracerProvideSdkTest.cs +++ b/test/OpenTelemetry.Tests/Trace/TracerProvideSdkTest.cs @@ -150,7 +150,7 @@ public void TracerSdkSetsActivityDataRequestBasedOnSamplingDecision() [Fact] public void TracerSdkSetsActivityDataRequestToNoneWhenSuppressInstrumentationIsTrue() { - using var scope = Sdk.SuppressInstrumentation.Begin(); + using var scope = SuppressInstrumentationScope.Begin(); var testSampler = new TestSampler(); using var activitySource = new ActivitySource(ActivitySourceName); From d8203edd3a1caa91f2223d6b7bbe6a872b600fca Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Fri, 14 Aug 2020 15:49:02 -0700 Subject: [PATCH 4/4] Put AlwaysOn/AlwaysOff sampler check back in the constructor --- src/OpenTelemetry/Trace/TracerProviderSdk.cs | 37 ++++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/src/OpenTelemetry/Trace/TracerProviderSdk.cs b/src/OpenTelemetry/Trace/TracerProviderSdk.cs index 41f43b8f43b..8905f179fdf 100644 --- a/src/OpenTelemetry/Trace/TracerProviderSdk.cs +++ b/src/OpenTelemetry/Trace/TracerProviderSdk.cs @@ -89,8 +89,24 @@ internal TracerProviderSdk( AutoGenerateRootContextTraceId = true, }; - // This delegate informs ActivitySource about sampling decision when the parent context is an ActivityContext. - listener.GetRequestedDataUsingContext = (ref ActivityCreationOptions options) => ComputeActivityDataRequest(options, sampler); + if (sampler is AlwaysOnSampler) + { + listener.GetRequestedDataUsingContext = (ref ActivityCreationOptions options) => + !Sdk.SuppressInstrumentation ? ActivityDataRequest.AllDataAndRecorded : ActivityDataRequest.None; + } + else if (sampler is AlwaysOffSampler) + { + /*TODO: Change options.Parent.SpanId to options.Parent.TraceId + once AutoGenerateRootContextTraceId is removed.*/ + listener.GetRequestedDataUsingContext = (ref ActivityCreationOptions options) => + !Sdk.SuppressInstrumentation ? PropagateOrIgnoreData(options.Parent.SpanId) : ActivityDataRequest.None; + } + else + { + // This delegate informs ActivitySource about sampling decision when the parent context is an ActivityContext. + listener.GetRequestedDataUsingContext = (ref ActivityCreationOptions options) => + !Sdk.SuppressInstrumentation ? ComputeActivityDataRequest(options, sampler) : ActivityDataRequest.None; + } if (sources.Any()) { @@ -191,23 +207,6 @@ private static ActivityDataRequest ComputeActivityDataRequest( in ActivityCreationOptions options, Sampler sampler) { - if (Sdk.SuppressInstrumentation) - { - return ActivityDataRequest.None; - } - - if (sampler is AlwaysOnSampler) - { - return ActivityDataRequest.AllDataAndRecorded; - } - - if (sampler is AlwaysOffSampler) - { - /*TODO: Change options.Parent.SpanId to options.Parent.TraceId - once AutoGenerateRootContextTraceId is removed.*/ - return PropagateOrIgnoreData(options.Parent.SpanId); - } - // As we set ActivityListener.AutoGenerateRootContextTraceId = true, // Parent.TraceId will always be the TraceId of the to-be-created Activity, // if it get created.