Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ please check the latest changes
* Added new constructor with optional parameters to allow customization of
`ParentBasedSampler` behavior. ([#1727](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1727))

* 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
Expand Down
20 changes: 2 additions & 18 deletions src/OpenTelemetry/Trace/ParentBasedSampler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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);
Expand Down
28 changes: 6 additions & 22 deletions test/OpenTelemetry.Tests/Trace/ParentBasedSamplerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
Expand Down