Skip to content

Commit bc123ee

Browse files
authored
Fix race condition in Client Side Stats enablement (#8509)
## Summary of changes Fixes a race condition in client-side-stats enablement ## Reason for change A unit test was flaking, where there was a delay in obfuscation, so we had 2 buckets - one with obfuscated resource name, and one without. ## Implementation details Move the setting of `CanComputeStats = true` to the _end_ of the method, _after_ setting peer tags and obfuscation, so that when it switches `false` -> `true`, it already has the full config. > [!WARNING] > There _is_ still a race condition when config is _updated_ for an _already enabled_ stats config. In that case the obfuscation/peer tags will be briefly out of sync theoretically. But I don't think that's a big issue, and would only occur if the agent changes during application execution, which is rare ## Test coverage Covered by the unit tests already (they shouldn't flake now). ## Other details
1 parent 9bc311d commit bc123ee

1 file changed

Lines changed: 23 additions & 17 deletions

File tree

tracer/src/Datadog.Trace/Agent/StatsAggregator.cs

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ internal sealed class StatsAggregator : IStatsAggregator
5757
private int _tracerObfuscationVersion;
5858
private TraceFilter _traceFilter;
5959
private List<PeerTagKey> _peerTagKeys = [];
60+
private int _computeStatsState;
6061

6162
internal StatsAggregator(IApi api, TracerSettings settings, IDiscoveryService discoveryService, bool isOtlp)
6263
{
@@ -113,7 +114,12 @@ internal StatsAggregator(IApi api, TracerSettings settings, IDiscoveryService di
113114
/// </summary>
114115
internal StatsBuffer CurrentBuffer => _buffers[_currentBuffer];
115116

116-
public bool? CanComputeStats { get; private set; }
117+
public bool? CanComputeStats
118+
{
119+
// convert between -1 = false, 0 = null, 1 = true because we can't use volatile with a bool?
120+
get => Volatile.Read(ref _computeStatsState) switch { 1 => true, -1 => false, _ => null, };
121+
private set => Volatile.Write(ref _computeStatsState, value switch { true => 1, false => -1, _ => 0, });
122+
}
117123

118124
public static IStatsAggregator Create(IApi api, TracerSettings settings, IDiscoveryService discoveryService, bool isOtlp)
119125
{
@@ -563,20 +569,22 @@ private void AddToBuffer(Span span)
563569

564570
private void HandleConfigUpdate(AgentConfiguration config)
565571
{
566-
CanComputeStats = !string.IsNullOrWhiteSpace(config.StatsEndpoint)
567-
&& config.ClientDropP0s;
572+
var shouldCompute = !string.IsNullOrWhiteSpace(config.StatsEndpoint)
573+
&& config.ClientDropP0s;
568574

569-
if (CanComputeStats.Value)
570-
{
571-
Log.Debug("Stats computation enabled.");
572-
}
573-
else
575+
if (!shouldCompute)
574576
{
575577
Log.Warning("Stats computation disabled because the detected agent does not support this feature.");
576-
// early return, because there's no point doing all the extra work if stats isn't enabled anyway
578+
// No point setting up the rest of the config if stats isn't enabled anyway
579+
CanComputeStats = false;
577580
return;
578581
}
579582

583+
// Publish all config-derived state BEFORE CanComputeStats becomes true, so that any
584+
// observer that sees CanComputeStats == true also sees the consistent config. Each of
585+
// the writes below already uses release semantics (Interlocked / Volatile.Write); the
586+
// final Volatile.Write to _tracerObfuscationVersion acts as a release fence for the
587+
// plain store to CanComputeStats that follows.
580588
if (config.PeerTags is { Count: > 0 })
581589
{
582590
// Sort, deduplicate, and pre-compute the UTF-8 key prefixes so that
@@ -595,19 +603,17 @@ private void HandleConfigUpdate(AgentConfiguration config)
595603
}
596604

597605
// Update trace filter from agent configuration
598-
if (config.TraceFilterConfig.HasFilters)
599-
{
600-
Volatile.Write(ref _traceFilter, new TraceFilter(config.TraceFilterConfig));
601-
}
602-
else
603-
{
604-
Volatile.Write(ref _traceFilter, null);
605-
}
606+
Volatile.Write(
607+
ref _traceFilter,
608+
config.TraceFilterConfig.HasFilters ? new TraceFilter(config.TraceFilterConfig) : null);
606609

607610
// Tracer obfuscation version is 1. If the agent's version is > 0 and <= ours, the tracer obfuscates.
608611
const int tracerObfuscationVersion = 1;
609612
var agentVersion = config.ObfuscationVersion;
610613
Volatile.Write(ref _tracerObfuscationVersion, agentVersion > 0 && agentVersion <= tracerObfuscationVersion ? tracerObfuscationVersion : 0);
614+
615+
CanComputeStats = true;
616+
Log.Debug("Stats computation enabled.");
611617
}
612618

613619
internal readonly struct PeerTagKey(string name)

0 commit comments

Comments
 (0)