Skip to content

Commit 07039f0

Browse files
authored
Set DD_TRACE_OTEL_ENABLED=true by default for all integration tests (#8370)
## Summary of changes - Set `DD_TRACE_OTEL_ENABLED=true` by default in integration tests - Add additional excludes for activity handlers with equivalent custom instrumentation ## Reason for change We had an escalation recently, which highlighted that we were missing some entries in the `IgnoreActivityHandler`. To avoid hitting similar issues in the future, we can set `DD_TRACE_OTEL_ENABLED=true` by default, so we know as soon as a new `ActivitySource` lights up. ## Implementation details A _lot_ of trial and error here, mostly setting `DD_TRACE_OTEL_ENABLED=true` in the `TracingIntegrationTest` base class and seeing what breaks 😅 That pointed to a variety of extra handlers and differences in behaviour: - `Couchbase.DotnetSdk.OpenTelemetryRequestTracer`, I don't think we actually need this one strictly, but it showed up while I was trying to fix persistent issues with Couchbase3, so I think it makes sense to exclude it - The _real_ issue I had is that some early versions of Couchbase (3.0.0 - 3.2.0) create activities using `new Activity()`, which means there's _no_ `ActivitySource` associated, and so we have no way to filter them 💀 - Rather than fight with that, and because those versions are deprecated, just disabled OTel integration for these specific tested versions - `connector-net` - this is the ActivitySource for `MySql.Data` (we already excluded the one for `MySqlConnector`) - `RabbitMQ.Client.*` - these were the ones that caused the original issue, and were breaking DSM - `Experimental.System.Net.Security` - this one came in .NET 9, and was causing extra spans in gRPC and Yarp tests - `Grpc.Net.Client` - The gRPC client has had an `ActivitySource` [for a long time](grpc/grpc-dotnet#2244) 😅 - `Yarp.ReverseProxy` - ...[as has Yarp](dotnet/yarp#2098) In addition, there were some extra "fixes" to the tests required: - For the `HttpMessageHandler` tests, where the integration is disabled, we were previously verifying that W3C headers weren't injected, but they _will_ be if OTel is enabled, so just relaxed the restrictions there. - For `OpenTelemetrySdkTests.SubmitsOtlpLogs`, the data changes depending on whether otel is enabled or not, so just reset it to the default for simplicity rather than wrestle with it (I spent some time ping-ponging snapshots before I gave up 😅) - Updated the gRPC snapshots to add `grpc.method` and `grpc.status_code` which are now added to the aspnetcore spans ## Test coverage Hopefully self explanatory 😅 ## Other details Fixes https://datadoghq.atlassian.net/browse/DSMS-138 Technically, this could be a breaking change for some people, so maybe we should revisit making the ignore activity handler configurable?
1 parent de2deda commit 07039f0

20 files changed

+156
-33
lines changed

tracer/src/Datadog.Trace/Activity/Handlers/IgnoreActivityHandler.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,24 @@ internal sealed class IgnoreActivityHandler : IActivityHandler
1919
private static readonly string[] SourcesNames =
2020
{
2121
"Couchbase.DotnetSdk.RequestTracer",
22+
"Couchbase.DotnetSdk.OpenTelemetryRequestTracer",
23+
"Grpc.Net.Client",
2224
"HttpHandlerDiagnosticListener",
2325
"Microsoft.AspNetCore",
2426
"Microsoft.EntityFrameworkCore",
2527
"MongoDB.Driver",
2628
"MySqlConnector",
29+
"connector-net",
2730
"Npgsql",
31+
"RabbitMQ.Client.Publisher",
32+
"RabbitMQ.Client.Subscriber",
2833
"System.Net.Http.Desktop",
2934
"SqlClientDiagnosticListener",
3035
"Experimental.System.Net.NameResolution",
3136
"Experimental.System.Net.Http.Connections",
37+
"Experimental.System.Net.Security",
3238
"Experimental.System.Net.Sockets",
39+
"Yarp.ReverseProxy",
3340
};
3441

3542
public static bool ShouldIgnoreByOperationName(string? operationName)

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/AspNetCore/AspNetCoreMvcTestBase.cs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,6 @@ protected AspNetCoreMvcTestBase(string sampleName, AspNetCoreTestFixture fixture
6161
SetEnvironmentVariable(ConfigurationKeys.FeatureFlags.RouteTemplateResourceNamesEnabled, (flags == AspNetCoreFeatureFlags.RouteTemplateResourceNames).ToString());
6262
SetEnvironmentVariable(ConfigurationKeys.FeatureFlags.SingleSpanAspNetCoreEnabled, (flags == AspNetCoreFeatureFlags.SingleSpan).ToString());
6363

64-
// Enable OpenTelemetry interop to test that the OTel Baggage API integration works correctly.
65-
SetEnvironmentVariable("DD_TRACE_OTEL_ENABLED", "true");
66-
6764
Fixture = fixture;
6865
Fixture.SetOutput(output);
6966
}

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/Azure/AzureServiceBusTests.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ public static IEnumerable<object[]> GetEnabledConfig()
4848
public async Task SubmitsTraces(string packageVersion, string metadataSchemaVersion)
4949
{
5050
SetEnvironmentVariable("DD_TRACE_SPAN_ATTRIBUTE_SCHEMA", metadataSchemaVersion);
51-
SetEnvironmentVariable("DD_TRACE_OTEL_ENABLED", "true");
5251

5352
// If you want to use a custom connection string, set it here
5453
// SetEnvironmentVariable("ASB_CONNECTION_STRING", null);

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/Azure/DataStreamsMonitoringAzureServiceBusTests.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ public static IEnumerable<object[]> GetPackageVersions()
3838
public async Task HandleProduceAndConsume(string packageVersion)
3939
{
4040
SetEnvironmentVariable(ConfigurationKeys.DataStreamsMonitoring.Enabled, "1");
41-
SetEnvironmentVariable("DD_TRACE_OTEL_ENABLED", "true");
4241

4342
// If you want to use a custom connection string, set it here
4443
// SetEnvironmentVariable("ASB_CONNECTION_STRING", null);
@@ -67,7 +66,6 @@ await Verifier.Verify(PayloadsToPoints(agent.DataStreams), settings)
6766
public async Task ValidateSpanTags(string packageVersion)
6867
{
6968
SetEnvironmentVariable(ConfigurationKeys.DataStreamsMonitoring.Enabled, "1");
70-
SetEnvironmentVariable("DD_TRACE_OTEL_ENABLED", "true");
7169

7270
// If you want to use a custom connection string, set it here
7371
// SetEnvironmentVariable("ASB_CONNECTION_STRING", null);

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,15 @@ public static IEnumerable<object[]> GetEnabledConfig()
4242
public async Task SubmitTraces(string packageVersion, string metadataSchemaVersion)
4343
{
4444
SetEnvironmentVariable("DD_TRACE_SPAN_ATTRIBUTE_SCHEMA", metadataSchemaVersion);
45+
46+
if (GetPackageVersion(packageVersion) < new Version(3, 2, 0))
47+
{
48+
// Early versions of tracing just called new Activity(), which results in activities with no source, so
49+
// our IgnoreActivityHandler fails to detect it: https://github.com/couchbase/couchbase-net-client/blob/3.1.0/src/Couchbase/Core/Diagnostics/Tracing/ActivityRequestTracer.cs#L24
50+
// for simplicity, just disable OTEL integration for these cases
51+
SetEnvironmentVariable("DD_TRACE_OTEL_ENABLED", "false");
52+
}
53+
4554
var isExternalSpan = metadataSchemaVersion == "v0";
4655
var clientSpanServiceName = isExternalSpan ? $"{EnvironmentHelper.FullSampleName}-couchbase" : EnvironmentHelper.FullSampleName;
4756

@@ -50,8 +59,8 @@ public async Task SubmitTraces(string packageVersion, string metadataSchemaVersi
5059
using (await RunSampleAndWaitForExit(agent, packageVersion: packageVersion))
5160
{
5261
var spans = (await agent.WaitForSpansAsync(10, 500))
53-
.Where(s => s.Type == "db")
54-
.ToList();
62+
.Where(s => s.Type == "db")
63+
.ToList();
5564

5665
ValidateIntegrationSpans(spans, metadataSchemaVersion, expectedServiceName: clientSpanServiceName, isExternalSpan);
5766

@@ -91,7 +100,7 @@ await VerifyHelper.VerifySpans(spans, settings)
91100

92101
private static string GetVersionSuffix(string packageVersion)
93102
{
94-
var version = new Version(string.IsNullOrEmpty(packageVersion) ? "3.4.1" : packageVersion); // default version in csproj
103+
var version = GetPackageVersion(packageVersion);
95104
if (version < new Version("3.1.2"))
96105
{
97106
return "_3_0";
@@ -119,5 +128,8 @@ private static string GetVersionSuffix(string packageVersion)
119128

120129
return string.Empty;
121130
}
131+
132+
private static Version GetPackageVersion(string packageVersion)
133+
=> new(string.IsNullOrEmpty(packageVersion) ? "3.4.1" : packageVersion); // default version in csproj
122134
}
123135
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ public class GrpcLegacyTests : GrpcTestsBase
2929
public GrpcLegacyTests(ITestOutputHelper output)
3030
: base("GrpcLegacy", output, usesAspNetCore: false)
3131
{
32-
SetEnvironmentVariable("DD_TRACE_OTEL_ENABLED", "true");
3332
// the sample uses protobuf, but we are only interested in testing the grpc instrumentation,
3433
// so we disable the proto one which would add unexpected tags to the spans.
3534
SetEnvironmentVariable("DD_TRACE_PROTOBUF_ENABLED", "false");
@@ -373,6 +372,7 @@ static void FixVerySlowServerSpans(IImmutableList<MockSpan> spans, HttpClientInt
373372
{
374373
// May be missing in some cases
375374
span.Tags["http.status_code"] = "200";
375+
span.Tags["grpc.status_code"] = "4";
376376
}
377377

378378
// there is a race between the server cancelling a deadline and the client cancelling it

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,6 @@ public HangfireTests(ITestOutputHelper output)
3434
[Trait("RunOnWindows", "True")]
3535
public async Task SubmitsTraces()
3636
{
37-
// needed for context propagation test
38-
SetEnvironmentVariable("DD_TRACE_OTEL_ENABLED", "true");
39-
4037
using (var telemetry = this.ConfigureTelemetry())
4138
using (var agent = EnvironmentHelper.GetMockAgent())
4239
using (await RunSampleAndWaitForExit(agent))

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,11 @@ public async Task TracingDisabled_DoesNotSubmitsTraces(
212212
headers.Should().NotContainKey(HttpHeaderNames.PropagatedTags);
213213

214214
// W3C trace context headers
215-
headers.Should().NotContainKey(W3CTraceContextPropagator.TraceParentHeaderName);
216-
headers.Should().NotContainKey(W3CTraceContextPropagator.TraceStateHeaderName);
215+
// These are still injected when we have otel enabled, but they're not added by us
216+
// TODO: Verify whether the injected values are actually _correct_ - they _should_ be,
217+
// because our IgnoreActivityHandler should fix them
218+
// headers.Should().NotContainKey(W3CTraceContextPropagator.TraceParentHeaderName);
219+
// headers.Should().NotContainKey(W3CTraceContextPropagator.TraceStateHeaderName);
217220

218221
// B3 trace context headers
219222
headers.Should().NotContainKey(B3SingleHeaderContextPropagator.B3);

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ public static IEnumerable<object[]> GetEnabledConfig()
4848
public async Task SubmitsTraces(string packageVersion, string metadataSchemaVersion)
4949
{
5050
SetEnvironmentVariable("DD_TRACE_SPAN_ATTRIBUTE_SCHEMA", metadataSchemaVersion);
51-
SetEnvironmentVariable("DD_TRACE_OTEL_ENABLED", "true");
5251

5352
var isExternalSpan = metadataSchemaVersion == "v0";
5453
var clientSpanServiceName = isExternalSpan ? $"{EnvironmentHelper.FullSampleName}-mongodb" : EnvironmentHelper.FullSampleName;

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ public NetActivitySdkTests(ITestOutputHelper output)
5959
[Trait("RunOnWindows", "True")]
6060
public async Task SubmitsTraces()
6161
{
62-
SetEnvironmentVariable("DD_TRACE_OTEL_ENABLED", "true");
6362
SetEnvironmentVariable("DD_TRACE_DISABLED_ACTIVITY_SOURCES", "Disabled.By.ExactMatch,*.By.Glob*");
6463

6564
using (var telemetry = this.ConfigureTelemetry())

0 commit comments

Comments
 (0)