ParentBasedSampler should not explicitly consider Links (#1851)

* Update test. * Removed link-checking, update comments, changelog * Fix markdown * Remove extra spaces
open-telemetry · Mar 6, 2021 · 21c440a · 21c440a
1 parent af27e78
commit 21c440a
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 40 deletions.
diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md
@@ -29,6 +29,10 @@ please check the latest changes
 * Resource Attributes now accept primitive arrays as values.
   ([#1852](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1852))
 
+* Fixed [#1846](https://github.com/open-telemetry/opentelemetry-dotnet/issues/1846):
+  `ParentBasedSampler` will no longer explicitly consider Activity links.
+  ([#1851](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1851))
+
 ## 1.0.1
 
 Released 2021-Feb-10

diff --git a/src/OpenTelemetry/Trace/ParentBasedSampler.cs b/src/OpenTelemetry/Trace/ParentBasedSampler.cs
@@ -19,7 +19,7 @@
 namespace OpenTelemetry.Trace
 {
     /// <summary>
-    /// Sampler implementation which by default will take a sample if parent Activity or any linked Activity is sampled.
+    /// Sampler implementation which by default will take a sample if parent Activity is sampled.
     /// Otherwise, samples root traces according to the specified root sampler.
     /// </summary>
     /// <remarks>
@@ -114,23 +114,7 @@ public override SamplingResult ShouldSample(in SamplingParameters samplingParame
                 }
             }
 
-            if (samplingParameters.Links != null)
-            {
-                // If any linked context is sampled keep the sampling decision.
-                // TODO: This is not mentioned in the spec.
-                // Follow up with spec to see if context from Links
-                // must be used in ParentBasedSampler.
-                foreach (var parentLink in samplingParameters.Links)
-                {
-                    if ((parentLink.Context.TraceFlags & ActivityTraceFlags.Recorded) != 0)
-                    {
-                        return new SamplingResult(SamplingDecision.RecordAndSample);
-                    }
-                }
-            }
-
-            // If parent was not sampled (and no linked context exists) => delegate to the "not sampled"
-            // inner samplers.
+            // If parent is not sampled => delegate to the "not sampled" inner samplers.
             if (parentContext.IsRemote)
             {
                 return this.remoteParentNotSampled.ShouldSample(samplingParameters);

diff --git a/test/OpenTelemetry.Tests/Trace/ParentBasedSamplerTests.cs b/test/OpenTelemetry.Tests/Trace/ParentBasedSamplerTests.cs
@@ -66,18 +66,12 @@ public void SampledParent()
                         kind: ActivityKind.Client)));
         }
 
+        /// <summary>
+        /// Checks fix for https://github.com/open-telemetry/opentelemetry-dotnet/issues/1846.
+        /// </summary>
         [Fact]
-        public void SampledParentLink()
+        public void DoNotExamineLinks()
         {
-            var notSampledLink = new ActivityLink[]
-            {
-                new ActivityLink(
-                    new ActivityContext(
-                        ActivityTraceId.CreateRandom(),
-                        ActivitySpanId.CreateRandom(),
-                        ActivityTraceFlags.None)),
-            };
-
             var sampledLink = new ActivityLink[]
             {
                 new ActivityLink(
@@ -92,20 +86,10 @@ public void SampledParentLink()
                 ActivitySpanId.CreateRandom(),
                 ActivityTraceFlags.None);
 
-            // Not sampled link, don't sample.
+            // Parent is not sampled - default behavior should be to DROP,
+            // even if a sampled linked activity exists.
             Assert.Equal(
                 new SamplingResult(SamplingDecision.Drop),
-                this.parentBasedOnSampler.ShouldSample(
-                    new SamplingParameters(
-                        parentContext: notSampledParent,
-                        traceId: default,
-                        name: "Span",
-                        kind: ActivityKind.Client,
-                        links: notSampledLink)));
-
-            // Sampled link, sample.
-            Assert.Equal(
-                new SamplingResult(SamplingDecision.RecordAndSample),
                 this.parentBasedOffSampler.ShouldSample(
                     new SamplingParameters(
                         parentContext: notSampledParent,