Skip to content

Commit 0158624

Browse files
chojomokbouwkastzacharycmontoya
authored
[Tracing] Fix Hangfire baggage accumulation (#8900)
## Summary of changes Fixes baggage accumulating across sequential Hangfire jobs processed by the same worker. `DatadogHangfireServerFilter.OnPerforming` now replaces `Baggage.Current` with the job's own extracted baggage instead of merging into it, saving whatever baggage was ambient beforehand; `OnPerformed` restores that saved baggage after the job completes. ## Reason for change `Baggage.Current` is a mutable reference type stored in an `AsyncLocal`. Hangfire workers process jobs sequentially on one persistent thread/`ExecutionContext` (unlike, e.g., one `ExecutionContext` per HTTP request), so that `AsyncLocal` is never naturally reset between jobs. `OnPerforming` was merging each job's extracted baggage into the existing `Baggage.Current` instance rather than replacing it, and `OnPerformed` never reset it, so every job's baggage accumulated into whatever the previous job on that worker had left behind. Over time this produces oversized `baggage` headers on downstream HTTP calls, which can be rejected by services enforcing header size limits. ## Implementation details In `DatadogHangfireServerFilter.cs`: - `OnPerforming`: save the current `Baggage.Current` into the job's `Items` bag, then replace it with `propagationContext.Baggage ?? new Baggage()` (the job's own extracted baggage) instead of merging into the ambient instance. - `OnPerformed`: restore the baggage saved in `OnPerforming` (in a `finally` block, so it runs even if scope disposal throws), since the worker's `ExecutionContext` is persistent across jobs. This mirrors the save/replace/restore pattern used by OpenTelemetry's own Hangfire instrumentation. ## Test coverage Added `HangfireTests.BaggageDoesNotAccumulateAcrossSequentialJobs`, which runs two sequential jobs on a single-worker Hangfire server, each enqueued from its own isolated producer context with distinct baggage, and asserts that the second job's `Baggage.Current` contains only its own entry (not the first job's). The test also asserts both jobs ran on the same worker thread, confirming the single-worker/persistent-`ExecutionContext` precondition that makes the bug reproducible. ## Other details Fixes issue [#8895](#8895) --------- Co-authored-by: Steven Bouwkamp <steven.bouwkamp@datadoghq.com> Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com>
1 parent ef7e8d0 commit 0158624

6 files changed

Lines changed: 111 additions & 11 deletions

File tree

tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Hangfire/DatadogHangfireServerFilter.cs

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,14 @@ public void OnPerforming(object context)
2929
return;
3030
}
3131

32+
// Hangfire workers reuse one ExecutionContext across sequential jobs, so save/restore
33+
// baggage around each job instead of leaking it into the next one (see OnPerformed).
34+
performingContext.Items[HangfireConstants.DatadogBaggageKey] = Baggage.Current;
35+
3236
var spanContextData = performingContext.GetJobParameter<Dictionary<string, string?>?>(HangfireConstants.DatadogContextKey);
33-
PropagationContext propagationContext = Tracer.Instance.TracerManager.SpanContextPropagator.Extract(spanContextData).MergeBaggageInto(Baggage.Current);
37+
var propagationContext = Tracer.Instance.TracerManager.SpanContextPropagator.Extract(spanContextData);
38+
Baggage.Current = propagationContext.Baggage ?? new Baggage();
39+
3440
var parentContext = propagationContext.SpanContext;
3541
Scope? scope = HangfireCommon.CreateScope(Tracer.Instance, new HangfireTags(), performingContext, parentContext);
3642
((Dictionary<string, object?>)performingContext.Items).Add(HangfireConstants.DatadogScopeKey, scope);
@@ -45,19 +51,29 @@ public void OnPerformed(object context)
4551
{
4652
if (context.TryDuckCast<IPerformedContextProxy>(out var performedContext))
4753
{
48-
if (performedContext.Items.TryGetValue(HangfireConstants.DatadogScopeKey, out var scope))
54+
try
4955
{
50-
if (scope is not Scope typedScope)
56+
if (performedContext.Items.TryGetValue(HangfireConstants.DatadogScopeKey, out var scope))
5157
{
52-
return;
53-
}
58+
if (scope is not Scope typedScope)
59+
{
60+
return;
61+
}
62+
63+
if (performedContext.Exception is not null)
64+
{
65+
HangfireCommon.SetStatusAndRecordException(typedScope, performedContext.Exception);
66+
}
5467

55-
if (performedContext.Exception is not null)
68+
typedScope.Dispose();
69+
}
70+
}
71+
finally
72+
{
73+
if (performedContext.Items.TryGetValue(HangfireConstants.DatadogBaggageKey, out var previousBaggage))
5674
{
57-
HangfireCommon.SetStatusAndRecordException(typedScope, performedContext.Exception);
75+
Baggage.Current = previousBaggage is Baggage baggage ? baggage : new Baggage();
5876
}
59-
60-
typedScope.Dispose();
6177
}
6278
}
6379
}

tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Hangfire/HangfireConstants.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ internal static class HangfireConstants
1414
internal const string OnPerformOperation = "hangfire.perform";
1515
internal const string DatadogScopeKey = "datadog_scope_key";
1616
internal const string DatadogContextKey = "datadog_context_key";
17+
internal const string DatadogBaggageKey = "datadog_baggage_key";
1718
internal const string JobIdTag = "job.id";
1819
internal const string JobCreatedAtTag = "job.createdat";
1920
internal const string ResourceNamePrefix = "job ";

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/HangfireTests.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,25 @@ public HangfireTests(ITestOutputHelper output)
2929

3030
public override Result ValidateIntegrationSpan(MockSpan span, string metadataSchemaVersion) => span.IsHangfire(metadataSchemaVersion);
3131

32+
[SkippableFact]
33+
[Trait("Category", "EndToEnd")]
34+
[Trait("RunOnWindows", "True")]
35+
public async Task BaggageDoesNotAccumulateAcrossSequentialJobs()
36+
{
37+
using var agent = EnvironmentHelper.GetMockAgent();
38+
using var process = await RunSampleAndWaitForExit(agent, arguments: "baggage");
39+
40+
var workerThreadIds = Regex.Matches(process.StandardOutput, @"Worker thread for job-(?:one|two): (\d+)")
41+
.Cast<Match>()
42+
.Select(x => x.Groups[1].Value)
43+
.ToArray();
44+
45+
workerThreadIds.Should().HaveCount(2);
46+
workerThreadIds.Distinct().Should().ContainSingle();
47+
process.StandardOutput.Should().Contain("Baggage for job-one: [job-one=one]");
48+
process.StandardOutput.Should().Contain("Baggage for job-two: [job-two=two]");
49+
}
50+
3251
[SkippableFact]
3352
[Trait("Category", "EndToEnd")]
3453
[Trait("RunOnWindows", "True")]

tracer/test/test-applications/integrations/Samples.Hangfire/Jobs/TestJob.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Linq;
23
using System.Threading.Tasks;
34

45
namespace Samples.Hangfire.Jobs;
@@ -11,6 +12,16 @@ public Task Execute()
1112
return Task.CompletedTask;
1213
}
1314

15+
public void PrintBaggage(string jobName)
16+
{
17+
var baggage = OpenTelemetry.Baggage.Current.GetBaggage()
18+
.OrderBy(x => x.Key)
19+
.Select(x => $"{x.Key}={x.Value}");
20+
21+
Console.WriteLine($"Worker thread for {jobName}: {Environment.CurrentManagedThreadId}");
22+
Console.WriteLine($"Baggage for {jobName}: [{string.Join(", ", baggage)}]");
23+
}
24+
1425
public Task ThrowException()
1526
{
1627
Console.WriteLine("TestJob.ThrowException running...");

tracer/test/test-applications/integrations/Samples.Hangfire/Program.cs

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ public static async Task Main(string[] args)
3030
GlobalJobFilters.Filters.Add(new JobCompletionFilter());
3131

3232
Console.WriteLine("Starting Hangfire server...");
33-
using var server = new BackgroundJobServer();
33+
var isBaggageScenario = args.Length > 0 && args[0] == "baggage";
34+
using var server = CreateServer(isBaggageScenario);
3435

3536
try
3637
{
@@ -43,6 +44,13 @@ public static async Task Main(string[] args)
4344
kind: ActivityKind.Internal,
4445
tags: tags
4546
);
47+
48+
if (isBaggageScenario)
49+
{
50+
await Should_Not_Accumulate_Baggage_Across_Jobs();
51+
return;
52+
}
53+
4654
// run tests
4755
await Should_Create_Span();
4856
await Should_Create_Span_With_Status_Error_When_Job_Failed();
@@ -54,6 +62,50 @@ public static async Task Main(string[] args)
5462
}
5563
}
5664

65+
private static BackgroundJobServer CreateServer(bool suppressExecutionContext)
66+
{
67+
if (!suppressExecutionContext)
68+
{
69+
return new BackgroundJobServer();
70+
}
71+
72+
// Keep the worker's mutable AsyncLocal baggage instance isolated from the in-process producers.
73+
using (ExecutionContext.SuppressFlow())
74+
{
75+
return new BackgroundJobServer(new BackgroundJobServerOptions { WorkerCount = 1 });
76+
}
77+
}
78+
79+
private static async Task Should_Not_Accumulate_Baggage_Across_Jobs()
80+
{
81+
await Run_Baggage_Job("job-one", "one");
82+
await Run_Baggage_Job("job-two", "two");
83+
}
84+
85+
private static async Task Run_Baggage_Job(string baggageKey, string baggageValue)
86+
{
87+
// Model an independent producer request, with its own execution context and baggage instance.
88+
Task<string> enqueueTask;
89+
using (ExecutionContext.SuppressFlow())
90+
{
91+
enqueueTask = Task.Run(() =>
92+
{
93+
using var activity = AdditionalActivitySource.StartActivity($"Enqueue {baggageKey}");
94+
OpenTelemetry.Baggage.Current = OpenTelemetry.Baggage.Create(new Dictionary<string, string>
95+
{
96+
[baggageKey] = baggageValue,
97+
});
98+
99+
return BackgroundJob.Enqueue<TestJob>(x => x.PrintBaggage(baggageKey));
100+
});
101+
}
102+
103+
var jobId = await enqueueTask;
104+
var result = await JobCompletion.Register(jobId, new CancellationTokenSource(TimeSpan.FromSeconds(30)).Token);
105+
106+
Console.WriteLine($"Job {result.JobId} completed successfully = {result.Succeeded}");
107+
}
108+
57109
private static async Task Should_Create_Span()
58110
{
59111
var jobId = BackgroundJob.Enqueue<TestJob>(x => x.Execute());

tracer/test/test-applications/integrations/Samples.Hangfire/Samples.Hangfire.csproj

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
<ItemGroup>
1313
<PackageReference Include="Hangfire" Version="$(ApiVersion)" />
1414
<PackageReference Include="Hangfire.MemoryStorage" Version="1.4.0" />
15+
<PackageReference Include="OpenTelemetry.Api" Version="1.10.0" />
1516
<PackageReference Include="System.Diagnostics.DiagnosticSource" Version="9.0.8" />
1617
</ItemGroup>
1718

18-
</Project>
19+
</Project>

0 commit comments

Comments
 (0)