Create span when SamplingDecision.Drop

src/OpenTelemetry/CHANGELOG.md

@@ -2,6 +2,13 @@
 
 ## Unreleased
 
+* Create spans even on a sampling decision of drop. This conforms the .NET SDK
+ to the specification regarding
+ [span creation](https:/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk.md#sdk-span-creation).
+ This may have a performance impact as an `Activity` will always be allocated
+ irrespective of sampling decision.
+ ([#3841](https:/open-telemetry/opentelemetry-dotnet/pull/3841))
+
 ## 1.4.0
 
 Released 2023-Feb-24

src/OpenTelemetry/Trace/TracerProviderSdk.cs

@@ -17,7 +17,6 @@
 #nullable enable
 
 using System.Diagnostics;
-using System.Runtime.CompilerServices;
 using System.Text;
 using Microsoft.Extensions.DependencyInjection;
 using OpenTelemetry.Internal;
@@ -240,7 +239,7 @@ internal TracerProviderSdk(
  else if (this.sampler is AlwaysOffSampler)
  {
  listener.Sample = (ref ActivityCreationOptions<ActivityContext> options) =>
- !Sdk.SuppressInstrumentation ? PropagateOrIgnoreData(options.Parent) : ActivitySamplingResult.None;
+ !Sdk.SuppressInstrumentation ? ActivitySamplingResult.PropagationData : ActivitySamplingResult.None;
  this.getRequestedDataAction = this.RunGetRequestedDataAlwaysOffSampler;
  }
  else
@@ -452,23 +451,9 @@ private static ActivitySamplingResult ComputeActivitySamplingResult(
  {
  options = options with { TraceState = samplingResult.TraceStateString };
  }
-
- return activitySamplingResult;
  }
 
- return PropagateOrIgnoreData(options.Parent);
- }
-
- [MethodImpl(MethodImplOptions.AggressiveInlining)]
- private static ActivitySamplingResult PropagateOrIgnoreData(in ActivityContext parentContext)
- {
- var isRootSpan = parentContext.TraceId == default;
-
- // If it is the root span or the parent is remote select PropagationData so the trace ID is preserved
- // even if no activity of the trace is recorded (sampled per OpenTelemetry parlance).
- return (isRootSpan || parentContext.IsRemote)
- ? ActivitySamplingResult.PropagationData
- : ActivitySamplingResult.None;
+ return activitySamplingResult;
  }
 
  private void RunGetRequestedDataAlwaysOnSampler(Activity activity)

test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs

@@ -618,14 +618,6 @@ public async Task CustomPropagatorCalled(bool sample, bool createParentActivity)
  {
  Assert.Equal(contextFromPropagator, exportedItems[0].Context);
  }
-
-#if NETFRAMEWORK
- if (!sample && createParentActivity)
- {
- Assert.Equal(parentContext.TraceId, contextFromPropagator.TraceId);
- Assert.Equal(parentContext.SpanId, contextFromPropagator.SpanId);
- }
-#endif
  }
 
  public void Dispose()

test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.cs

@@ -296,14 +296,6 @@ public async Task CustomPropagatorCalled(bool sample, bool createParentActivity)
  {
  Assert.Equal(contextFromPropagator, exportedItems[0].Context);
  }
-
-#if NETFRAMEWORK
- if (!sample && createParentActivity)
- {
- Assert.Equal(parentContext.TraceId, contextFromPropagator.TraceId);
- Assert.Equal(parentContext.SpanId, contextFromPropagator.SpanId);
- }
-#endif
  }
 
  [Theory]

test/OpenTelemetry.Tests/Trace/TracerProviderSdkTest.cs

@@ -250,39 +250,6 @@ public void TracerProviderSdkSamplerAttributesAreAppliedToActivity(SamplingDecis
  }
  }
 
- [Fact]
- public void TracerSdkSetsActivitySamplingResultAsPropagationWhenParentIsRemote()
- {
- var activitySourceName = Utils.GetCurrentMethodName();
- var testSampler = new TestSampler();
- using var activitySource = new ActivitySource(activitySourceName);
- using var tracerProvider = Sdk.CreateTracerProviderBuilder()
- .AddSource(activitySourceName)
- .SetSampler(testSampler)
- .Build();
-
- testSampler.SamplingAction = (samplingParameters) =>
- {
- return new SamplingResult(SamplingDecision.Drop);
- };
-
- ActivityContext ctx = new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.None, isRemote: true);
-
- using (var activity = activitySource.StartActivity("root", ActivityKind.Server, ctx))
- {
- // Even if sampling returns false, for activities with remote parent,
- // activity is still created with PropagationOnly.
- Assert.NotNull(activity);
- Assert.False(activity.IsAllDataRequested);
- Assert.False(activity.Recorded);
-
- // This is not a root activity and parent is not remote.
- // If sampling returns false, no activity is created at all.
- using var innerActivity = activitySource.StartActivity("inner");
- Assert.Null(innerActivity);
- }
- }
-
  [Fact]
  public void TracerSdkSetsActivitySamplingResultBasedOnSamplingDecision()
  {
@@ -327,16 +294,31 @@ public void TracerSdkSetsActivitySamplingResultBasedOnSamplingDecision()
 
  using (var activity = activitySource.StartActivity("root"))
  {
- // Even if sampling returns false, for root activities,
- // activity is still created with PropagationOnly.
+ // Even if sampling returns false, for root activities and their children,
+ // are still created with PropagationOnly.
+ Assert.NotNull(activity);
+ Assert.False(activity.IsAllDataRequested);
+ Assert.False(activity.Recorded);
+
+ using var innerActivity = activitySource.StartActivity("inner");
+ Assert.NotNull(innerActivity);
+ Assert.False(activity.IsAllDataRequested);
+ Assert.False(activity.Recorded);
+ }
+
+ ActivityContext ctx = new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.None, isRemote: true);
+ using (var activity = activitySource.StartActivity("root", ActivityKind.Server, ctx))
+ {
+ // Even if sampling returns false, for activities with remote parent and their children,
+ // are still created with PropagationOnly.
  Assert.NotNull(activity);
  Assert.False(activity.IsAllDataRequested);
  Assert.False(activity.Recorded);
 
- // This is not a root activity.
- // If sampling returns false, no activity is created at all.
  using var innerActivity = activitySource.StartActivity("inner");
- Assert.Null(innerActivity);
+ Assert.NotNull(innerActivity);
+ Assert.False(innerActivity.IsAllDataRequested);
+ Assert.False(innerActivity.Recorded);
  }
  }