Skip to content

Commit f5b85b7

Browse files
andrewlockbouwkast
andauthored
Avoid generating resource name that will be thrown away if it's not required (#7312)
## Summary of changes - Add `HasResourceBasedSamplingRule` to `ITraceSampler` and `ISpanSampler` - If there are no resource-based sampling rule, we can avoid calculating the resource name ## Reason for change In #6936, we fixed an issue where resource-based sampling wasn't work. This was because we can't calculate the "final" resource name for a span until after the MVC framework has run. Previously, we were delaying the resource name calculation to this point. For the fix, we calculate a _provisional_ resource name, which we subsequently change. It's sub-optimal in that - The resource name changes - If the customer _isn't_ using resource based sampling, we are allocating for ultimately no benefit This PR exposes a property which lets us check whether resource based sampling is being used. If it is, then we _must_ calculate the resource name up-front. If it's not, then we can delay creation for performance reasons. ## Implementation details - Expose `IsResourceBasedSamplingRule` on `ISamplingRule` and `ISpanSamplingRule` - Expose `HasResourceBasedSamplingRule` on `ISampler` and `ISpanSampler` - Expose `HasResourceBasedSamplingRule` on `PerTraceSettings` - Check this value prior to creating the initial resource name in `TracingHttpModule` ## Test coverage - Added unit tests for the new properties - Existing integration tests cover the implementation in IIS ## Other details This is mostly a setup for adding a fix to the ASP.NET Core provider to give the same functionality. In general, it feels a bit hacky, and it assumes that nothing _else_ (other than sampling) is dependent on the resource name. _Ideally_, we would have a system like @dougqh has discussed previously to evaluate properties lazily based on the available data in the span. But until then, this is a a stop gap which means we can avoid the perf hit for the majority of customers who _don't_ have resource-based sampling. Part of a stack - #7311 - #7312 👈 - #7316 --------- Co-authored-by: Steven Bouwkamp <[email protected]>
1 parent 98f5d1c commit f5b85b7

16 files changed

+152
-2
lines changed

tracer/src/Datadog.Trace/AspNet/TracingHttpModule.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,9 @@ private void OnBeginRequest(object sender, EventArgs eventArgs)
192192
// Attempt to set Resource Name to something that will be close to what is expected
193193
// Note: we will go and re-do it in OnEndRequest, but doing it here will allow for resource-based sampling
194194
// this likely won't be perfect - but we need something to try and allow resource-based sampling to function
195-
var resourceName = BuildResourceName(tracer, httpRequest);
195+
var resourceName = tracer.TracerManager.PerTraceSettings.HasResourceBasedSamplingRule
196+
? BuildResourceName(tracer, httpRequest)
197+
: null;
196198
scope.Span.DecorateWebServerSpan(resourceName: resourceName, httpMethod, host, url, userAgent, tags);
197199
tracer.TracerManager.SpanContextPropagator.AddHeadersToSpanAsTags(scope.Span, headers, tracer.Settings.HeaderTags, defaultTagPrefix: SpanContextPropagator.HttpRequestHeadersTagPrefix);
198200

tracer/src/Datadog.Trace/Ci/Sampling/CISampler.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ namespace Datadog.Trace.Ci.Sampling
1111
{
1212
internal class CISampler : ITraceSampler
1313
{
14+
// The Ci Sampler keeps all spans, so it doesn't depend on the resource name
15+
public bool HasResourceBasedSamplingRule => false;
16+
1417
public SamplingDecision MakeSamplingDecision(Span span)
1518
{
1619
return new SamplingDecision(SamplingPriorityValues.UserKeep, mechanism: null, rate: null, limiterRate: null);

tracer/src/Datadog.Trace/Configuration/PerTraceSettings.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ public PerTraceSettings(ITraceSampler? traceSampler, ISpanSampler? spanSampler,
2222
SpanSampler = spanSampler;
2323
ServiceNames = serviceNames;
2424
Schema = schema;
25+
HasResourceBasedSamplingRule = (traceSampler?.HasResourceBasedSamplingRule ?? false) || (spanSampler?.HasResourceBasedSamplingRule ?? false);
2526
}
2627

2728
public ITraceSampler? TraceSampler { get; }
@@ -32,6 +33,8 @@ public PerTraceSettings(ITraceSampler? traceSampler, ISpanSampler? spanSampler,
3233

3334
public NamingSchema Schema { get; }
3435

36+
public bool HasResourceBasedSamplingRule { get; }
37+
3538
internal string GetServiceName(Tracer tracer, string serviceName)
3639
{
3740
if (ServiceNames.TryGetValue(serviceName, out var name))

tracer/src/Datadog.Trace/Sampling/AgentSamplingRule.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ internal class AgentSamplingRule : ISamplingRule
2828
Datadog.Trace.Sampling.SamplingMechanism.Default :
2929
Datadog.Trace.Sampling.SamplingMechanism.AgentRate;
3030

31+
// Agent sampling rules do not depend on span resource names, only service and environment names.
32+
public bool IsResourceBasedSamplingRule => false;
33+
3134
public bool IsMatch(Span span) => true;
3235

3336
public float GetSamplingRate(Span span)

tracer/src/Datadog.Trace/Sampling/CustomSamplingRule.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ _resourceNameRegex is null &&
5050
}
5151
}
5252

53+
public bool IsResourceBasedSamplingRule => _resourceNameRegex is not null;
54+
5355
public abstract string Provenance { get; }
5456

5557
public abstract string SamplingMechanism { get; }

tracer/src/Datadog.Trace/Sampling/GlobalSamplingRateRule.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ public GlobalSamplingRateRule(float rate)
1818

1919
public string SamplingMechanism => Datadog.Trace.Sampling.SamplingMechanism.LocalTraceSamplingRule;
2020

21+
// Doesn't depend on span at all
22+
public bool IsResourceBasedSamplingRule => false;
23+
2124
public bool IsMatch(Span span) => true;
2225

2326
public float GetSamplingRate(Span span)

tracer/src/Datadog.Trace/Sampling/ISamplingRule.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ internal interface ISamplingRule
1111
{
1212
string SamplingMechanism { get; }
1313

14+
bool IsResourceBasedSamplingRule { get; }
15+
1416
bool IsMatch(Span span);
1517

1618
float GetSamplingRate(Span span);

tracer/src/Datadog.Trace/Sampling/ISpanSampler.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,14 @@ namespace Datadog.Trace.Sampling
1212
/// </summary>
1313
internal interface ISpanSampler
1414
{
15+
/// <summary>
16+
/// Gets a value indicating whether the sampler contains a sampling rule that depends on span resource names.
17+
/// </summary>
18+
/// <remarks>Can be used to perform optimizations, such as delaying the creation of a resource name for spans
19+
/// which are subsequently going to be sampled, or for which a resource name can't be accurately calculated</remarks>
20+
/// <returns><c>true</c> if one of the registered rules depends on span resource names, <c>false</c> otherwise</returns>
21+
bool HasResourceBasedSamplingRule { get; }
22+
1523
/// <summary>
1624
/// Makes a sampling decision on the given <paramref name="span"/> and adds necessary tags to the span.
1725
/// </summary>

tracer/src/Datadog.Trace/Sampling/ISpanSamplingRule.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ internal interface ISpanSamplingRule
2828
/// </value>
2929
float? MaxPerSecond { get; }
3030

31+
/// <summary>
32+
/// Gets a value indicating whether the rule depends on span resource names.
33+
/// </summary>
34+
/// <returns><c>true</c> if the rule depends on span resource names, <c>false</c> otherwise</returns>
35+
bool IsResourceBasedSamplingRule { get; }
36+
3137
/// <summary>
3238
/// Checks whether or not the <paramref name="span"/> matches the glob patterns defined by this rule.
3339
/// </summary>

tracer/src/Datadog.Trace/Sampling/ITraceSampler.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,14 @@ namespace Datadog.Trace.Sampling
1111
{
1212
internal interface ITraceSampler
1313
{
14+
/// <summary>
15+
/// Gets a value indicating whether the sampler contains a sampling rule that depends on span resource names.
16+
/// </summary>
17+
/// <remarks>Can be used to perform optimizations, such as delaying the creation of a resource name for spans
18+
/// which are subsequently going to be sampled, or for which a resource name can't be accurately calculated</remarks>
19+
/// <returns><c>true</c> if one of the registered rules depends on span resource names, <c>false</c> otherwise</returns>
20+
bool HasResourceBasedSamplingRule { get; }
21+
1422
void SetDefaultSampleRates(IReadOnlyDictionary<string, float> sampleRates);
1523

1624
SamplingDecision MakeSamplingDecision(Span span);

0 commit comments

Comments
 (0)