Skip to content

Commit 7955e93

Browse files
authored
Fix flakiness in the TraceAnnotation tests (#7554)
## Summary of changes Fixes flakiness in the `TraceAnnotation` tests ## Reason for change #7287 incidentally introduced flakiness in the test when the profiler is not available (i.e. only on Windows). We temporarily disabled the test in #7551 and #7552. This reinstates the test, and removes the flake. ## Implementation details The important thing is that we do a "real" instrumentation in the app, to insure that we send instrumentation telemetry. Without this, there's a race condition between us instrumenting our "own" `HttpClient` usages and the app ending. In more detail, we see this flake because: - The integration telemetry is only sent when something is instrumented (or subsequently errors) - The delay in profiling introduced in #7287 causes a delay in sending the calltarget definitions - We will address that delay in a separate PR - The telemetry thread runs in the background, and is already started before we P/Invoke into the profiler - The delay causes us to not instrument the HttpClient calls that the background telemetry thread makes initially - if we skip the P/Invoke, then they are instrumented at this point - The app shuts down, which causes another telemetry flush - At this point the `HttpClient` calls are instrumented, and the instrumentation details are collected, but now we're shutting down, so this data is never sent. By forcing an instrumentation in the app, we bypass the race condition entirely. ## Test coverage Excluded the span from the tests so it's effectively the same. We don't care about the contents of that span (or whether the request passes or fails) - we just want to make sure we have instrumentation. ## Other details
1 parent c6bd674 commit 7955e93

File tree

2 files changed

+22
-6
lines changed

2 files changed

+22
-6
lines changed

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ protected TraceAnnotationsTests(string sampleAppName, bool enableTelemetry, ITes
7878
[SkippableFact]
7979
public async Task SubmitTraces()
8080
{
81-
const int expectedSpanCount = 50;
81+
const int expectedSpanCount = 51;
8282
var ddTraceMethodsString = string.Empty;
8383

8484
foreach (var type in TestTypes)
@@ -98,7 +98,11 @@ public async Task SubmitTraces()
9898
{
9999
var spans = await agent.WaitForSpansAsync(expectedSpanCount);
100100

101-
var orderedSpans = spans.OrderBy(s => s.Start).ToList();
101+
// Exclude the http span
102+
var orderedSpans = spans
103+
.Where(s => !s.Tags.ContainsKey("http-client-handler-type"))
104+
.OrderBy(s => s.Start)
105+
.ToList();
102106
var rootSpan = orderedSpans.First();
103107
var remainingSpans = orderedSpans.Skip(1).ToList();
104108

@@ -181,12 +185,10 @@ static Dictionary<string, double> ApmNumericTagsScrubber(MockSpan target, Dictio
181185
[Trait("RunOnWindows", "True")]
182186
public async Task IntegrationDisabled()
183187
{
184-
#if NET6_0
185-
Skip.If(true, "Starting failing after https://github.com/DataDog/dd-trace-dotnet/pull/7287");
186-
#endif
187188
// Don't bother with telemetry in version mismatch scenarios because older versions may only support V1 telemetry
188189
// which we no longer support in our mock telemetry agent
189190
// FIXME: Could be fixed with an upgrade to the NuGet package (after .NET 8?)
191+
EnvironmentHelper.DebugModeEnabled = true;
190192
MockTelemetryAgent telemetry = _enableTelemetry ? this.ConfigureTelemetry() : null;
191193
SetEnvironmentVariable("DD_TRACE_METHODS", string.Empty);
192194
SetEnvironmentVariable("DD_TRACE_ANNOTATIONS_ENABLED", "false");
@@ -195,7 +197,8 @@ public async Task IntegrationDisabled()
195197
using var process = await RunSampleAndWaitForExit(agent);
196198
var spans = agent.Spans;
197199

198-
Assert.Empty(spans);
200+
// Only the http client spans should be there
201+
spans.Should().OnlyContain(s => s.Tags.ContainsKey("http-client-handler-type"));
199202
if (telemetry != null)
200203
{
201204
await telemetry.AssertIntegrationAsync(IntegrationId.TraceAnnotations, enabled: false, autoEnabled: false);

tracer/test/test-applications/integrations/Samples.TraceAnnotations/ProgramHelpers.cs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,19 @@ public static async Task RunTestsAsync()
112112
HttpRequestMessage message = new HttpRequestMessage();
113113
message.Method = HttpMethod.Get;
114114

115+
try
116+
{
117+
// trigger a "standard" instrumented integration to ensure that integration telemetry is always sent,
118+
// even when trace annotations are disabled
119+
message.RequestUri = new Uri("http://www.google.com");
120+
var httpClient = new HttpClient();
121+
using var response = await httpClient.SendAsync(message);
122+
}
123+
catch
124+
{
125+
// Ignore
126+
}
127+
115128
// Delay
116129
await Task.Delay(500);
117130

0 commit comments

Comments
 (0)