Skip to content

Commit 810fba4

Browse files
martincostellomartincostello
authored
[OpenTelemetry] Optimize trace sampling (#7057)
1 parent 78dffdc commit 810fba4

File tree

3 files changed

+154
-7
lines changed

3 files changed

+154
-7
lines changed

‎src/OpenTelemetry/Trace/Sampler/SamplingResult.cs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ namespace OpenTelemetry.Trace;
88
/// </summary>
99
public readonly struct SamplingResult : IEquatable<SamplingResult>
1010
{
11+
// Null when no attributes were supplied; avoids a GetEnumerator() call (and enumerator boxing)
12+
// on the hot path inside TracerProviderSdk when the sampler returns no attributes.
13+
private readonly IEnumerable<KeyValuePair<string, object>>? attributesField;
14+
1115
/// <summary>
1216
/// Initializes a new instance of the <see cref="SamplingResult"/> struct.
1317
/// </summary>
@@ -61,7 +65,7 @@ public SamplingResult(SamplingDecision decision, IEnumerable<KeyValuePair<string
6165
// Note: Decision object takes ownership of the collection.
6266
// Current implementation has no means to ensure the collection will not be modified by the caller.
6367
// If this behavior will be abused we must switch to cloning of the collection.
64-
this.Attributes = attributes ?? [];
68+
this.attributesField = attributes;
6569

6670
this.TraceStateString = traceStateString;
6771
}
@@ -74,13 +78,16 @@ public SamplingResult(SamplingDecision decision, IEnumerable<KeyValuePair<string
7478
/// <summary>
7579
/// Gets a map of attributes associated with the sampling decision.
7680
/// </summary>
77-
public IEnumerable<KeyValuePair<string, object>> Attributes { get; }
81+
public IEnumerable<KeyValuePair<string, object>> Attributes => this.attributesField ?? [];
7882

7983
/// <summary>
8084
/// Gets the tracestate.
8185
/// </summary>
8286
public string? TraceStateString { get; }
8387

88+
// Internal accessor used by TracerProviderSdk to skip iteration entirely when null.
89+
internal IEnumerable<KeyValuePair<string, object>>? AttributesOrNull => this.attributesField;
90+
8491
/// <summary>
8592
/// Compare two <see cref="SamplingResult"/> for equality.
8693
/// </summary>

‎src/OpenTelemetry/Trace/TracerProviderSdk.cs‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ internal TracerProviderSdk(
222222

223223
if (this.Sampler is AlwaysOnSampler)
224224
{
225-
activityListener.Sample = (ref options) =>
225+
activityListener.Sample = static (ref _) =>
226226
!Sdk.SuppressInstrumentation ? ActivitySamplingResult.AllDataAndRecorded : ActivitySamplingResult.None;
227227
this.getRequestedDataAction = this.RunGetRequestedDataAlwaysOnSampler;
228228
}
@@ -483,9 +483,12 @@ private static ActivitySamplingResult ComputeActivitySamplingResult(
483483

484484
if (activitySamplingResult > ActivitySamplingResult.PropagationData)
485485
{
486-
foreach (var att in samplingResult.Attributes)
486+
if (samplingResult.AttributesOrNull is { } attributes)
487487
{
488-
options.SamplingTags.Add(att.Key, att.Value);
488+
foreach (var att in attributes)
489+
{
490+
options.SamplingTags.Add(att.Key, att.Value);
491+
}
489492
}
490493
}
491494

@@ -579,9 +582,12 @@ private void RunGetRequestedDataOtherSampler(Activity activity)
579582

580583
if (samplingResult.Decision != SamplingDecision.Drop)
581584
{
582-
foreach (var att in samplingResult.Attributes)
585+
if (samplingResult.AttributesOrNull is { } attributes)
583586
{
584-
activity.SetTag(att.Key, att.Value);
587+
foreach (var att in attributes)
588+
{
589+
activity.SetTag(att.Key, att.Value);
590+
}
585591
}
586592
}
587593

‎test/Benchmarks/Trace/SamplingResultBenchmarks.cs‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
// Copyright The OpenTelemetry Authors
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
using System.Diagnostics;
5+
using BenchmarkDotNet.Attributes;
6+
using OpenTelemetry;
7+
using OpenTelemetry.Trace;
8+
9+
namespace Benchmarks.Trace;
10+
11+
#pragma warning disable CA1001 // Types that own disposable fields should be disposable - handled by GlobalCleanup
12+
[MemoryDiagnoser]
13+
public class SamplingResultBenchmarks
14+
#pragma warning restore CA1001 // Types that own disposable fields should be disposable - handled by GlobalCleanup
15+
{
16+
private static readonly KeyValuePair<string, object>[] SamplingAttributes =
17+
[
18+
new("sampling.priority", 1),
19+
new("sampling.rule", "always"),
20+
];
21+
22+
private ActivitySource? sourceNoAttributes;
23+
private ActivitySource? sourceWithAttributeArray;
24+
private ActivitySource? sourceWithAttributeList;
25+
private ActivitySource? sourceDrop;
26+
private ActivitySource? sourceParentBased;
27+
28+
private ActivityContext sampledRemoteParent;
29+
30+
private TracerProvider? providerNoAttributes;
31+
private TracerProvider? providerWithAttributeArray;
32+
private TracerProvider? providerWithAttributeList;
33+
private TracerProvider? providerDrop;
34+
private TracerProvider? providerParentBased;
35+
36+
[GlobalSetup]
37+
public void Setup()
38+
{
39+
this.sourceNoAttributes = new ActivitySource("SamplingResult.NoAttributes");
40+
this.sourceWithAttributeArray = new ActivitySource("SamplingResult.WithAttributeArray");
41+
this.sourceWithAttributeList = new ActivitySource("SamplingResult.WithAttributeList");
42+
this.sourceDrop = new ActivitySource("SamplingResult.Drop");
43+
this.sourceParentBased = new ActivitySource("SamplingResult.ParentBased");
44+
45+
this.sampledRemoteParent = new ActivityContext(
46+
ActivityTraceId.CreateRandom(),
47+
ActivitySpanId.CreateRandom(),
48+
ActivityTraceFlags.Recorded,
49+
traceState: null,
50+
isRemote: true);
51+
52+
// Sampler returns RecordAndSample with no attributes - the common case.
53+
this.providerNoAttributes = Sdk.CreateTracerProviderBuilder()
54+
.AddSource(this.sourceNoAttributes.Name)
55+
.SetSampler(new DelegateSampler(_ => new SamplingResult(SamplingDecision.RecordAndSample)))
56+
.Build();
57+
58+
// Sampler returns attributes as a T[] - exercises the array fast-path.
59+
this.providerWithAttributeArray = Sdk.CreateTracerProviderBuilder()
60+
.AddSource(this.sourceWithAttributeArray.Name)
61+
.SetSampler(new DelegateSampler(_ => new SamplingResult(SamplingDecision.RecordAndSample, SamplingAttributes)))
62+
.Build();
63+
64+
// Sampler returns attributes as a List<T> - exercises the IEnumerable fallback path.
65+
this.providerWithAttributeList = Sdk.CreateTracerProviderBuilder()
66+
.AddSource(this.sourceWithAttributeList.Name)
67+
.SetSampler(new DelegateSampler(_ => new SamplingResult(SamplingDecision.RecordAndSample, [.. SamplingAttributes])))
68+
.Build();
69+
70+
// Sampler drops the span - attribute loop is never entered.
71+
this.providerDrop = Sdk.CreateTracerProviderBuilder()
72+
.AddSource(this.sourceDrop.Name)
73+
.SetSampler(new DelegateSampler(_ => new SamplingResult(SamplingDecision.Drop)))
74+
.Build();
75+
76+
// ParentBasedSampler with AlwaysOnSampler root - realistic production default.
77+
this.providerParentBased = Sdk.CreateTracerProviderBuilder()
78+
.AddSource(this.sourceParentBased.Name)
79+
.SetSampler(new ParentBasedSampler(new AlwaysOnSampler()))
80+
.Build();
81+
}
82+
83+
[GlobalCleanup]
84+
public void Cleanup()
85+
{
86+
this.sourceNoAttributes?.Dispose();
87+
this.sourceWithAttributeArray?.Dispose();
88+
this.sourceWithAttributeList?.Dispose();
89+
this.sourceDrop?.Dispose();
90+
this.sourceParentBased?.Dispose();
91+
92+
this.providerNoAttributes?.Dispose();
93+
this.providerWithAttributeArray?.Dispose();
94+
this.providerWithAttributeList?.Dispose();
95+
this.providerDrop?.Dispose();
96+
this.providerParentBased?.Dispose();
97+
}
98+
99+
[Benchmark(Baseline = true)]
100+
public void NoAttributes()
101+
{
102+
using var activity = this.sourceNoAttributes!.StartActivity("Benchmark", ActivityKind.Server, this.sampledRemoteParent);
103+
}
104+
105+
[Benchmark]
106+
public void WithAttributeArray()
107+
{
108+
using var activity = this.sourceWithAttributeArray!.StartActivity("Benchmark", ActivityKind.Server, this.sampledRemoteParent);
109+
}
110+
111+
[Benchmark]
112+
public void WithAttributeList()
113+
{
114+
using var activity = this.sourceWithAttributeList!.StartActivity("Benchmark", ActivityKind.Server, this.sampledRemoteParent);
115+
}
116+
117+
[Benchmark]
118+
public void Drop()
119+
{
120+
using var activity = this.sourceDrop!.StartActivity("Benchmark", ActivityKind.Server, this.sampledRemoteParent);
121+
}
122+
123+
[Benchmark]
124+
public void ParentBasedSampled()
125+
{
126+
using var activity = this.sourceParentBased!.StartActivity("Benchmark", ActivityKind.Server, this.sampledRemoteParent);
127+
}
128+
129+
private sealed class DelegateSampler(Func<SamplingParameters, SamplingResult> sample) : Sampler
130+
{
131+
public override SamplingResult ShouldSample(in SamplingParameters samplingParameters)
132+
=> sample(samplingParameters);
133+
}
134+
}

0 commit comments

Comments
 (0)