From 69b6163fffe70348b69eb839d7ca566126d6294e Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Mon, 16 Sep 2024 11:40:06 -0700 Subject: [PATCH 01/38] Extract Upper64 bit trace ID from extension response --- .../AWS/Lambda/LambdaCommon.cs | 44 +++++++++++++++++-- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs index 24f5c9bb23a0..afc092a7afd5 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs @@ -21,7 +21,7 @@ internal abstract class LambdaCommon private const double ServerlessMaxWaitingFlushTime = 3; private const string LogLevelEnvName = "DD_LOG_LEVEL"; - internal static Scope CreatePlaceholderScope(Tracer tracer, string traceId, string samplingPriority) + internal static Scope CreatePlaceholderScope(Tracer tracer, string traceId, ulong traceIdUpper64, string samplingPriority) { Span span; @@ -32,8 +32,17 @@ internal static Scope CreatePlaceholderScope(Tracer tracer, string traceId, stri } else { - Log($"creating the placeholder traceId = {traceId}"); - span = tracer.StartSpan(PlaceholderOperationName, tags: null, serviceName: PlaceholderServiceName, traceId: (TraceId)Convert.ToUInt64(traceId), addToTraceContext: false); + if (traceIdUpper64 == 0) + { + Log($"creating the placeholder traceId = {traceId}"); + span = tracer.StartSpan(PlaceholderOperationName, tags: null, serviceName: PlaceholderServiceName, traceId: (TraceId)Convert.ToUInt64(traceId), addToTraceContext: false); + } + else + { + var traceIdLower64 = Convert.ToUInt64(traceId); + Log($"creating the placeholder traceId = {traceIdUpper64}{traceIdLower64}"); + span = tracer.StartSpan(PlaceholderOperationName, tags: null, serviceName: PlaceholderServiceName, traceId: new TraceId(traceIdUpper64, traceIdLower64), addToTraceContext: false); + } } if (samplingPriority == null) @@ -58,15 +67,42 @@ internal static Scope SendStartInvocation(ILambdaExtensionRequest requestBuilder WriteRequestHeaders(request, context); var response = (HttpWebResponse)request.GetResponse(); var traceId = response.Headers.Get(HttpHeaderNames.TraceId); + var traceIdUpper64 = GetTraceIdUpper64(response.Headers.Get(HttpHeaderNames.PropagatedTags)); var samplingPriority = response.Headers.Get(HttpHeaderNames.SamplingPriority); if (ValidateOkStatus(response)) { - return CreatePlaceholderScope(Tracer.Instance, traceId, samplingPriority); + return CreatePlaceholderScope(Tracer.Instance, traceId, traceIdUpper64, samplingPriority); } return null; } + /// + /// GetTraceIdUpper64 searches the x-datadog-tags tags for the upper 64 bits of the trace id. + /// Per the 128 bit tracing RFC, the upper 64 bits are stored in the _dd.p.tid tag as a hex string. + /// + /// The propagated tags from the x-datadog-tags header + /// the upper 64 bits of the traceId as a ulong + internal static ulong GetTraceIdUpper64(string ddTags) + { + if (ddTags == null) + { + return 0; + } + + var tags = ddTags.Split(','); + foreach (var tag in tags) + { + var keyValue = tag.Trim().Split('='); + if (keyValue.Length == 2 && keyValue[0] == "_dd.p.tid") + { + return Convert.ToUInt64(keyValue[1], 16); + } + } + + return 0; + } + internal static void SendEndInvocation(ILambdaExtensionRequest requestBuilder, Scope scope, bool isError, string data) { var request = requestBuilder.GetEndInvocationRequest(scope, isError); From 42c242a5c5255cddda9e2674fdbe2ab68b6ee2f3 Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Tue, 24 Sep 2024 10:04:35 -0700 Subject: [PATCH 02/38] Use SpanContextPropagator.Instance.Extract() --- .../AWS/Lambda/LambdaCommon.cs | 43 +++++--------- .../Datadog.Trace.Tests/LambdaCommonTests.cs | 57 ++++++++++++------- .../LambdaRequestBuilderTests.cs | 17 +++++- 3 files changed, 65 insertions(+), 52 deletions(-) diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs index afc092a7afd5..b4d29bd43917 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs @@ -5,9 +5,12 @@ #if NET6_0_OR_GREATER using System; using System.Collections.Generic; +using System.Linq; using System.Net; using System.Text; using System.Threading.Tasks; +using Datadog.Trace.Headers; +using Datadog.Trace.Propagators; using Datadog.Trace.Telemetry; using Datadog.Trace.Telemetry.Metrics; using Datadog.Trace.Util; @@ -66,41 +69,23 @@ internal static Scope SendStartInvocation(ILambdaExtensionRequest requestBuilder WriteRequestPayload(request, data); WriteRequestHeaders(request, context); var response = (HttpWebResponse)request.GetResponse(); - var traceId = response.Headers.Get(HttpHeaderNames.TraceId); - var traceIdUpper64 = GetTraceIdUpper64(response.Headers.Get(HttpHeaderNames.PropagatedTags)); - var samplingPriority = response.Headers.Get(HttpHeaderNames.SamplingPriority); - if (ValidateOkStatus(response)) + + // response.Headers is a WebHeaderCollection, which is not convertable to IHeadersCollection, so we have to make it into a dictionary. + var myHeaders = response.Headers.AllKeys.ToDictionary(k => k, k => response.Headers.Get(k)); + if (!ValidateOkStatus(response)) { - return CreatePlaceholderScope(Tracer.Instance, traceId, traceIdUpper64, samplingPriority); + return null; } - return null; + var tracer = Tracer.Instance; + return NewCreatePlaceholderScope(tracer, myHeaders); } - /// - /// GetTraceIdUpper64 searches the x-datadog-tags tags for the upper 64 bits of the trace id. - /// Per the 128 bit tracing RFC, the upper 64 bits are stored in the _dd.p.tid tag as a hex string. - /// - /// The propagated tags from the x-datadog-tags header - /// the upper 64 bits of the traceId as a ulong - internal static ulong GetTraceIdUpper64(string ddTags) + internal static Scope NewCreatePlaceholderScope(Tracer tracer, Dictionary myHeaders) { - if (ddTags == null) - { - return 0; - } - - var tags = ddTags.Split(','); - foreach (var tag in tags) - { - var keyValue = tag.Trim().Split('='); - if (keyValue.Length == 2 && keyValue[0] == "_dd.p.tid") - { - return Convert.ToUInt64(keyValue[1], 16); - } - } - - return 0; + var sc = SpanContextPropagator.Instance.Extract(myHeaders); + TelemetryFactory.Metrics.RecordCountSpanCreated(MetricTags.IntegrationName.AwsLambda); + return tracer.StartActiveInternal(PlaceholderOperationName, sc); } internal static void SendEndInvocation(ILambdaExtensionRequest requestBuilder, Scope scope, bool isError, string data) diff --git a/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs b/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs index d7a1756c53be..54cb2544bca9 100644 --- a/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs +++ b/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs @@ -17,6 +17,8 @@ namespace Datadog.Trace.Tests { + extern alias DatadogTraceManual; + public class LambdaCommonTests { private readonly Mock _lambdaRequestMock = new(); @@ -25,8 +27,9 @@ public class LambdaCommonTests public async Task TestCreatePlaceholderScopeSuccessWithTraceIdOnly() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var scope = LambdaCommon.CreatePlaceholderScope(tracer, "1234", null); + var myHeaders = new Dictionary { { HttpHeaderNames.TraceId, "1234" }, }; + var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); scope.Should().NotBeNull(); scope.Span.TraceId128.Should().Be((TraceId)1234); ((ISpan)scope.Span).TraceId.Should().Be(1234); @@ -37,7 +40,11 @@ public async Task TestCreatePlaceholderScopeSuccessWithTraceIdOnly() public async Task TestCreatePlaceholderScopeSuccessWithSamplingPriorityOnly() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var scope = LambdaCommon.CreatePlaceholderScope(tracer, null, "-1"); + var myHeaders = new Dictionary + { + { HttpHeaderNames.SamplingPriority, "-1" }, + }; + var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); scope.Should().NotBeNull(); scope.Span.TraceId128.Should().BeGreaterThan(TraceId.Zero); @@ -47,10 +54,14 @@ public async Task TestCreatePlaceholderScopeSuccessWithSamplingPriorityOnly() } [Fact] - public async Task TestCreatePlaceholderScopeSuccessWithFullContext() + public async Task TestCreatePlaceholderScopeSuccessWith64BitTraceIdContext() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var scope = LambdaCommon.CreatePlaceholderScope(tracer, "1234", "-1"); + var myHeaders = new Dictionary + { + { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } + }; + var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); scope.Should().NotBeNull(); scope.Span.TraceId128.Should().Be((TraceId)1234); @@ -60,30 +71,33 @@ public async Task TestCreatePlaceholderScopeSuccessWithFullContext() } [Fact] - [Trait("Category", "ArmUnsupported")] - public async Task TestCreatePlaceholderScopeSuccessWithoutContext() + public async Task TestCreatePlaceholderScopeSuccessWith128BitTraceIdContext() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var scope = LambdaCommon.CreatePlaceholderScope(tracer, null, null); + var myHeaders = new Dictionary + { + { HttpHeaderNames.TraceId, "15744042798732701615" }, { HttpHeaderNames.SamplingPriority, "-1" }, { HttpHeaderNames.PropagatedTags, "_dd.p.tid=1914fe7789eb32be" } + }; + var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); scope.Should().NotBeNull(); - scope.Span.TraceId128.Should().BeGreaterThan((TraceId.Zero)); - ((ISpan)scope.Span).TraceId.Should().BeGreaterThan(0); + scope.Span.TraceId128.ToString().Should().Be("1914fe7789eb32be4fb6f07e011a6faf"); + ((ISpan)scope.Span).TraceId.Should().Be(1234); scope.Span.SpanId.Should().BeGreaterThan(0); + scope.Span.Context.TraceContext.SamplingPriority.Should().Be(-1); } [Fact] - public async Task TestCreatePlaceholderScopeInvalidTraceId() + [Trait("Category", "ArmUnsupported")] + public async Task TestCreatePlaceholderScopeSuccessWithoutContext() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - Assert.Throws(() => LambdaCommon.CreatePlaceholderScope(tracer, "invalid-trace-id", "-1")); - } + var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, new Dictionary()); - [Fact] - public async Task TestCreatePlaceholderScopeInvalidSamplingPriority() - { - await using var tracer = TracerHelper.CreateWithFakeAgent(); - Assert.Throws(() => LambdaCommon.CreatePlaceholderScope(tracer, "1234", "invalid-sampling-priority")); + scope.Should().NotBeNull(); + scope.Span.TraceId128.Should().BeGreaterThan((TraceId.Zero)); + ((ISpan)scope.Span).TraceId.Should().BeGreaterThan(0); + scope.Span.SpanId.Should().BeGreaterThan(0); } [Fact] @@ -146,7 +160,8 @@ public void TestSendStartInvocationSuccess() public async Task TestSendEndInvocationFailure() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var scope = LambdaCommon.CreatePlaceholderScope(tracer, "1234", "-1"); + var myHeaders = new Dictionary { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; + var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); var response = new Mock(MockBehavior.Loose); var responseStream = new Mock(MockBehavior.Loose); @@ -166,7 +181,8 @@ public async Task TestSendEndInvocationFailure() public async Task TestSendEndInvocationSuccess() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var scope = LambdaCommon.CreatePlaceholderScope(tracer, "1234", "-1"); + var myHeaders = new Dictionary { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; + var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); var response = new Mock(MockBehavior.Loose); var responseStream = new Mock(MockBehavior.Loose); @@ -189,7 +205,8 @@ public async Task TestSendEndInvocationSuccess() public async Task TestSendEndInvocationFalse() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var scope = LambdaCommon.CreatePlaceholderScope(tracer, "1234", "-1"); + var myHeaders = new Dictionary { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; + var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); var response = new Mock(MockBehavior.Loose); var responseStream = new Mock(MockBehavior.Loose); diff --git a/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs b/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs index 2797a0f82d89..23420645b669 100644 --- a/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs +++ b/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs @@ -3,6 +3,7 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. // #if NET6_0_OR_GREATER +using System.Collections.Generic; using System.Threading.Tasks; using Datadog.Trace.ClrProfiler.AutoInstrumentation.AWS.Lambda; using Datadog.Trace.TestHelpers; @@ -13,6 +14,8 @@ namespace Datadog.Trace.Tests { + extern alias DatadogTraceManual; + [Collection(nameof(WebRequestCollection))] public class LambdaRequestBuilderTests { @@ -20,7 +23,8 @@ public class LambdaRequestBuilderTests public async Task TestGetEndInvocationRequestWithError() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var scope = LambdaCommon.CreatePlaceholderScope(tracer, traceId: null, samplingPriority: null); + var myHeaders = new Dictionary(); + var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder(); var request = requestBuilder.GetEndInvocationRequest(scope, isError: true); @@ -35,7 +39,9 @@ public async Task TestGetEndInvocationRequestWithError() public async Task TestGetEndInvocationRequestWithoutError() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var scope = LambdaCommon.CreatePlaceholderScope(tracer, traceId: null, samplingPriority: null); + var myHeaders = new Dictionary(); + var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); + ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder(); var request = requestBuilder.GetEndInvocationRequest(scope, isError: false); @@ -50,7 +56,12 @@ public async Task TestGetEndInvocationRequestWithoutError() public async Task TestGetEndInvocationRequestWithScope() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var scope = LambdaCommon.CreatePlaceholderScope(tracer, traceId: "1234", samplingPriority: "-1"); + var myHeaders = new Dictionary + { + { HttpHeaderNames.TraceId, "1234" }, + { HttpHeaderNames.SamplingPriority, "-1" } + }; + var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder(); var request = requestBuilder.GetEndInvocationRequest(scope, isError: false); From 94d9f2443ec86e9f6410a51be368fdf39b25a8cb Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Wed, 25 Sep 2024 14:18:17 -0700 Subject: [PATCH 03/38] replace CreateNewPlaceholderScope with NewCreatePlaceholderScope in tests --- .../test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs b/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs index 23420645b669..02632b84060a 100644 --- a/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs +++ b/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs @@ -42,7 +42,6 @@ public async Task TestGetEndInvocationRequestWithoutError() var myHeaders = new Dictionary(); var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); - ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder(); var request = requestBuilder.GetEndInvocationRequest(scope, isError: false); request.Headers.Get("x-datadog-invocation-error").Should().BeNull(); @@ -88,7 +87,8 @@ public void TestGetEndInvocationRequestWithoutScope() public async Task TestGetEndInvocationRequestWithErrorTags() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var scope = LambdaCommon.CreatePlaceholderScope(tracer, traceId: null, samplingPriority: null); + var myHeaders = new Dictionary(); + var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); scope.Span.SetTag("error.msg", "Exception"); scope.Span.SetTag("error.type", "Exception"); scope.Span.SetTag("error.stack", "everything is " + System.Environment.NewLine + "fine"); @@ -109,7 +109,8 @@ public async Task TestGetEndInvocationRequestWithErrorTags() public async Task TestGetEndInvocationRequestWithoutErrorTags() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var scope = LambdaCommon.CreatePlaceholderScope(tracer, null, null); + var myHeaders = new Dictionary(); + var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder(); var request = requestBuilder.GetEndInvocationRequest(scope, true); From 00c5475956324f14e3f60f3b10575603e552ff70 Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Wed, 25 Sep 2024 14:19:12 -0700 Subject: [PATCH 04/38] Remove createPlaceholderScope --- .../AWS/Lambda/LambdaCommon.cs | 39 ------------------- 1 file changed, 39 deletions(-) diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs index b4d29bd43917..26766d8dae96 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs @@ -24,45 +24,6 @@ internal abstract class LambdaCommon private const double ServerlessMaxWaitingFlushTime = 3; private const string LogLevelEnvName = "DD_LOG_LEVEL"; - internal static Scope CreatePlaceholderScope(Tracer tracer, string traceId, ulong traceIdUpper64, string samplingPriority) - { - Span span; - - if (traceId == null) - { - Log("traceId not found"); - span = tracer.StartSpan(PlaceholderOperationName, tags: null, serviceName: PlaceholderServiceName, addToTraceContext: false); - } - else - { - if (traceIdUpper64 == 0) - { - Log($"creating the placeholder traceId = {traceId}"); - span = tracer.StartSpan(PlaceholderOperationName, tags: null, serviceName: PlaceholderServiceName, traceId: (TraceId)Convert.ToUInt64(traceId), addToTraceContext: false); - } - else - { - var traceIdLower64 = Convert.ToUInt64(traceId); - Log($"creating the placeholder traceId = {traceIdUpper64}{traceIdLower64}"); - span = tracer.StartSpan(PlaceholderOperationName, tags: null, serviceName: PlaceholderServiceName, traceId: new TraceId(traceIdUpper64, traceIdLower64), addToTraceContext: false); - } - } - - if (samplingPriority == null) - { - Log("samplingPriority not found"); - _ = span.Context.TraceContext?.GetOrMakeSamplingDecision(); - } - else - { - Log($"setting the placeholder sampling priority to = {samplingPriority}"); - span.Context.TraceContext?.SetSamplingPriority(Convert.ToInt32(samplingPriority), notifyDistributedTracer: false); - } - - TelemetryFactory.Metrics.RecordCountSpanCreated(MetricTags.IntegrationName.AwsLambda); - return tracer.TracerManager.ScopeManager.Activate(span, false); - } - internal static Scope SendStartInvocation(ILambdaExtensionRequest requestBuilder, string data, IDictionary context) { var request = requestBuilder.GetStartInvocationRequest(); From 55245cd8975ecd1f8ac264758471101b4268794a Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Wed, 25 Sep 2024 14:20:06 -0700 Subject: [PATCH 05/38] Rename NewCreatePlaceholderScope to CreatePlaceholderscope --- .../AWS/Lambda/LambdaCommon.cs | 4 ++-- .../Datadog.Trace.Tests/LambdaCommonTests.cs | 16 ++++++++-------- .../LambdaRequestBuilderTests.cs | 10 +++++----- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs index 26766d8dae96..6adecc639d62 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs @@ -39,10 +39,10 @@ internal static Scope SendStartInvocation(ILambdaExtensionRequest requestBuilder } var tracer = Tracer.Instance; - return NewCreatePlaceholderScope(tracer, myHeaders); + return CreatePlaceholderScope(tracer, myHeaders); } - internal static Scope NewCreatePlaceholderScope(Tracer tracer, Dictionary myHeaders) + internal static Scope CreatePlaceholderScope(Tracer tracer, Dictionary myHeaders) { var sc = SpanContextPropagator.Instance.Extract(myHeaders); TelemetryFactory.Metrics.RecordCountSpanCreated(MetricTags.IntegrationName.AwsLambda); diff --git a/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs b/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs index 54cb2544bca9..895ee75d964e 100644 --- a/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs +++ b/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs @@ -29,7 +29,7 @@ public async Task TestCreatePlaceholderScopeSuccessWithTraceIdOnly() await using var tracer = TracerHelper.CreateWithFakeAgent(); var myHeaders = new Dictionary { { HttpHeaderNames.TraceId, "1234" }, }; - var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); + var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); scope.Should().NotBeNull(); scope.Span.TraceId128.Should().Be((TraceId)1234); ((ISpan)scope.Span).TraceId.Should().Be(1234); @@ -44,7 +44,7 @@ public async Task TestCreatePlaceholderScopeSuccessWithSamplingPriorityOnly() { { HttpHeaderNames.SamplingPriority, "-1" }, }; - var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); + var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); scope.Should().NotBeNull(); scope.Span.TraceId128.Should().BeGreaterThan(TraceId.Zero); @@ -61,7 +61,7 @@ public async Task TestCreatePlaceholderScopeSuccessWith64BitTraceIdContext() { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; - var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); + var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); scope.Should().NotBeNull(); scope.Span.TraceId128.Should().Be((TraceId)1234); @@ -78,7 +78,7 @@ public async Task TestCreatePlaceholderScopeSuccessWith128BitTraceIdContext() { { HttpHeaderNames.TraceId, "15744042798732701615" }, { HttpHeaderNames.SamplingPriority, "-1" }, { HttpHeaderNames.PropagatedTags, "_dd.p.tid=1914fe7789eb32be" } }; - var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); + var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); scope.Should().NotBeNull(); scope.Span.TraceId128.ToString().Should().Be("1914fe7789eb32be4fb6f07e011a6faf"); @@ -92,7 +92,7 @@ public async Task TestCreatePlaceholderScopeSuccessWith128BitTraceIdContext() public async Task TestCreatePlaceholderScopeSuccessWithoutContext() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, new Dictionary()); + var scope = LambdaCommon.CreatePlaceholderScope(tracer, new Dictionary()); scope.Should().NotBeNull(); scope.Span.TraceId128.Should().BeGreaterThan((TraceId.Zero)); @@ -161,7 +161,7 @@ public async Task TestSendEndInvocationFailure() { await using var tracer = TracerHelper.CreateWithFakeAgent(); var myHeaders = new Dictionary { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; - var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); + var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); var response = new Mock(MockBehavior.Loose); var responseStream = new Mock(MockBehavior.Loose); @@ -182,7 +182,7 @@ public async Task TestSendEndInvocationSuccess() { await using var tracer = TracerHelper.CreateWithFakeAgent(); var myHeaders = new Dictionary { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; - var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); + var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); var response = new Mock(MockBehavior.Loose); var responseStream = new Mock(MockBehavior.Loose); @@ -206,7 +206,7 @@ public async Task TestSendEndInvocationFalse() { await using var tracer = TracerHelper.CreateWithFakeAgent(); var myHeaders = new Dictionary { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; - var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); + var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); var response = new Mock(MockBehavior.Loose); var responseStream = new Mock(MockBehavior.Loose); diff --git a/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs b/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs index 02632b84060a..9eb2c255bac8 100644 --- a/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs +++ b/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs @@ -24,7 +24,7 @@ public async Task TestGetEndInvocationRequestWithError() { await using var tracer = TracerHelper.CreateWithFakeAgent(); var myHeaders = new Dictionary(); - var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); + var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder(); var request = requestBuilder.GetEndInvocationRequest(scope, isError: true); @@ -40,7 +40,7 @@ public async Task TestGetEndInvocationRequestWithoutError() { await using var tracer = TracerHelper.CreateWithFakeAgent(); var myHeaders = new Dictionary(); - var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); + var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder(); var request = requestBuilder.GetEndInvocationRequest(scope, isError: false); @@ -60,7 +60,7 @@ public async Task TestGetEndInvocationRequestWithScope() { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; - var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); + var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder(); var request = requestBuilder.GetEndInvocationRequest(scope, isError: false); @@ -88,7 +88,7 @@ public async Task TestGetEndInvocationRequestWithErrorTags() { await using var tracer = TracerHelper.CreateWithFakeAgent(); var myHeaders = new Dictionary(); - var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); + var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); scope.Span.SetTag("error.msg", "Exception"); scope.Span.SetTag("error.type", "Exception"); scope.Span.SetTag("error.stack", "everything is " + System.Environment.NewLine + "fine"); @@ -110,7 +110,7 @@ public async Task TestGetEndInvocationRequestWithoutErrorTags() { await using var tracer = TracerHelper.CreateWithFakeAgent(); var myHeaders = new Dictionary(); - var scope = LambdaCommon.NewCreatePlaceholderScope(tracer, myHeaders); + var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder(); var request = requestBuilder.GetEndInvocationRequest(scope, true); From cee09c077855426d42f64d1eea4ec60ec3bb7372 Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Mon, 30 Sep 2024 09:05:32 -0400 Subject: [PATCH 06/38] Savepoint --- .../AWS/Lambda/LambdaCommon.cs | 13 +++++++++++-- .../Datadog.Trace.Tests/LambdaCommonTests.cs | 17 ----------------- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs index 6adecc639d62..a1b791ec0d0a 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs @@ -9,6 +9,7 @@ using System.Net; using System.Text; using System.Threading.Tasks; +using Datadog.Trace.ExtensionMethods; using Datadog.Trace.Headers; using Datadog.Trace.Propagators; using Datadog.Trace.Telemetry; @@ -44,9 +45,17 @@ internal static Scope SendStartInvocation(ILambdaExtensionRequest requestBuilder internal static Scope CreatePlaceholderScope(Tracer tracer, Dictionary myHeaders) { - var sc = SpanContextPropagator.Instance.Extract(myHeaders); + var spanContext = SpanContextPropagator.Instance.Extract(myHeaders); TelemetryFactory.Metrics.RecordCountSpanCreated(MetricTags.IntegrationName.AwsLambda); - return tracer.StartActiveInternal(PlaceholderOperationName, sc); + + if (spanContext != null) + { + // The problem is, we're not setting span.SamplingPriority + return tracer.StartActiveInternal(operationName: PlaceholderOperationName, parent: spanContext, serviceName: PlaceholderServiceName); + } + + var span = tracer.StartSpan(PlaceholderOperationName, tags: null, serviceName: PlaceholderServiceName, addToTraceContext: false); + return tracer.TracerManager.ScopeManager.Activate(span, false); } internal static void SendEndInvocation(ILambdaExtensionRequest requestBuilder, Scope scope, bool isError, string data) diff --git a/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs b/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs index 895ee75d964e..d8499e783a1e 100644 --- a/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs +++ b/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs @@ -36,23 +36,6 @@ public async Task TestCreatePlaceholderScopeSuccessWithTraceIdOnly() scope.Span.SpanId.Should().BeGreaterThan(0); } - [Fact] - public async Task TestCreatePlaceholderScopeSuccessWithSamplingPriorityOnly() - { - await using var tracer = TracerHelper.CreateWithFakeAgent(); - var myHeaders = new Dictionary - { - { HttpHeaderNames.SamplingPriority, "-1" }, - }; - var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); - - scope.Should().NotBeNull(); - scope.Span.TraceId128.Should().BeGreaterThan(TraceId.Zero); - ((ISpan)scope.Span).TraceId.Should().BeGreaterThan(0); - scope.Span.SpanId.Should().BeGreaterThan(0); - scope.Span.Context.TraceContext.SamplingPriority.Should().Be(-1); - } - [Fact] public async Task TestCreatePlaceholderScopeSuccessWith64BitTraceIdContext() { From ddd0fb6a0956182b4fdccb8cd64dac9359f82606 Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Mon, 30 Sep 2024 09:11:17 -0400 Subject: [PATCH 07/38] Fix test for 128 bit trace IDs --- tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs b/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs index d8499e783a1e..3a44d9fbaa95 100644 --- a/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs +++ b/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs @@ -59,13 +59,12 @@ public async Task TestCreatePlaceholderScopeSuccessWith128BitTraceIdContext() await using var tracer = TracerHelper.CreateWithFakeAgent(); var myHeaders = new Dictionary { - { HttpHeaderNames.TraceId, "15744042798732701615" }, { HttpHeaderNames.SamplingPriority, "-1" }, { HttpHeaderNames.PropagatedTags, "_dd.p.tid=1914fe7789eb32be" } + { HttpHeaderNames.TraceId, "5744042798732701615" }, { HttpHeaderNames.SamplingPriority, "-1" }, { HttpHeaderNames.PropagatedTags, "_dd.p.tid=1914fe7789eb32be" } }; var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); scope.Should().NotBeNull(); scope.Span.TraceId128.ToString().Should().Be("1914fe7789eb32be4fb6f07e011a6faf"); - ((ISpan)scope.Span).TraceId.Should().Be(1234); scope.Span.SpanId.Should().BeGreaterThan(0); scope.Span.Context.TraceContext.SamplingPriority.Should().Be(-1); } From b300797b82f7a1029c4ef9a588bad9883b2d6ca0 Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Mon, 30 Sep 2024 09:45:07 -0400 Subject: [PATCH 08/38] Reorganize LambdaCommon a little bit --- .../AWS/Lambda/LambdaCommon.cs | 31 +++++++++---------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs index a1b791ec0d0a..edb7ece7c7cc 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs @@ -9,8 +9,6 @@ using System.Net; using System.Text; using System.Threading.Tasks; -using Datadog.Trace.ExtensionMethods; -using Datadog.Trace.Headers; using Datadog.Trace.Propagators; using Datadog.Trace.Telemetry; using Datadog.Trace.Telemetry.Metrics; @@ -25,6 +23,20 @@ internal abstract class LambdaCommon private const double ServerlessMaxWaitingFlushTime = 3; private const string LogLevelEnvName = "DD_LOG_LEVEL"; + internal static Scope CreatePlaceholderScope(Tracer tracer, Dictionary myHeaders) + { + var spanContext = SpanContextPropagator.Instance.Extract(myHeaders); + TelemetryFactory.Metrics.RecordCountSpanCreated(MetricTags.IntegrationName.AwsLambda); + + if (spanContext != null) + { + return tracer.StartActiveInternal(operationName: PlaceholderOperationName, parent: spanContext, serviceName: PlaceholderServiceName); + } + + var span = tracer.StartSpan(PlaceholderOperationName, tags: null, serviceName: PlaceholderServiceName, addToTraceContext: false); + return tracer.TracerManager.ScopeManager.Activate(span, false); + } + internal static Scope SendStartInvocation(ILambdaExtensionRequest requestBuilder, string data, IDictionary context) { var request = requestBuilder.GetStartInvocationRequest(); @@ -43,21 +55,6 @@ internal static Scope SendStartInvocation(ILambdaExtensionRequest requestBuilder return CreatePlaceholderScope(tracer, myHeaders); } - internal static Scope CreatePlaceholderScope(Tracer tracer, Dictionary myHeaders) - { - var spanContext = SpanContextPropagator.Instance.Extract(myHeaders); - TelemetryFactory.Metrics.RecordCountSpanCreated(MetricTags.IntegrationName.AwsLambda); - - if (spanContext != null) - { - // The problem is, we're not setting span.SamplingPriority - return tracer.StartActiveInternal(operationName: PlaceholderOperationName, parent: spanContext, serviceName: PlaceholderServiceName); - } - - var span = tracer.StartSpan(PlaceholderOperationName, tags: null, serviceName: PlaceholderServiceName, addToTraceContext: false); - return tracer.TracerManager.ScopeManager.Activate(span, false); - } - internal static void SendEndInvocation(ILambdaExtensionRequest requestBuilder, Scope scope, bool isError, string data) { var request = requestBuilder.GetEndInvocationRequest(scope, isError); From 3fd8cd6c7bca7e21070feaf20c57de0ba9fc57a7 Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Tue, 1 Oct 2024 10:10:26 -0400 Subject: [PATCH 09/38] CreatePlaceholderScope takes WebHeaderCollection --- .../AWS/Lambda/LambdaCommon.cs | 9 +++--- .../Datadog.Trace.Tests/LambdaCommonTests.cs | 32 ++++++++----------- .../LambdaRequestBuilderTests.cs | 26 +++++++-------- 3 files changed, 28 insertions(+), 39 deletions(-) diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs index edb7ece7c7cc..799fd2e5f02c 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs @@ -5,7 +5,6 @@ #if NET6_0_OR_GREATER using System; using System.Collections.Generic; -using System.Linq; using System.Net; using System.Text; using System.Threading.Tasks; @@ -23,9 +22,9 @@ internal abstract class LambdaCommon private const double ServerlessMaxWaitingFlushTime = 3; private const string LogLevelEnvName = "DD_LOG_LEVEL"; - internal static Scope CreatePlaceholderScope(Tracer tracer, Dictionary myHeaders) + internal static Scope CreatePlaceholderScope(Tracer tracer, WebHeaderCollection headers) { - var spanContext = SpanContextPropagator.Instance.Extract(myHeaders); + var spanContext = SpanContextPropagator.Instance.Extract(headers, (h, name) => h.GetValues(name)); TelemetryFactory.Metrics.RecordCountSpanCreated(MetricTags.IntegrationName.AwsLambda); if (spanContext != null) @@ -45,14 +44,14 @@ internal static Scope SendStartInvocation(ILambdaExtensionRequest requestBuilder var response = (HttpWebResponse)request.GetResponse(); // response.Headers is a WebHeaderCollection, which is not convertable to IHeadersCollection, so we have to make it into a dictionary. - var myHeaders = response.Headers.AllKeys.ToDictionary(k => k, k => response.Headers.Get(k)); + var headers = response.Headers; if (!ValidateOkStatus(response)) { return null; } var tracer = Tracer.Instance; - return CreatePlaceholderScope(tracer, myHeaders); + return CreatePlaceholderScope(tracer, headers); } internal static void SendEndInvocation(ILambdaExtensionRequest requestBuilder, Scope scope, bool isError, string data) diff --git a/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs b/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs index 3a44d9fbaa95..a17905d4b473 100644 --- a/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs +++ b/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs @@ -28,8 +28,8 @@ public async Task TestCreatePlaceholderScopeSuccessWithTraceIdOnly() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var myHeaders = new Dictionary { { HttpHeaderNames.TraceId, "1234" }, }; - var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); + var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" } }; + var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); scope.Should().NotBeNull(); scope.Span.TraceId128.Should().Be((TraceId)1234); ((ISpan)scope.Span).TraceId.Should().Be(1234); @@ -40,11 +40,8 @@ public async Task TestCreatePlaceholderScopeSuccessWithTraceIdOnly() public async Task TestCreatePlaceholderScopeSuccessWith64BitTraceIdContext() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var myHeaders = new Dictionary - { - { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } - }; - var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); + var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; + var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); scope.Should().NotBeNull(); scope.Span.TraceId128.Should().Be((TraceId)1234); @@ -57,11 +54,8 @@ public async Task TestCreatePlaceholderScopeSuccessWith64BitTraceIdContext() public async Task TestCreatePlaceholderScopeSuccessWith128BitTraceIdContext() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var myHeaders = new Dictionary - { - { HttpHeaderNames.TraceId, "5744042798732701615" }, { HttpHeaderNames.SamplingPriority, "-1" }, { HttpHeaderNames.PropagatedTags, "_dd.p.tid=1914fe7789eb32be" } - }; - var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); + var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "5744042798732701615" }, { HttpHeaderNames.SamplingPriority, "-1" }, { HttpHeaderNames.PropagatedTags, "_dd.p.tid=1914fe7789eb32be" } }; + var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); scope.Should().NotBeNull(); scope.Span.TraceId128.ToString().Should().Be("1914fe7789eb32be4fb6f07e011a6faf"); @@ -74,7 +68,7 @@ public async Task TestCreatePlaceholderScopeSuccessWith128BitTraceIdContext() public async Task TestCreatePlaceholderScopeSuccessWithoutContext() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var scope = LambdaCommon.CreatePlaceholderScope(tracer, new Dictionary()); + var scope = LambdaCommon.CreatePlaceholderScope(tracer, new WebHeaderCollection()); scope.Should().NotBeNull(); scope.Span.TraceId128.Should().BeGreaterThan((TraceId.Zero)); @@ -142,8 +136,8 @@ public void TestSendStartInvocationSuccess() public async Task TestSendEndInvocationFailure() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var myHeaders = new Dictionary { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; - var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); + var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; + var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); var response = new Mock(MockBehavior.Loose); var responseStream = new Mock(MockBehavior.Loose); @@ -163,8 +157,8 @@ public async Task TestSendEndInvocationFailure() public async Task TestSendEndInvocationSuccess() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var myHeaders = new Dictionary { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; - var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); + var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; + var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); var response = new Mock(MockBehavior.Loose); var responseStream = new Mock(MockBehavior.Loose); @@ -187,8 +181,8 @@ public async Task TestSendEndInvocationSuccess() public async Task TestSendEndInvocationFalse() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var myHeaders = new Dictionary { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; - var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); + var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; + var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); var response = new Mock(MockBehavior.Loose); var responseStream = new Mock(MockBehavior.Loose); diff --git a/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs b/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs index 9eb2c255bac8..313183dd6b8e 100644 --- a/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs +++ b/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs @@ -3,7 +3,7 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. // #if NET6_0_OR_GREATER -using System.Collections.Generic; +using System.Net; using System.Threading.Tasks; using Datadog.Trace.ClrProfiler.AutoInstrumentation.AWS.Lambda; using Datadog.Trace.TestHelpers; @@ -23,8 +23,8 @@ public class LambdaRequestBuilderTests public async Task TestGetEndInvocationRequestWithError() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var myHeaders = new Dictionary(); - var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); + var headers = new WebHeaderCollection(); + var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder(); var request = requestBuilder.GetEndInvocationRequest(scope, isError: true); @@ -39,8 +39,8 @@ public async Task TestGetEndInvocationRequestWithError() public async Task TestGetEndInvocationRequestWithoutError() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var myHeaders = new Dictionary(); - var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); + var headers = new WebHeaderCollection(); + var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder(); var request = requestBuilder.GetEndInvocationRequest(scope, isError: false); @@ -55,12 +55,8 @@ public async Task TestGetEndInvocationRequestWithoutError() public async Task TestGetEndInvocationRequestWithScope() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var myHeaders = new Dictionary - { - { HttpHeaderNames.TraceId, "1234" }, - { HttpHeaderNames.SamplingPriority, "-1" } - }; - var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); + var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; + var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder(); var request = requestBuilder.GetEndInvocationRequest(scope, isError: false); @@ -87,8 +83,8 @@ public void TestGetEndInvocationRequestWithoutScope() public async Task TestGetEndInvocationRequestWithErrorTags() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var myHeaders = new Dictionary(); - var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); + var headers = new WebHeaderCollection(); + var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); scope.Span.SetTag("error.msg", "Exception"); scope.Span.SetTag("error.type", "Exception"); scope.Span.SetTag("error.stack", "everything is " + System.Environment.NewLine + "fine"); @@ -109,8 +105,8 @@ public async Task TestGetEndInvocationRequestWithErrorTags() public async Task TestGetEndInvocationRequestWithoutErrorTags() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var myHeaders = new Dictionary(); - var scope = LambdaCommon.CreatePlaceholderScope(tracer, myHeaders); + var headers = new WebHeaderCollection(); + var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder(); var request = requestBuilder.GetEndInvocationRequest(scope, true); From 9fb8fcd39c8a4c765a95d52faf6f402df2bbffcb Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Wed, 2 Oct 2024 09:18:15 -0400 Subject: [PATCH 10/38] Get empty string array from WebHeadersCollection instead of null --- .../AutoInstrumentation/AWS/Lambda/LambdaCommon.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs index 799fd2e5f02c..9e5f7c7c324c 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs @@ -24,7 +24,7 @@ internal abstract class LambdaCommon internal static Scope CreatePlaceholderScope(Tracer tracer, WebHeaderCollection headers) { - var spanContext = SpanContextPropagator.Instance.Extract(headers, (h, name) => h.GetValues(name)); + var spanContext = SpanContextPropagator.Instance.Extract(headers, SafeGetValues); TelemetryFactory.Metrics.RecordCountSpanCreated(MetricTags.IntegrationName.AwsLambda); if (spanContext != null) @@ -36,6 +36,13 @@ internal static Scope CreatePlaceholderScope(Tracer tracer, WebHeaderCollection return tracer.TracerManager.ScopeManager.Activate(span, false); } + // parseUtility.ParseUInt64 falls over and dies when it tries to parse a null collection of values, and WebHeadersCollection.GetValues will return null if the header does not exist. + private static string[] SafeGetValues(WebHeaderCollection headers, string name) + { + var values = headers.GetValues(name); + return values ?? []; + } + internal static Scope SendStartInvocation(ILambdaExtensionRequest requestBuilder, string data, IDictionary context) { var request = requestBuilder.GetStartInvocationRequest(); From ad30105f6c0cfe743f4c7045556baf9abe807a69 Mon Sep 17 00:00:00 2001 From: Christopher Agocs Date: Thu, 3 Oct 2024 14:23:16 -0400 Subject: [PATCH 11/38] Update tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs Co-authored-by: Lucas Pimentel --- .../ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs index 9e5f7c7c324c..d08fe1962563 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs @@ -50,7 +50,6 @@ internal static Scope SendStartInvocation(ILambdaExtensionRequest requestBuilder WriteRequestHeaders(request, context); var response = (HttpWebResponse)request.GetResponse(); - // response.Headers is a WebHeaderCollection, which is not convertable to IHeadersCollection, so we have to make it into a dictionary. var headers = response.Headers; if (!ValidateOkStatus(response)) { From d2a6d929846196ba6257c007b3a6ba86bde751d7 Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Mon, 30 Sep 2024 09:05:32 -0400 Subject: [PATCH 12/38] Cherry pick commits from v3 --- .../AWS/Lambda/LambdaCommon.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs index d08fe1962563..b5b4aae06bd5 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs @@ -58,6 +58,22 @@ internal static Scope SendStartInvocation(ILambdaExtensionRequest requestBuilder var tracer = Tracer.Instance; return CreatePlaceholderScope(tracer, headers); + + } + + internal static Scope CreatePlaceholderScope(Tracer tracer, Dictionary myHeaders) + { + var spanContext = SpanContextPropagator.Instance.Extract(myHeaders); + TelemetryFactory.Metrics.RecordCountSpanCreated(MetricTags.IntegrationName.AwsLambda); + + if (spanContext != null) + { + // The problem is, we're not setting span.SamplingPriority + return tracer.StartActiveInternal(operationName: PlaceholderOperationName, parent: spanContext, serviceName: PlaceholderServiceName); + } + + var span = tracer.StartSpan(PlaceholderOperationName, tags: null, serviceName: PlaceholderServiceName, addToTraceContext: false); + return tracer.TracerManager.ScopeManager.Activate(span, false); } internal static void SendEndInvocation(ILambdaExtensionRequest requestBuilder, Scope scope, bool isError, string data) From 274d538fe2e0488b81e8083d0843224deb2e7f26 Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Mon, 30 Sep 2024 09:11:17 -0400 Subject: [PATCH 13/38] Fix test for 128 bit trace IDs From 83362d6cab79b732308133ce81f7dd665b6d71eb Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Mon, 30 Sep 2024 09:45:07 -0400 Subject: [PATCH 14/38] Reorganize LambdaCommon a little bit --- .../AWS/Lambda/LambdaCommon.cs | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs index b5b4aae06bd5..bff68c5f6a3d 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs @@ -61,21 +61,6 @@ internal static Scope SendStartInvocation(ILambdaExtensionRequest requestBuilder } - internal static Scope CreatePlaceholderScope(Tracer tracer, Dictionary myHeaders) - { - var spanContext = SpanContextPropagator.Instance.Extract(myHeaders); - TelemetryFactory.Metrics.RecordCountSpanCreated(MetricTags.IntegrationName.AwsLambda); - - if (spanContext != null) - { - // The problem is, we're not setting span.SamplingPriority - return tracer.StartActiveInternal(operationName: PlaceholderOperationName, parent: spanContext, serviceName: PlaceholderServiceName); - } - - var span = tracer.StartSpan(PlaceholderOperationName, tags: null, serviceName: PlaceholderServiceName, addToTraceContext: false); - return tracer.TracerManager.ScopeManager.Activate(span, false); - } - internal static void SendEndInvocation(ILambdaExtensionRequest requestBuilder, Scope scope, bool isError, string data) { var request = requestBuilder.GetEndInvocationRequest(scope, isError); From ed98cb18c93ae291087793e1aede3a8df19cadc7 Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Tue, 1 Oct 2024 10:10:26 -0400 Subject: [PATCH 15/38] CreatePlaceholderScope takes WebHeaderCollection --- .../ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs index bff68c5f6a3d..d08fe1962563 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs @@ -58,7 +58,6 @@ internal static Scope SendStartInvocation(ILambdaExtensionRequest requestBuilder var tracer = Tracer.Instance; return CreatePlaceholderScope(tracer, headers); - } internal static void SendEndInvocation(ILambdaExtensionRequest requestBuilder, Scope scope, bool isError, string data) From 52ccc64edca8a74c5fdc7f7a7899317a145f10f1 Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Wed, 2 Oct 2024 09:18:15 -0400 Subject: [PATCH 16/38] Get empty string array from WebHeadersCollection instead of null From 60ccc82fc1adea683141baed424084162507822d Mon Sep 17 00:00:00 2001 From: Christopher Agocs Date: Thu, 3 Oct 2024 14:23:16 -0400 Subject: [PATCH 17/38] Update tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs Co-authored-by: Lucas Pimentel From bf5a0109cd1585c2d889e791dbe1c017846baac4 Mon Sep 17 00:00:00 2001 From: Andrew Lock Date: Mon, 30 Sep 2024 15:54:23 +0100 Subject: [PATCH 18/38] Minor build fixes (#6099) ## Summary of changes - Ensure we always upload logs, even if a job is cancelled - Don't clean test results multiple times per run ## Reason for change We have some cases where integration tests are taking _way_ too long to run, and CI times out and cancels the job. Currently, that means we also don't get any logs. Also, we're (accidentally) cleaning the test log folder multiple times in some stages. For example, we run Windows integration tests and regressions tests (two different stages) in the CI with a single command. Both of the run steps will clear the test folder, so we end up losing the integration test logs (unless they fail, in which case the regression tests never run) ## Implementation details - Changed from `succeededOrFailed()` to `always()` for the log and snapshot upload steps - Extracted a "clean test folder" target which runs automatically _once_ for the full execution. ## Test coverage This is the test --- .azure-pipelines/ultimate-pipeline.yml | 112 +++++++++--------- tracer/build/_build/Build.ExplorationTests.cs | 2 +- tracer/build/_build/Build.Steps.cs | 23 ++-- tracer/build/_build/Build.cs | 10 ++ 4 files changed, 74 insertions(+), 73 deletions(-) diff --git a/.azure-pipelines/ultimate-pipeline.yml b/.azure-pipelines/ultimate-pipeline.yml index e0624cd7dfc5..6e567964e1a9 100644 --- a/.azure-pipelines/ultimate-pipeline.yml +++ b/.azure-pipelines/ultimate-pipeline.yml @@ -1357,7 +1357,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - task: PublishTestResults@2 @@ -1407,7 +1407,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - task: PublishTestResults@2 @@ -1460,7 +1460,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - task: PublishTestResults@2 @@ -1513,7 +1513,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - task: PublishTestResults@2 @@ -1585,13 +1585,13 @@ stages: - publish: artifacts/build_data displayName: Uploading integration_tests_windows tracer logs artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - publish: tracer/test/snapshots displayName: Uploading snapshots artifact: _$(System.StageName)_$(Agent.JobName)_snapshots_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - script: tracer\build.cmd CheckBuildLogsForErrors @@ -1648,13 +1648,13 @@ stages: - publish: artifacts/build_data displayName: Uploading integration_tests_windows_debugger tracer logs artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - publish: tracer/test/snapshots displayName: Uploading snapshots artifact: _$(System.StageName)_$(Agent.JobName)_snapshots_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - script: tracer\build.cmd CheckBuildLogsForErrors @@ -1708,13 +1708,13 @@ stages: - publish: artifacts/build_data displayName: Uploading integration_tests_windows_iis tracer logs artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - publish: tracer/test/snapshots displayName: Uploading snapshots artifact: _$(System.StageName)_$(Agent.JobName)_snapshots_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - script: tracer\build.cmd CheckBuildLogsForErrors @@ -1768,13 +1768,13 @@ stages: - publish: artifacts/build_data displayName: Uploading integration_tests_windows_iis tracer logs artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - publish: tracer/test/snapshots displayName: Uploading snapshots artifact: _$(System.StageName)_$(Agent.JobName)_snapshots_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - script: tracer\build.cmd CheckBuildLogsForErrors @@ -1840,13 +1840,13 @@ stages: - publish: artifacts/build_data displayName: Uploading tracer logs artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - publish: tracer/test/snapshots displayName: Uploading snapshots artifact: _$(System.StageName)_$(Agent.JobName)_snapshots_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - script: tracer\build.cmd CheckBuildLogsForErrors @@ -2052,13 +2052,13 @@ stages: - publish: artifacts/build_data displayName: Uploading integration_tests_windows_msi tracer logs artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - publish: tracer/test/snapshots displayName: Uploading snapshots artifact: _$(System.StageName)_$(Agent.JobName)_snapshots_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - script: tracer\build.cmd CheckBuildLogsForErrors @@ -2134,7 +2134,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - task: PublishTestResults@2 @@ -2147,7 +2147,7 @@ stages: - publish: tracer/test/snapshots displayName: Uploading snapshots artifact: _$(System.StageName)_$(Agent.JobName)_snapshots_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/run-in-docker.yml @@ -2234,7 +2234,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - task: PublishTestResults@2 @@ -2247,7 +2247,7 @@ stages: - publish: tracer/test/snapshots displayName: Uploading snapshots artifact: _$(System.StageName)_$(Agent.JobName)_snapshots_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/run-in-docker.yml @@ -2414,7 +2414,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - task: PublishTestResults@2 @@ -2427,7 +2427,7 @@ stages: - publish: tracer/test/snapshots displayName: Uploading snapshots artifact: _$(System.StageName)_$(Agent.JobName)_snapshots_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/run-in-docker.yml @@ -2508,7 +2508,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - task: PublishTestResults@2 @@ -2521,7 +2521,7 @@ stages: - publish: tracer/test/Datadog.Trace.Debugger.IntegrationTests/Approvals displayName: Uploading snapshots artifact: _$(System.StageName)_$(Agent.JobName)_snapshots_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/run-in-docker.yml @@ -2594,7 +2594,7 @@ stages: - publish: profiler/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - task: PublishTestResults@2 @@ -2702,7 +2702,7 @@ stages: - publish: profiler/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - task: PublishTestResults@2 @@ -2772,7 +2772,7 @@ stages: - publish: profiler/build_data displayName: Uploading Address sanitizer test results artifact: _$(System.StageName)_$(Agent.JobName)_test_results_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: false - job: Windows @@ -2798,7 +2798,7 @@ stages: - publish: profiler/build_data displayName: Uploading Address sanitizer test results artifact: _$(System.StageName)_$(Agent.JobName)_test_results_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: false - stage: ubsan_profiler_tests @@ -2861,7 +2861,7 @@ stages: - publish: profiler/build_data displayName: Uploading test results artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: false - stage: integration_tests_arm64 @@ -2928,7 +2928,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - task: PublishTestResults@2 @@ -2941,7 +2941,7 @@ stages: - publish: tracer/test/snapshots displayName: Uploading snapshots artifact: _$(System.StageName)_$(Agent.JobName)_snapshots_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/run-in-docker.yml @@ -3018,7 +3018,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - task: PublishTestResults@2 @@ -3031,7 +3031,7 @@ stages: - publish: tracer/test/snapshots displayName: Uploading snapshots artifact: _$(System.StageName)_$(Agent.JobName)_snapshots_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/run-in-docker.yml @@ -3104,7 +3104,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - task: PublishTestResults@2 @@ -3117,7 +3117,7 @@ stages: - publish: tracer/test/snapshots displayName: Uploading snapshots artifact: _$(System.StageName)_$(Agent.JobName)_snapshots_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/run-in-docker.yml @@ -3179,7 +3179,7 @@ stages: - publish: artifacts/build_data displayName: Uploading exploration_tests_windows tracer logs artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - script: tracer\build.cmd CheckBuildLogsForErrors @@ -3284,7 +3284,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/run-in-docker.yml @@ -3807,7 +3807,7 @@ stages: - publish: artifacts/build_data displayName: Uploading tool_artifacts_tests_windows logs artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - task: PublishTestResults@2 @@ -3935,7 +3935,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - task: PublishTestResults@2 @@ -5161,7 +5161,7 @@ stages: snapshotPrefix: "smoke_test" - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/run-in-docker.yml @@ -5242,7 +5242,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - stage: nuget_installer_smoke_tests @@ -5328,7 +5328,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/run-in-docker.yml @@ -5393,7 +5393,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/run-in-docker.yml @@ -5462,7 +5462,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/run-in-docker.yml @@ -5541,7 +5541,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/run-in-docker.yml @@ -5625,7 +5625,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/run-in-docker.yml @@ -5705,7 +5705,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - stage: nuget_installer_smoke_tests_arm64 @@ -5791,7 +5791,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/run-in-docker.yml @@ -5855,7 +5855,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/run-in-docker.yml @@ -5956,7 +5956,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/install-latest-dotnet-sdk.yml @@ -6030,7 +6030,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/install-latest-dotnet-sdk.yml @@ -6103,7 +6103,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/install-latest-dotnet-sdk.yml @@ -6177,7 +6177,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/install-latest-dotnet-sdk.yml @@ -6249,7 +6249,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - template: steps/install-latest-dotnet-sdk.yml @@ -6379,7 +6379,7 @@ stages: - publish: artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - script: ./tracer/build.sh CheckSmokeTestsForErrors @@ -6631,7 +6631,7 @@ stages: - publish: $(Build.SourcesDirectory)/artifacts/build_data artifact: _$(System.StageName)_$(Agent.JobName)_logs_$(System.JobAttempt) - condition: succeededOrFailed() + condition: always() continueOnError: true - stage: notify_slack_on_failures diff --git a/tracer/build/_build/Build.ExplorationTests.cs b/tracer/build/_build/Build.ExplorationTests.cs index 814cef75692f..63b6090a47f6 100644 --- a/tracer/build/_build/Build.ExplorationTests.cs +++ b/tracer/build/_build/Build.ExplorationTests.cs @@ -143,9 +143,9 @@ Target RunExplorationTests .Description("Run exploration tests.") .Requires(() => ExplorationTestUseCase) .After(Clean, BuildTracerHome, BuildNativeLoader, SetUpExplorationTests) + .DependsOn(CleanTestLogs) .Executes(() => { - FileSystemTasks.EnsureCleanDirectory(TestLogsDirectory); try { RunExplorationTestsGitUnitTest(); diff --git a/tracer/build/_build/Build.Steps.cs b/tracer/build/_build/Build.Steps.cs index 15e9ef753385..2d35789041bb 100644 --- a/tracer/build/_build/Build.Steps.cs +++ b/tracer/build/_build/Build.Steps.cs @@ -1160,10 +1160,9 @@ void PrepareMonitoringHomeLinuxForPackaging(AbsolutePath assetsDirectory, string Target RunManagedUnitTests => _ => _ .Unlisted() .After(CompileManagedUnitTests) + .DependsOn(CleanTestLogs) .Executes(() => { - EnsureCleanDirectory(TestLogsDirectory); - var testProjects = TracerDirectory.GlobFiles("test/**/*.Tests.csproj") .Select(x => Solution.GetProject(x)) .ToList(); @@ -1540,15 +1539,13 @@ _ when exclude.Contains(project.Path) => false, .After(CompileSamplesWindows) .After(CompileFrameworkReproductions) .After(BuildWindowsIntegrationTests) + .DependsOn(CleanTestLogs) .Requires(() => IsWin) .Requires(() => Framework) .Triggers(PrintSnapshotsDiff) .Executes(() => { var isDebugRun = IsDebugRun(); - EnsureCleanDirectory(TestLogsDirectory); - ParallelIntegrationTests.ForEach(EnsureResultsDirectory); - ClrProfilerIntegrationTests.ForEach(EnsureResultsDirectory); try { @@ -1634,6 +1631,7 @@ _ when exclude.Contains(project.Path) => false, .After(CompileIntegrationTests) .After(CompileAzureFunctionsSamplesWindows) .After(BuildWindowsIntegrationTests) + .DependsOn(CleanTestLogs) .Requires(() => IsWin) .Requires(() => Framework) .Triggers(PrintSnapshotsDiff) @@ -1641,7 +1639,6 @@ _ when exclude.Contains(project.Path) => false, { var isDebugRun = IsDebugRun(); var project = Solution.GetProject(Projects.ClrProfilerIntegrationTests); - EnsureCleanDirectory(TestLogsDirectory); EnsureResultsDirectory(project); try @@ -1677,13 +1674,12 @@ _ when exclude.Contains(project.Path) => false, .After(CompileRegressionSamples) .After(CompileFrameworkReproductions) .After(BuildNativeLoader) + .DependsOn(CleanTestLogs) .Requires(() => IsWin) .Requires(() => Framework) .Executes(() => { var isDebugRun = IsDebugRun(); - EnsureCleanDirectory(TestLogsDirectory); - ClrProfilerIntegrationTests.ForEach(EnsureResultsDirectory); try { @@ -2030,13 +2026,12 @@ var name when multiPackageProjects.Contains(name) => false, Target RunLinuxDdDotnetIntegrationTests => _ => _ .After(CompileLinuxOrOsxIntegrationTests) + .DependsOn(CleanTestLogs) .Description("Runs the linux dd-dotnet integration tests") .Requires(() => !IsWin) .Executes(() => { var project = Solution.GetProject(Projects.DdTraceIntegrationTests); - - EnsureCleanDirectory(TestLogsDirectory); EnsureResultsDirectory(project); try @@ -2062,6 +2057,7 @@ var name when multiPackageProjects.Contains(name) => false, Target RunLinuxIntegrationTests => _ => _ .After(CompileLinuxOrOsxIntegrationTests) + .DependsOn(CleanTestLogs) .Description("Runs the linux integration tests") .Requires(() => Framework) .Requires(() => !IsWin) @@ -2069,9 +2065,6 @@ var name when multiPackageProjects.Contains(name) => false, .Executes(() => { var isDebugRun = IsDebugRun(); - EnsureCleanDirectory(TestLogsDirectory); - ParallelIntegrationTests.ForEach(EnsureResultsDirectory); - ClrProfilerIntegrationTests.ForEach(EnsureResultsDirectory); var dockerFilter = IncludeTestsRequiringDocker switch { @@ -2143,6 +2136,7 @@ var name when multiPackageProjects.Contains(name) => false, Target RunOsxIntegrationTests => _ => _ .After(CompileLinuxOrOsxIntegrationTests) + .DependsOn(CleanTestLogs) .Description("Runs the osx integration tests") .Requires(() => Framework) .Requires(() => IsOsx) @@ -2150,9 +2144,6 @@ var name when multiPackageProjects.Contains(name) => false, .Executes(() => { var isDebugRun = IsDebugRun(); - EnsureCleanDirectory(TestLogsDirectory); - ParallelIntegrationTests.ForEach(EnsureResultsDirectory); - ClrProfilerIntegrationTests.ForEach(EnsureResultsDirectory); var dockerFilter = IncludeTestsRequiringDocker switch { diff --git a/tracer/build/_build/Build.cs b/tracer/build/_build/Build.cs index c8905f1030f3..cf923420a7e8 100644 --- a/tracer/build/_build/Build.cs +++ b/tracer/build/_build/Build.cs @@ -166,6 +166,16 @@ void DeleteReparsePoints(string path) } }); + Target CleanTestLogs => _ => _ + .Unlisted() + .Description("Cleans all test logs") + .Executes(() => + { + EnsureCleanDirectory(TestLogsDirectory); + ParallelIntegrationTests.ForEach(EnsureResultsDirectory); + ClrProfilerIntegrationTests.ForEach(EnsureResultsDirectory); + }); + Target CleanObjFiles => _ => _ .Unlisted() .Description("Deletes all build output files, but preserves folders to work around AzDo issues") From 7376508a0a4dc4443a5a8b538e74272989c95c87 Mon Sep 17 00:00:00 2001 From: Daniel Romano <108014683+daniel-romano-DD@users.noreply.github.com> Date: Tue, 1 Oct 2024 15:33:16 +0200 Subject: [PATCH 19/38] [IAST] Fix for VS Edit and Continue (#6097) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary of changes When VS enables Edit and Continue, it sets this env var `COMPLUS_ForceEnc=true` and some CallSite functions are not instrumented. ## Reason for change When `COMPLUS_ForceEnc=true` the profiler can skip the call to some Rejit events, leaving some functions (specially in callsite) uninstrumented ## Implementation details If this env var is detected, dataflow will set CallSite edited functionsIL in the JIT event as well as in the REJIT event, if this occurs. ## Test coverage Added integration tests ## Other details --- .../Datadog.Tracer.Native/cor_profiler.cpp | 2 +- .../src/Datadog.Tracer.Native/cor_profiler.h | 3 +- .../environment_variables.h | 6 ++ .../environment_variables_util.cpp | 13 +++ .../environment_variables_util.h | 1 + .../Datadog.Tracer.Native/iast/dataflow.cpp | 39 ++++--- .../src/Datadog.Tracer.Native/iast/dataflow.h | 3 +- .../AspNetBase.cs | 2 +- .../IAST/AspNetCore5IastTests.cs | 102 ++++++++++-------- 9 files changed, 111 insertions(+), 60 deletions(-) diff --git a/tracer/src/Datadog.Tracer.Native/cor_profiler.cpp b/tracer/src/Datadog.Tracer.Native/cor_profiler.cpp index 738a7324a620..9441009da13a 100644 --- a/tracer/src/Datadog.Tracer.Native/cor_profiler.cpp +++ b/tracer/src/Datadog.Tracer.Native/cor_profiler.cpp @@ -345,7 +345,7 @@ HRESULT STDMETHODCALLTYPE CorProfiler::Initialize(IUnknown* cor_profiler_info_un if (isIastEnabled || isRaspEnabled) { - _dataflow = new iast::Dataflow(info_, rejit_handler); + _dataflow = new iast::Dataflow(info_, rejit_handler, runtime_information_); if (FAILED(_dataflow->Init())) { Logger::Error("Callsite Dataflow failed to initialize"); diff --git a/tracer/src/Datadog.Tracer.Native/cor_profiler.h b/tracer/src/Datadog.Tracer.Native/cor_profiler.h index a59de0e629ce..99aad8f9fb51 100644 --- a/tracer/src/Datadog.Tracer.Native/cor_profiler.h +++ b/tracer/src/Datadog.Tracer.Native/cor_profiler.h @@ -170,10 +170,10 @@ class CorProfiler : public CorProfilerBase HRESULT STDMETHODCALLTYPE ProfilerDetachSucceeded() override; HRESULT STDMETHODCALLTYPE JITInlining(FunctionID callerId, FunctionID calleeId, BOOL* pfShouldInline) override; + // // ReJIT Methods // - HRESULT STDMETHODCALLTYPE GetReJITParameters(ModuleID moduleId, mdMethodDef methodId, ICorProfilerFunctionControl* pFunctionControl) override; @@ -251,7 +251,6 @@ class CorProfiler : public CorProfilerBase // // Getters for exception filter // - bool IsCallTargetBubbleUpExceptionTypeAvailable() const; bool IsCallTargetBubbleUpFunctionAvailable() const; }; diff --git a/tracer/src/Datadog.Tracer.Native/environment_variables.h b/tracer/src/Datadog.Tracer.Native/environment_variables.h index a89a8a7359b5..b977ca839aeb 100644 --- a/tracer/src/Datadog.Tracer.Native/environment_variables.h +++ b/tracer/src/Datadog.Tracer.Native/environment_variables.h @@ -127,6 +127,12 @@ namespace environment // Enables the workaround for dotnet issue 77973 (https://github.com/dotnet/runtime/issues/77973) const shared::WSTRING internal_workaround_77973_enabled = WStr("DD_INTERNAL_WORKAROUND_77973_ENABLED"); + // IDE Edit and Continue. If enabled, profiler behavior is modified slightly + const shared::WSTRING ide_edit_and_continue_core = WStr("COMPLUS_ForceEnc"); + + // IDE Edit and Continue. If enabled, profiler behavior is modified slightly + const shared::WSTRING ide_edit_and_continue_netfx = WStr("DOTNET_ForceEnc"); + } // namespace environment } // namespace trace diff --git a/tracer/src/Datadog.Tracer.Native/environment_variables_util.cpp b/tracer/src/Datadog.Tracer.Native/environment_variables_util.cpp index 3079949f6e24..cca86cdd2848 100644 --- a/tracer/src/Datadog.Tracer.Native/environment_variables_util.cpp +++ b/tracer/src/Datadog.Tracer.Native/environment_variables_util.cpp @@ -68,4 +68,17 @@ bool IsRaspEnabled() return IsRaspSettingEnabled() && IsAsmSettingEnabled(); } +bool IsEditAndContinueEnabledCore() +{ + ToBooleanWithDefault(shared::GetEnvironmentValue(environment::ide_edit_and_continue_core), false); +} +bool IsEditAndContinueEnabledNetFx() +{ + ToBooleanWithDefault(shared::GetEnvironmentValue(environment::ide_edit_and_continue_netfx), false); +} +bool IsEditAndContinueEnabled() +{ + return IsEditAndContinueEnabledCore() || IsEditAndContinueEnabledNetFx(); +} + } // namespace trace \ No newline at end of file diff --git a/tracer/src/Datadog.Tracer.Native/environment_variables_util.h b/tracer/src/Datadog.Tracer.Native/environment_variables_util.h index 10d7d567c663..e459932927d8 100644 --- a/tracer/src/Datadog.Tracer.Native/environment_variables_util.h +++ b/tracer/src/Datadog.Tracer.Native/environment_variables_util.h @@ -60,6 +60,7 @@ bool IsAzureFunctionsEnabled(); bool IsVersionCompatibilityEnabled(); bool IsIastEnabled(); bool IsRaspEnabled(); +bool IsEditAndContinueEnabled(); } // namespace trace diff --git a/tracer/src/Datadog.Tracer.Native/iast/dataflow.cpp b/tracer/src/Datadog.Tracer.Native/iast/dataflow.cpp index fb7ddbf92d87..c145459fb65f 100644 --- a/tracer/src/Datadog.Tracer.Native/iast/dataflow.cpp +++ b/tracer/src/Datadog.Tracer.Native/iast/dataflow.cpp @@ -10,6 +10,7 @@ #include #include #include "../../../../shared/src/native-src/com_ptr.h" +#include "../environment_variables_util.h" using namespace std::chrono; @@ -153,23 +154,26 @@ AspectFilter* ModuleAspects::GetFilter(DataflowAspectFilterValue filterValue) //-------------------- -Dataflow::Dataflow(ICorProfilerInfo* profiler, std::shared_ptr rejitHandler) : +Dataflow::Dataflow(ICorProfilerInfo* profiler, std::shared_ptr rejitHandler, + const RuntimeInformation& runtimeInfo) : Rejitter(rejitHandler, RejitterPriority::Low) { - HRESULT hr = profiler->QueryInterface(__uuidof(ICorProfilerInfo3), (void**) &_profiler); - if (_profiler != nullptr) + m_runtimeType = runtimeInfo.runtime_type; + m_runtimeVersion = VersionInfo{runtimeInfo.major_version, runtimeInfo.minor_version, runtimeInfo.build_version, 0}; + trace::Logger::Info("Dataflow::Dataflow -> Detected runtime version : ", m_runtimeVersion.ToString()); + + this->_setILOnJit = trace::IsEditAndContinueEnabled(); + if (this->_setILOnJit) { - USHORT major; - USHORT minor; - USHORT build; + trace::Logger::Info("Dataflow detected Edit and Continue feature (COMPLUS_ForceEnc != 0) : Enabling SetILCode in JIT event."); + } - if (SUCCEEDED(_profiler->GetRuntimeInformation(nullptr, &m_runtimeType, &major, &minor, &build, nullptr, 0, - nullptr, nullptr))) - { - m_runtimeVersion = VersionInfo{major, minor, build, 0}; - } + HRESULT hr = profiler->QueryInterface(__uuidof(ICorProfilerInfo3), (void**) &_profiler); + if (FAILED(hr)) + { + _profiler = nullptr; + trace::Logger::Error("Dataflow::Dataflow -> Something very wrong happened, as QI on ICorProfilerInfo3 failed. Disabling Dataflow. HRESULT : ", Hex(hr)); } - trace::Logger::Info("Dataflow::Dataflow -> Detected runtime version : ", m_runtimeVersion.ToString()); } Dataflow::~Dataflow() { @@ -182,6 +186,10 @@ HRESULT Dataflow::Init() { return S_FALSE; } + if (_profiler == nullptr) + { + return E_FAIL; + } HRESULT hr = S_OK; try { @@ -654,8 +662,9 @@ HRESULT Dataflow::RewriteMethod(MethodInfo* method, trace::FunctionControlWrappe } } } - if (!pFunctionControl && written) + if (!pFunctionControl && written && !_setILOnJit) { + // We are in JIT event. If _setILOnJit is false, we abort the commit and request a rejit context.aborted = true; } method->SetInstrumented(written); @@ -668,6 +677,10 @@ HRESULT Dataflow::RewriteMethod(MethodInfo* method, trace::FunctionControlWrappe { hr = method->ApplyFinalInstrumentation((ICorProfilerFunctionControl*) pFunctionControl); } + else if (_setILOnJit) + { + hr = method->ApplyFinalInstrumentation(); + } else { std::vector modulesVector = {module->_id}; diff --git a/tracer/src/Datadog.Tracer.Native/iast/dataflow.h b/tracer/src/Datadog.Tracer.Native/iast/dataflow.h index 804f00e40ee1..6ffe1c70bac2 100644 --- a/tracer/src/Datadog.Tracer.Native/iast/dataflow.h +++ b/tracer/src/Datadog.Tracer.Native/iast/dataflow.h @@ -52,7 +52,7 @@ namespace iast friend class ModuleInfo; friend class ModuleAspects; public: - Dataflow(ICorProfilerInfo* profiler, std::shared_ptr rejitHandler); + Dataflow(ICorProfilerInfo* profiler, std::shared_ptr rejitHandler, const RuntimeInformation& runtimeInfo); virtual ~Dataflow(); private: CS _cs; @@ -75,6 +75,7 @@ namespace iast protected: bool _initialized = false; bool _loaded = false; + bool _setILOnJit = false; std::vector _aspectClasses; std::vector _aspects; diff --git a/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetBase.cs b/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetBase.cs index 9f61a0912a99..7237367bece8 100644 --- a/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetBase.cs +++ b/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetBase.cs @@ -409,7 +409,7 @@ protected IImmutableList WaitForSpans(MockTracerAgent agent, int expec agent.SpanFilters.Add(s => s.Tags.ContainsKey("http.url") && s.Tags["http.url"].IndexOf(url, StringComparison.InvariantCultureIgnoreCase) > -1); } - var spans = agent.WaitForSpans(expectedSpans, minDateTime: minDateTime); + var spans = agent.WaitForSpans(expectedSpans, minDateTime: minDateTime, assertExpectedCount: false); if (spans.Count != expectedSpans) { Output?.WriteLine($"spans.Count: {spans.Count} != expectedSpans: {expectedSpans}, this is phase: {phase}"); diff --git a/tracer/test/Datadog.Trace.Security.IntegrationTests/IAST/AspNetCore5IastTests.cs b/tracer/test/Datadog.Trace.Security.IntegrationTests/IAST/AspNetCore5IastTests.cs index 65fa1d9eb3a6..e185ea999d85 100644 --- a/tracer/test/Datadog.Trace.Security.IntegrationTests/IAST/AspNetCore5IastTests.cs +++ b/tracer/test/Datadog.Trace.Security.IntegrationTests/IAST/AspNetCore5IastTests.cs @@ -350,7 +350,7 @@ await VerifyHelper.VerifySpans(spansFiltered, settings) } } -// Class to test particular features +// Classes to test particular features public class AspNetCore5IastTestsStackTraces : AspNetCore5IastTests { public AspNetCore5IastTestsStackTraces(AspNetCoreTestFixture fixture, ITestOutputHelper outputHelper) @@ -406,6 +406,65 @@ await VerifyHelper.VerifySpans(spans, settings) } } +public class AspNetCore5IastTestsCompatEditAndContinue : AspNetCore5IastTests +{ + public AspNetCore5IastTestsCompatEditAndContinue(AspNetCoreTestFixture fixture, ITestOutputHelper outputHelper) + : base(fixture, outputHelper, enableIast: true, testName: "AspNetCore5IastTestsCompat", samplingRate: 100, isIastDeduplicationEnabled: false, vulnerabilitiesPerRequest: 200, redactionEnabled: true) + { + SetEnvironmentVariable("COMPLUS_ForceEnc", "1"); + } + + [SkippableFact] + [Trait("RunOnWindows", "True")] + public async Task TestIastCustomSpanRequestManual() + { + var filename = "Iast.CustomManual.AspNetCore5." + (IastEnabled ? "IastEnabled" : "IastDisabled"); + if (RedactionEnabled is true) { filename += ".RedactionEnabled"; } + var url = "/Iast/CustomManual?userName=Vicent"; + IncludeAllHttpSpans = true; + await TryStartApp(); + var agent = Fixture.Agent; + var spans = await SendRequestsAsync(agent, 3, new string[] { url }); + var spansFiltered = spans.Where(s => !s.Resource.StartsWith("CREATE TABLE") && !s.Resource.StartsWith("INSERT INTO")).ToList(); + + var settings = VerifyHelper.GetSpanVerifierSettings(); + settings.AddIastScrubbing(); + await VerifyHelper.VerifySpans(spansFiltered, settings) + .UseFileName(filename) + .DisableRequireUniquePrefix(); + } +} + +public class AspNetCore5IastTestsCompat : AspNetCore5IastTestsCompatEditAndContinue +{ + public AspNetCore5IastTestsCompat(AspNetCoreTestFixture fixture, ITestOutputHelper outputHelper) + : base(fixture, outputHelper) + { + SetEnvironmentVariable("COMPLUS_ForceEnc", "0"); + } + + [SkippableFact] + [Trait("RunOnWindows", "True")] + public async Task TestIastCustomSpanRequestAttribute() + { + var filename = "Iast.CustomAttribute.AspNetCore5." + (IastEnabled ? "IastEnabled" : "IastDisabled"); + if (RedactionEnabled is true) { filename += ".RedactionEnabled"; } + var url = "/Iast/CustomAttribute?userName=Vicent"; + IncludeAllHttpSpans = true; + await TryStartApp(); + var agent = Fixture.Agent; + var spans = await SendRequestsAsync(agent, 2, new string[] { url }); + var spansFiltered = spans.Where(s => !s.Resource.StartsWith("CREATE TABLE") && !s.Resource.StartsWith("INSERT INTO")).ToList(); + + var settings = VerifyHelper.GetSpanVerifierSettings(); + settings.AddIastScrubbing(); + settings.AddRegexScrubber(new Regex(@"_dd.agent_psr: .{1,3},"), string.Empty); + await VerifyHelper.VerifySpans(spansFiltered, settings) + .UseFileName(filename) + .DisableRequireUniquePrefix(); + } +} + public class AspNetCore5IastTestsSpanTelemetryIastEnabled : AspNetCore5IastTests { public AspNetCore5IastTestsSpanTelemetryIastEnabled(AspNetCoreTestFixture fixture, ITestOutputHelper outputHelper) @@ -1169,47 +1228,6 @@ await VerifyHelper.VerifySpans(spansFiltered, settings) .DisableRequireUniquePrefix(); } - [SkippableFact] - [Trait("RunOnWindows", "True")] - public async Task TestIastCustomSpanRequestAttribute() - { - var filename = "Iast.CustomAttribute.AspNetCore5." + (IastEnabled ? "IastEnabled" : "IastDisabled"); - if (RedactionEnabled is true) { filename += ".RedactionEnabled"; } - var url = "/Iast/CustomAttribute?userName=Vicent"; - IncludeAllHttpSpans = true; - await TryStartApp(); - var agent = Fixture.Agent; - var spans = await SendRequestsAsync(agent, 3, new string[] { url }); - var spansFiltered = spans.Where(s => !s.Resource.StartsWith("CREATE TABLE") && !s.Resource.StartsWith("INSERT INTO")).ToList(); - - var settings = VerifyHelper.GetSpanVerifierSettings(); - settings.AddIastScrubbing(); - settings.AddRegexScrubber(new Regex(@"_dd.agent_psr: .{1,3},"), string.Empty); - await VerifyHelper.VerifySpans(spansFiltered, settings) - .UseFileName(filename) - .DisableRequireUniquePrefix(); - } - - [SkippableFact] - [Trait("RunOnWindows", "True")] - public async Task TestIastCustomSpanRequestManual() - { - var filename = "Iast.CustomManual.AspNetCore5." + (IastEnabled ? "IastEnabled" : "IastDisabled"); - if (RedactionEnabled is true) { filename += ".RedactionEnabled"; } - var url = "/Iast/CustomManual?userName=Vicent"; - IncludeAllHttpSpans = true; - await TryStartApp(); - var agent = Fixture.Agent; - var spans = await SendRequestsAsync(agent, 3, new string[] { url }); - var spansFiltered = spans.Where(s => !s.Resource.StartsWith("CREATE TABLE") && !s.Resource.StartsWith("INSERT INTO")).ToList(); - - var settings = VerifyHelper.GetSpanVerifierSettings(); - settings.AddIastScrubbing(); - await VerifyHelper.VerifySpans(spansFiltered, settings) - .UseFileName(filename) - .DisableRequireUniquePrefix(); - } - [SkippableFact] [Trait("RunOnWindows", "True")] public async Task TestNHibernateSqlInjection() From c0c88f4191c9bd6b9dbc437dd80ebf2b67cfe05a Mon Sep 17 00:00:00 2001 From: Steven Bouwkamp Date: Tue, 1 Oct 2024 11:00:45 -0400 Subject: [PATCH 20/38] Remove warning about non-serializable test data (#6102) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary of changes Removes warnings about non-serializable test data ``` Datadog.Trace.Tests: Non-serializable data ('System.Object[]') found for 'Datadog.Trace.Tests.Tagging.ActivityTagsTests.Tags_ShouldBe_PlacedInMetricsOrMeta'; falling back to single test case. ``` ## Reason for change Warnings about `Non-serializable data ('System.Object[]')`. ## Implementation details Ignored them by setting `DisableDiscoveryEnumeration = true` on `MemberData`. ## Test coverage Ran tests locally, warning gone. ## Other details Maybe I could've changed them to have serializable test data 🤷 --- .../Agent/MessagePack/SpanMetaStructTests.cs | 2 +- tracer/test/Datadog.Trace.Tests/Tagging/ActivityTagsTests.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tracer/test/Datadog.Trace.Tests/Agent/MessagePack/SpanMetaStructTests.cs b/tracer/test/Datadog.Trace.Tests/Agent/MessagePack/SpanMetaStructTests.cs index 7bba73810f66..8e0159570dcd 100644 --- a/tracer/test/Datadog.Trace.Tests/Agent/MessagePack/SpanMetaStructTests.cs +++ b/tracer/test/Datadog.Trace.Tests/Agent/MessagePack/SpanMetaStructTests.cs @@ -103,7 +103,7 @@ public class SpanMetaStructTests } }; - [MemberData(nameof(Data))] + [MemberData(nameof(Data), DisableDiscoveryEnumeration = true)] [Theory] public static void GivenAEncodedSpanWithMetaStruct_WhenDecoding_ThenMetaStructIsCorrectlyDecoded(List> dataToEncode) { diff --git a/tracer/test/Datadog.Trace.Tests/Tagging/ActivityTagsTests.cs b/tracer/test/Datadog.Trace.Tests/Tagging/ActivityTagsTests.cs index 30c3e326e59c..ecdedf06decd 100644 --- a/tracer/test/Datadog.Trace.Tests/Tagging/ActivityTagsTests.cs +++ b/tracer/test/Datadog.Trace.Tests/Tagging/ActivityTagsTests.cs @@ -70,7 +70,7 @@ public enum TagKind // TODO what about an array of object that contains string and numeric objects? [Theory] - [MemberData(nameof(TagData))] + [MemberData(nameof(TagData), DisableDiscoveryEnumeration = true)] public void Tags_ShouldBe_PlacedInMetricsOrMeta(string tagKey, object tagValue, TagKind expectedTagKind) { var activityMock = new Mock(); @@ -97,7 +97,7 @@ public void Tags_ShouldBe_PlacedInMetricsOrMeta(string tagKey, object tagValue, } [Theory] - [MemberData(nameof(ArrayTagData))] + [MemberData(nameof(ArrayTagData), DisableDiscoveryEnumeration = true)] public void ArrayedTags_ShouldBe_PlacedInMeta(string tagKey, object tagValue, TagKind expectedTagKind, Dictionary expectedTagValues) { var activityMock = new Mock(); From 1bf0873871c46d8bfa3339fe0b36d9624c4f254c Mon Sep 17 00:00:00 2001 From: Anna Date: Wed, 2 Oct 2024 10:07:24 +0200 Subject: [PATCH 21/38] [ASM][RCM] Read waf actions per file and remove per file (#6072) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary of changes [Read waf actions](https://datadoghq.atlassian.net/browse/APPSEC-54989) per file and remove per file ## Reason for change ## Implementation details ## Test coverage ## Other details --- .../Datadog.Trace/AppSec/Rcm/AsmProduct.cs | 21 +++---- .../AppSec/Rcm/ConfigurationStatus.cs | 7 ++- .../AppSec/Rcm/Models/Asm/Action.cs | 2 +- .../Waf/ReturnTypes.Managed/UpdateResult.cs | 11 +++- tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs | 11 +++- .../ActionChangeTests.cs | 61 ++++++------------- .../RASP/RaspWafTests.cs | 2 +- 7 files changed, 51 insertions(+), 64 deletions(-) diff --git a/tracer/src/Datadog.Trace/AppSec/Rcm/AsmProduct.cs b/tracer/src/Datadog.Trace/AppSec/Rcm/AsmProduct.cs index d8e47fd22183..1da14fab3190 100644 --- a/tracer/src/Datadog.Trace/AppSec/Rcm/AsmProduct.cs +++ b/tracer/src/Datadog.Trace/AppSec/Rcm/AsmProduct.cs @@ -46,20 +46,8 @@ public void ProcessUpdates(ConfigurationStatus configurationStatus, List CustomRulesByFile { get; } = new(); - internal IncomingUpdateStatus IncomingUpdateState { get; } = new(); + internal Dictionary ActionsByFile { get; init; } = new(); - internal IDictionary Actions { get; set; } = new Dictionary(); + internal IncomingUpdateStatus IncomingUpdateState { get; } = new(); internal static List MergeRuleData(IEnumerable res) { @@ -125,7 +125,8 @@ internal Dictionary BuildDictionaryForWafAccordingToIncomingUpda if (IncomingUpdateState.WafKeysToApply.Contains(WafActionsKey)) { - dictionary.Add(WafActionsKey, Actions.Select(a => a.Value.ToKeyValuePair()).ToArray()); + var actions = ActionsByFile.SelectMany(x => x.Value).ToList(); + dictionary.Add(WafActionsKey, actions.Select(r => r.ToKeyValuePair()).ToArray()); } if (IncomingUpdateState.WafKeysToApply.Contains(WafCustomRulesKey)) diff --git a/tracer/src/Datadog.Trace/AppSec/Rcm/Models/Asm/Action.cs b/tracer/src/Datadog.Trace/AppSec/Rcm/Models/Asm/Action.cs index edbfa3a09819..609440f04bee 100644 --- a/tracer/src/Datadog.Trace/AppSec/Rcm/Models/Asm/Action.cs +++ b/tracer/src/Datadog.Trace/AppSec/Rcm/Models/Asm/Action.cs @@ -19,5 +19,5 @@ internal class Action [JsonProperty("parameters")] public Parameter? Parameters { get; set; } - public List> ToKeyValuePair() => new() { new("type", Type), new("id", Id), new("parameters", Parameters?.ToKeyValuePair()) }; + public List> ToKeyValuePair() => [new("type", Type), new("id", Id), new("parameters", Parameters?.ToKeyValuePair())]; } diff --git a/tracer/src/Datadog.Trace/AppSec/Waf/ReturnTypes.Managed/UpdateResult.cs b/tracer/src/Datadog.Trace/AppSec/Waf/ReturnTypes.Managed/UpdateResult.cs index af78d9259d12..3069ea9587e6 100644 --- a/tracer/src/Datadog.Trace/AppSec/Waf/ReturnTypes.Managed/UpdateResult.cs +++ b/tracer/src/Datadog.Trace/AppSec/Waf/ReturnTypes.Managed/UpdateResult.cs @@ -13,7 +13,7 @@ namespace Datadog.Trace.AppSec.Waf.ReturnTypes.Managed { internal class UpdateResult { - internal UpdateResult(DdwafObjectStruct? diagObject, bool success, bool unusableRules = false) + private UpdateResult(DdwafObjectStruct? diagObject, bool success, bool unusableRules = false, bool nothingToUpdate = false) { if (diagObject != null) { @@ -33,12 +33,15 @@ internal UpdateResult(DdwafObjectStruct? diagObject, bool success, bool unusable Success = success; UnusableRules = unusableRules; + NothingToUpdate = nothingToUpdate; } internal bool Success { get; } internal bool UnusableRules { get; } + public bool NothingToUpdate { get; } + internal ushort? FailedToLoadRules { get; } /// @@ -56,6 +59,12 @@ internal UpdateResult(DdwafObjectStruct? diagObject, bool success, bool unusable public static UpdateResult FromUnusableRules() => new(null, false, true); + public static UpdateResult FromNothingToUpdate() => new(null, true, nothingToUpdate: true); + public static UpdateResult FromFailed() => new(null, false); + + public static UpdateResult FromFailed(DdwafObjectStruct diagObj) => new(diagObj, false); + + public static UpdateResult FromSuccess(DdwafObjectStruct diagObj) => new(diagObj, true); } } diff --git a/tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs b/tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs index 3f6ee8115eae..45ba8519b81f 100644 --- a/tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs +++ b/tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs @@ -15,6 +15,7 @@ using Datadog.Trace.AppSec.Waf.NativeBindings; using Datadog.Trace.AppSec.Waf.ReturnTypes.Managed; using Datadog.Trace.AppSec.WafEncoding; +using Datadog.Trace.ExtensionMethods; using Datadog.Trace.Logging; using Datadog.Trace.Telemetry; @@ -146,7 +147,7 @@ private unsafe UpdateResult UpdateWafAndDispose(IEncodeResult updateData) _wafHandle = newHandle; _wafLocker.ExitWriteLock(); _wafLibraryInvoker.Destroy(oldHandle); - return new(diagnosticsValue, true); + return UpdateResult.FromSuccess(diagnosticsValue); } _wafLibraryInvoker.Destroy(newHandle); @@ -158,7 +159,7 @@ private unsafe UpdateResult UpdateWafAndDispose(IEncodeResult updateData) } finally { - res ??= new(diagnosticsValue, false); + res ??= UpdateResult.FromFailed(diagnosticsValue); _wafLibraryInvoker.ObjectFree(ref diagnosticsValue); updateData.Dispose(); } @@ -169,6 +170,12 @@ private unsafe UpdateResult UpdateWafAndDispose(IEncodeResult updateData) public UpdateResult UpdateWafFromConfigurationStatus(ConfigurationStatus configurationStatus) { var dic = configurationStatus.BuildDictionaryForWafAccordingToIncomingUpdate(); + if (dic.IsEmpty()) + { + Log.Warning("A waf update came from remote configuration but final merged dictionary for waf is empty, no update will be performed."); + return UpdateResult.FromNothingToUpdate(); + } + return Update(dic); } diff --git a/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs b/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs index 7749acb2597d..7d60c91dfcd3 100644 --- a/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs +++ b/tracer/test/Datadog.Trace.Security.Unit.Tests/ActionChangeTests.cs @@ -20,12 +20,12 @@ namespace Datadog.Trace.Security.Unit.Tests; public class ActionChangeTests : WafLibraryRequiredTest { [Theory] - [InlineData("dummy_rule", "test-dummy-rule", "rasp-rule-set.json", "block", BlockingAction.BlockRequestType, 500)] - [InlineData("dummyrule2", "test-dummy-rule2", "rasp-rule-set.json", "customblock", BlockingAction.BlockRequestType, 500)] - // Redirect status code is restricted in newer versions of the WAF to 301, 302, 303, 307 - [InlineData("dummyrule2", "test-dummy-rule2", "rasp-rule-set.json", "customblock", BlockingAction.RedirectRequestType, 303)] - [InlineData("dummy_rule", "test-dummy-rule", "rasp-rule-set.json", "block", BlockingAction.RedirectRequestType, 303)] - public void GivenADummyRule_WhenActionReturnCodeIsChanged_ThenChangesAreApplied(string paramValue, string rule, string ruleFile, string action, string actionType, int newStatus) + [InlineData("dummy_rule", "test-dummy-rule", "block", BlockingAction.BlockRequestType, 500)] + [InlineData("dummyrule2", "test-dummy-rule2", "customblock", BlockingAction.BlockRequestType, 500)] + // Redirect status code is restricted in newer versions of the waf to 301, 302, 303, 307 + [InlineData("dummyrule2", "test-dummy-rule2", "customblock", BlockingAction.RedirectRequestType, 303)] + [InlineData("dummy_rule", "test-dummy-rule", "block", BlockingAction.RedirectRequestType, 303)] + public void GivenADummyRule_WhenActionReturnCodeIsChanged_ThenChangesAreApplied(string paramValue, string rule, string action, string actionType, int newStatus) { var args = CreateArgs(paramValue); var initResult = Waf.Create( @@ -33,7 +33,7 @@ public void GivenADummyRule_WhenActionReturnCodeIsChanged_ThenChangesAreApplied( string.Empty, string.Empty, useUnsafeEncoder: true, - embeddedRulesetPath: ruleFile); + embeddedRulesetPath: "rasp-rule-set.json"); var waf = initResult.Waf; waf.Should().NotBeNull(); @@ -44,13 +44,9 @@ public void GivenADummyRule_WhenActionReturnCodeIsChanged_ThenChangesAreApplied( var jsonString = JsonConvert.SerializeObject(result.Data); var resultData = JsonConvert.DeserializeObject(jsonString).FirstOrDefault(); resultData.Rule.Id.Should().Be(rule); - - ConfigurationStatus configurationStatus = new(string.Empty); var newAction = CreateNewStatusAction(action, actionType, newStatus); - configurationStatus.Actions[action] = newAction; - configurationStatus.IncomingUpdateState.WafKeysToApply.Add(ConfigurationStatus.WafActionsKey); - var res = waf.UpdateWafFromConfigurationStatus(configurationStatus); - res.Success.Should().BeTrue(); + UpdateWafWithActions([newAction], waf); + using var contextNew = waf.CreateContext(); result = contextNew.Run(args, TimeoutMicroSeconds); result.Timeout.Should().BeFalse("Timeout should be false"); @@ -65,22 +61,21 @@ public void GivenADummyRule_WhenActionReturnCodeIsChanged_ThenChangesAreApplied( } } - [Theory] - [InlineData("dummyrule2", "rasp-rule-set.json", "customblock", BlockingAction.BlockRequestType)] - public void GivenADummyRule_WhenActionReturnCodeIsChangedAfterInit_ThenChangesAreApplied(string paramValue, string ruleFile, string action, string actionType) + [Fact] + public void GivenADummyRule_WhenActionReturnCodeIsChangedAfterInit_ThenChangesAreApplied() { - var args = CreateArgs(paramValue); + var args = CreateArgs("dummyrule2"); var initResult = Waf.Create( WafLibraryInvoker, string.Empty, string.Empty, useUnsafeEncoder: true, - embeddedRulesetPath: ruleFile); + embeddedRulesetPath: "rasp-rule-set.json"); var waf = initResult.Waf; waf.Should().NotBeNull(); - UpdateAction(action, actionType, waf); + UpdateWafWithActions([CreateNewStatusAction("customblock", BlockingAction.BlockRequestType, 500)], waf); using var context = waf.CreateContext(); var result = context.Run(args, TimeoutMicroSeconds); @@ -88,26 +83,11 @@ public void GivenADummyRule_WhenActionReturnCodeIsChangedAfterInit_ThenChangesAr result.BlockInfo["status_code"].Should().Be("500"); } - private Dictionary CreateArgs(string requestParam) - { - return new Dictionary - { - { AddressesConstants.RequestUriRaw, "http://localhost:54587/" }, - { AddressesConstants.RequestBody, new[] { "param", requestParam } }, - { AddressesConstants.RequestMethod, "GET" } - }; - } + private Dictionary CreateArgs(string requestParam) => new() { { AddressesConstants.RequestUriRaw, "http://localhost:54587/" }, { AddressesConstants.RequestBody, new[] { "param", requestParam } }, { AddressesConstants.RequestMethod, "GET" } }; - private void UpdateAction(string action, string actionType, Waf waf) + private void UpdateWafWithActions(Action[] actions, Waf waf) { - ConfigurationStatus configurationStatus = new(string.Empty); - var newAction = new Action(); - newAction.Id = action; - newAction.Type = actionType; - newAction.Parameters = new AppSec.Rcm.Models.Asm.Parameter(); - newAction.Parameters.StatusCode = 500; - newAction.Parameters.Type = "auto"; - configurationStatus.Actions[action] = newAction; + ConfigurationStatus configurationStatus = new(string.Empty) { ActionsByFile = { ["file"] = actions } }; configurationStatus.IncomingUpdateState.WafKeysToApply.Add(ConfigurationStatus.WafActionsKey); var res = waf.UpdateWafFromConfigurationStatus(configurationStatus); res.Success.Should().BeTrue(); @@ -115,12 +95,7 @@ private void UpdateAction(string action, string actionType, Waf waf) private Action CreateNewStatusAction(string action, string actionType, int newStatus) { - var newAction = new Action(); - newAction.Id = action; - newAction.Type = actionType; - newAction.Parameters = new AppSec.Rcm.Models.Asm.Parameter(); - newAction.Parameters.StatusCode = newStatus; - newAction.Parameters.Type = "auto"; + var newAction = new Action { Id = action, Type = actionType, Parameters = new AppSec.Rcm.Models.Asm.Parameter { StatusCode = newStatus, Type = "auto" } }; if (newAction.Type == BlockingAction.RedirectRequestType) { diff --git a/tracer/test/Datadog.Trace.Security.Unit.Tests/RASP/RaspWafTests.cs b/tracer/test/Datadog.Trace.Security.Unit.Tests/RASP/RaspWafTests.cs index b6bb1ef30a7b..f30da4b0835f 100644 --- a/tracer/test/Datadog.Trace.Security.Unit.Tests/RASP/RaspWafTests.cs +++ b/tracer/test/Datadog.Trace.Security.Unit.Tests/RASP/RaspWafTests.cs @@ -64,7 +64,7 @@ public void GivenALfiRule_WhenActionReturnCodeIsChanged_ThenChangesAreApplied(st ConfigurationStatus configurationStatus = new(string.Empty); var newAction = CreateNewStatusAction(action, actionType, 500); - configurationStatus.Actions[action] = newAction; + configurationStatus.ActionsByFile[action] = [newAction]; configurationStatus.IncomingUpdateState.WafKeysToApply.Add(ConfigurationStatus.WafActionsKey); var res = waf.UpdateWafFromConfigurationStatus(configurationStatus); res.Success.Should().BeTrue(); From 758301fd29e0168c4af66c3454021b47255c8c78 Mon Sep 17 00:00:00 2001 From: Andrew Lock Date: Wed, 2 Oct 2024 12:43:25 +0100 Subject: [PATCH 22/38] Run CallTargetNativeTests on Windows (#5754) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary of changes Runs the `CallTargetNativeTests` on Windows ## Reason for change We weren't running these on Windows, and we should be ## Implementation details Add the `RunOnWindows` trait. Longer term I want to reverse the role of this trait, i.e. we run on windows by default and you add `RunOnWindows=False` to disable that. ## Test coverage More now ## Other details ~~I thought we had a bug related to ref structs here, but it appears to be a Schrödinger's bug~~ Oops, no, there it is! --- tracer/build/_build/Build.Steps.cs | 2 ++ .../CallTargetNativeTests.cs | 10 ++++++++++ .../test/Datadog.Trace.TestHelpers/ProfilerHelper.cs | 2 +- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/tracer/build/_build/Build.Steps.cs b/tracer/build/_build/Build.Steps.cs index 2d35789041bb..3128e948e21b 100644 --- a/tracer/build/_build/Build.Steps.cs +++ b/tracer/build/_build/Build.Steps.cs @@ -1452,6 +1452,7 @@ _ when path.Contains("dependency-libs") => false, { // This does some "unnecessary" rebuilding and restoring var includeIntegration = TracerDirectory.GlobFiles("test/test-applications/integrations/**/*.csproj"); + var includeInstrumentation = TracerDirectory.GlobFiles("test/test-applications/instrumentation/**/*.csproj"); // Don't build aspnet full framework sample in this step var includeSecurity = TracerDirectory.GlobFiles("test/test-applications/security/*/*.csproj"); @@ -1460,6 +1461,7 @@ _ when path.Contains("dependency-libs") => false, .Concat(TracerDirectory.GlobFiles("test/test-applications/integrations/Samples.AzureServiceBus/*.csproj")); var projects = includeIntegration + .Concat(includeInstrumentation) .Concat(includeSecurity) .Select(x => Solution.GetProject(x)) .Where(project => diff --git a/tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/CallTargetNativeTests.cs b/tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/CallTargetNativeTests.cs index 952f20dfbf23..210b9c2ae450 100644 --- a/tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/CallTargetNativeTests.cs +++ b/tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/CallTargetNativeTests.cs @@ -34,6 +34,7 @@ public static IEnumerable MethodArgumentsData() [SkippableTheory] [MemberData(nameof(MethodArgumentsData))] [Trait("SupportsInstrumentationVerification", "True")] + [Trait("RunOnWindows", "True")] public async Task MethodArgumentsInstrumentation(int numberOfArguments, bool fastPath) { SetInstrumentationVerification(); @@ -86,6 +87,7 @@ public async Task MethodArgumentsInstrumentation(int numberOfArguments, bool fas [SkippableFact] [Trait("SupportsInstrumentationVerification", "True")] + [Trait("RunOnWindows", "True")] public async Task MethodRefArguments() { SetInstrumentationVerification(); @@ -117,6 +119,7 @@ public async Task MethodRefArguments() [SkippableFact] [Trait("SupportsInstrumentationVerification", "True")] + [Trait("RunOnWindows", "True")] public async Task MethodOutArguments() { SetInstrumentationVerification(); @@ -147,6 +150,7 @@ public async Task MethodOutArguments() [SkippableFact] [Trait("SupportsInstrumentationVerification", "True")] + [Trait("RunOnWindows", "True")] public async Task MethodAbstract() { SetInstrumentationVerification(); @@ -177,6 +181,7 @@ public async Task MethodAbstract() } [SkippableFact] + [Trait("RunOnWindows", "True")] public async Task MethodInterface() { int agentPort = TcpPortProvider.GetOpenPort(); @@ -205,6 +210,7 @@ public async Task MethodInterface() [SkippableFact] [Trait("SupportsInstrumentationVerification", "True")] + [Trait("RunOnWindows", "True")] public async Task RemoveIntegrations() { SetInstrumentationVerification(); @@ -228,6 +234,7 @@ public async Task RemoveIntegrations() [SkippableFact] [Trait("SupportsInstrumentationVerification", "True")] + [Trait("RunOnWindows", "True")] public async Task ExtraIntegrations() { SetInstrumentationVerification(); @@ -248,6 +255,7 @@ public async Task ExtraIntegrations() [SkippableFact] [Trait("SupportsInstrumentationVerification", "True")] + [Trait("RunOnWindows", "True")] public async Task CallTargetBubbleUpExceptionIntegrations() { SetInstrumentationVerification(); @@ -262,6 +270,7 @@ public async Task CallTargetBubbleUpExceptionIntegrations() [SkippableFact] [Trait("SupportsInstrumentationVerification", "True")] + [Trait("RunOnWindows", "True")] public async Task CategorizedCallTargetIntegrations() { SetInstrumentationVerification(); @@ -283,6 +292,7 @@ public async Task CategorizedCallTargetIntegrations() [SkippableFact] [Trait("SupportsInstrumentationVerification", "True")] + [Trait("RunOnWindows", "True")] public async Task MethodRefStructArguments() { SetInstrumentationVerification(); diff --git a/tracer/test/Datadog.Trace.TestHelpers/ProfilerHelper.cs b/tracer/test/Datadog.Trace.TestHelpers/ProfilerHelper.cs index 0edf1fa3052d..6c0d4ab4f061 100644 --- a/tracer/test/Datadog.Trace.TestHelpers/ProfilerHelper.cs +++ b/tracer/test/Datadog.Trace.TestHelpers/ProfilerHelper.cs @@ -125,7 +125,7 @@ public static void SetCorFlags(string executable, ITestOutputHelper output, bool } output?.WriteLine($"Updating {Path.GetFileName(executable)} using {setBit}"); - var opts = new ProcessStartInfo(corFlagsExe, $"\"{executable}\" {setBit}") + var opts = new ProcessStartInfo(corFlagsExe, $"\"{executable}\" {setBit} /force") { CreateNoWindow = true, UseShellExecute = false From 6e7966ada7cc8de35398635cbe600b936b3c76ff Mon Sep 17 00:00:00 2001 From: NachoEchevarria <53266532+NachoEchevarria@users.noreply.github.com> Date: Wed, 2 Oct 2024 15:42:03 +0200 Subject: [PATCH 23/38] [ASM] Suspicious attacker blocking (#6057) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary of changes This PR adds the required code to support the suspicious attack functionality. It has added the required code to read the exclusion data from the RC. It also has modified the existing code regarding RC actions. Previously, if a configuration was received with an action, the configuration was stored and later sent to the WAF. If new values with an empty action array would come later, the previous action configurations would be deleted, but if a new array would come with a new action different than the previous one, we would report them both to the WAF. This behavior was making the suspicious attacker system tests fail because we would keep RC changes from previous tests. This change seems to be aligned with the behavior of other libraries. The file [AspNetBase.cs](https://github.com/DataDog/dd-trace-dotnet/pull/6057/files#diff-0faff2451113067d7669566ba9199908b720a3764914b00d6f33d4b376098d74) has been updated. Now, tests have more control over the used headers by allowing them to remove headers or replace previous values with new ones. ## Reason for change ## Implementation details ## Test coverage ## Other details --- .../AppSec/Rcm/AsmDataProduct.cs | 24 ++- .../AppSec/Rcm/ConfigurationStatus.cs | 9 ++ .../AppSec/Rcm/Models/AsmData/Payload.cs | 5 +- .../src/Datadog.Trace/AppSec/Waf/Context.cs | 3 +- tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs | 7 + .../AspNetBase.cs | 10 +- .../Rcm/AspNetCore5AsmAttackerBlocking.cs | 138 ++++++++++++++++++ 7 files changed, 188 insertions(+), 8 deletions(-) create mode 100644 tracer/test/Datadog.Trace.Security.IntegrationTests/Rcm/AspNetCore5AsmAttackerBlocking.cs diff --git a/tracer/src/Datadog.Trace/AppSec/Rcm/AsmDataProduct.cs b/tracer/src/Datadog.Trace/AppSec/Rcm/AsmDataProduct.cs index 66870e1bd7d9..20d737e8e4f6 100644 --- a/tracer/src/Datadog.Trace/AppSec/Rcm/AsmDataProduct.cs +++ b/tracer/src/Datadog.Trace/AppSec/Rcm/AsmDataProduct.cs @@ -1,4 +1,4 @@ -// +// // Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. // @@ -18,26 +18,40 @@ public void ProcessUpdates(ConfigurationStatus configurationStatus, List(); - var rulesData = asmDataConfig.TypedFile!.RulesData; + var rulesData = asmDataConfig.TypedFile?.RulesData; if (rulesData != null) { configurationStatus.RulesDataByFile[rawFile.Path.Path] = rulesData; configurationStatus.IncomingUpdateState.WafKeysToApply.Add(ConfigurationStatus.WafRulesDataKey); } + + var exclusionsData = asmDataConfig.TypedFile?.ExclusionsData; + if (exclusionsData != null) + { + configurationStatus.ExclusionsDataByFile[rawFile.Path.Path] = exclusionsData; + configurationStatus.IncomingUpdateState.WafKeysToApply.Add(ConfigurationStatus.WafExclusionsDataKey); + } } } public void ProcessRemovals(ConfigurationStatus configurationStatus, List removedConfigsForThisProduct) { - var removedData = false; + var removedRulesData = false; + var removedExclusionsData = false; foreach (var configurationPath in removedConfigsForThisProduct) { - removedData |= configurationStatus.RulesDataByFile.Remove(configurationPath.Path); + removedRulesData |= configurationStatus.RulesDataByFile.Remove(configurationPath.Path); + removedExclusionsData |= configurationStatus.ExclusionsDataByFile.Remove(configurationPath.Path); } - if (removedData) + if (removedRulesData) { configurationStatus.IncomingUpdateState.WafKeysToApply.Add(ConfigurationStatus.WafRulesDataKey); } + + if (removedExclusionsData) + { + configurationStatus.IncomingUpdateState.WafKeysToApply.Add(ConfigurationStatus.WafExclusionsDataKey); + } } } diff --git a/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs b/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs index 78cd40181b5e..2e4ce01d72f2 100644 --- a/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs +++ b/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs @@ -35,6 +35,7 @@ internal record ConfigurationStatus internal const string WafRulesOverridesKey = "rules_override"; internal const string WafExclusionsKey = "exclusions"; internal const string WafRulesDataKey = "rules_data"; + internal const string WafExclusionsDataKey = "exclusion_data"; internal const string WafCustomRulesKey = "custom_rules"; internal const string WafActionsKey = "actions"; private readonly IAsmConfigUpdater _asmFeatureProduct = new AsmFeaturesProduct(); @@ -57,6 +58,8 @@ internal record ConfigurationStatus internal Dictionary RulesDataByFile { get; } = new(); + internal Dictionary ExclusionsDataByFile { get; } = new(); + internal Dictionary ExclusionsByFile { get; } = new(); internal Dictionary RulesByFile { get; } = new(); @@ -123,6 +126,12 @@ internal Dictionary BuildDictionaryForWafAccordingToIncomingUpda dictionary.Add(WafRulesDataKey, rulesData.Select(r => r.ToKeyValuePair()).ToArray()); } + if (IncomingUpdateState.WafKeysToApply.Contains(WafExclusionsDataKey)) + { + var rulesData = MergeRuleData(ExclusionsDataByFile.SelectMany(x => x.Value)); + dictionary.Add(WafExclusionsDataKey, rulesData.Select(r => r.ToKeyValuePair()).ToArray()); + } + if (IncomingUpdateState.WafKeysToApply.Contains(WafActionsKey)) { var actions = ActionsByFile.SelectMany(x => x.Value).ToList(); diff --git a/tracer/src/Datadog.Trace/AppSec/Rcm/Models/AsmData/Payload.cs b/tracer/src/Datadog.Trace/AppSec/Rcm/Models/AsmData/Payload.cs index 21c8aabf0c72..79b738d87a1b 100644 --- a/tracer/src/Datadog.Trace/AppSec/Rcm/Models/AsmData/Payload.cs +++ b/tracer/src/Datadog.Trace/AppSec/Rcm/Models/AsmData/Payload.cs @@ -1,4 +1,4 @@ -// +// // Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. // @@ -11,4 +11,7 @@ internal class Payload { [JsonProperty("rules_data")] public RuleData[]? RulesData { get; set; } + + [JsonProperty("exclusion_data")] + public RuleData[]? ExclusionsData { get; set; } } diff --git a/tracer/src/Datadog.Trace/AppSec/Waf/Context.cs b/tracer/src/Datadog.Trace/AppSec/Waf/Context.cs index 2a72d96b891d..ad6c93eb12ee 100644 --- a/tracer/src/Datadog.Trace/AppSec/Waf/Context.cs +++ b/tracer/src/Datadog.Trace/AppSec/Waf/Context.cs @@ -140,8 +140,9 @@ private Context(IntPtr contextHandle, Waf waf, WafLibraryInvoker wafLibraryInvok if (Log.IsEnabled(LogEventLevel.Debug)) { Log.Debug( - "DDAS-0011-00: AppSec In-App WAF returned: {ReturnCode} {Data}", + "DDAS-0011-00: AppSec In-App WAF returned: {ReturnCode} {BlockInfo} {Data}", result.ReturnCode, + result.BlockInfo, result.Data); } diff --git a/tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs b/tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs index 45ba8519b81f..1f0be2ffb0f6 100644 --- a/tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs +++ b/tracer/src/Datadog.Trace/AppSec/Waf/Waf.cs @@ -18,6 +18,8 @@ using Datadog.Trace.ExtensionMethods; using Datadog.Trace.Logging; using Datadog.Trace.Telemetry; +using Datadog.Trace.Vendors.Newtonsoft.Json; +using Datadog.Trace.Vendors.Serilog.Events; namespace Datadog.Trace.AppSec.Waf { @@ -218,6 +220,11 @@ private UpdateResult Update(IDictionary arguments) UpdateResult updated; try { + if (Log.IsEnabled(LogEventLevel.Debug)) + { + Log.Debug("Updating WAF with new configuration: {Arguments}", JsonConvert.SerializeObject(arguments)); + } + var encodedArgs = _encoder.Encode(arguments, applySafetyLimits: false); updated = UpdateWafAndDispose(encodedArgs); diff --git a/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetBase.cs b/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetBase.cs index 7237367bece8..5d2112756a28 100644 --- a/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetBase.cs +++ b/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetBase.cs @@ -372,7 +372,15 @@ protected async Task TestRateLimiter(bool enableSecurity, string url, MockTracer { foreach (var header in headers) { - _httpClient.DefaultRequestHeaders.Add(header.Key, header.Value); + if (_httpClient.DefaultRequestHeaders.Contains(header.Key)) + { + _httpClient.DefaultRequestHeaders.Remove(header.Key); + } + + if (header.Value is not null) + { + _httpClient.DefaultRequestHeaders.Add(header.Key, header.Value); + } } } diff --git a/tracer/test/Datadog.Trace.Security.IntegrationTests/Rcm/AspNetCore5AsmAttackerBlocking.cs b/tracer/test/Datadog.Trace.Security.IntegrationTests/Rcm/AspNetCore5AsmAttackerBlocking.cs new file mode 100644 index 000000000000..d7e820a7c089 --- /dev/null +++ b/tracer/test/Datadog.Trace.Security.IntegrationTests/Rcm/AspNetCore5AsmAttackerBlocking.cs @@ -0,0 +1,138 @@ +// +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. +// + +#if NETCOREAPP3_0_OR_GREATER + +using System; +using System.Collections.Generic; +using System.Net; +using System.Threading.Tasks; +using Datadog.Trace.AppSec; +using Datadog.Trace.AppSec.Rcm.Models.AsmFeatures; +using Datadog.Trace.Configuration; +using Datadog.Trace.TestHelpers; +using Datadog.Trace.Vendors.Newtonsoft.Json.Linq; +using FluentAssertions; +using Xunit; +using Xunit.Abstractions; +using Action = Datadog.Trace.AppSec.Rcm.Models.Asm.Action; + +namespace Datadog.Trace.Security.IntegrationTests.Rcm; + +public class AspNetCore5AsmAttackerBlocking : RcmBase +{ + private const string AsmProduct = "ASM"; + + public AspNetCore5AsmAttackerBlocking(AspNetCoreTestFixture fixture, ITestOutputHelper outputHelper) + : base(fixture, outputHelper, enableSecurity: true, testName: nameof(AspNetCore5AsmAttackerBlocking)) + { + SetEnvironmentVariable(ConfigurationKeys.DebugEnabled, "0"); + SetEnvironmentVariable("DD_APPSEC_WAF_DEBUG", "0"); + } + + [Fact] + [Trait("RunOnWindows", "True")] + public async Task TestSuspiciousAttackerBlocking() + { + List> headersAttacker = new() + { + new KeyValuePair("http.client_ip", "34.65.27.85"), + new KeyValuePair("X-Real-Ip", "34.65.27.85"), + new KeyValuePair("accept-encoding", "identity"), + new KeyValuePair("x-forwarded-for", null), + }; + + List> headersRegular = new() + { + new KeyValuePair("X-Real-Ip", null), + new KeyValuePair("accept-encoding", "identity"), + new KeyValuePair("x-forwarded-for", null), + }; + + var headersAttackerArachni = new List>(headersAttacker) + { + new KeyValuePair("User-Agent", "Arachni/v1"), + }; + + var headersRegularArachni = new List>(headersRegular) + { + new KeyValuePair("User-Agent", "Arachni/v1"), + }; + + var headersAttackerScanner = new List>(headersAttacker) + { + new KeyValuePair("User-Agent", "dd-test-scanner-log-block"), + }; + + var headersRegularScanner = new List>(headersRegular) + { + new KeyValuePair("User-Agent", "dd-test-scanner-log-block"), + }; + + string url = "/Health"; + IncludeAllHttpSpans = true; + await TryStartApp(); + var agent = Fixture.Agent; + var result = SubmitRequest(url, null, null, headers: headersAttackerScanner); + result.Result.StatusCode.Should().Be(HttpStatusCode.Forbidden); + + var configurationInitial = new[] + { + ((object)new AppSec.Rcm.Models.Asm.Payload + { + Actions = new[] + { + new Action { Id = "block", Type = BlockingAction.BlockRequestType, Parameters = new AppSec.Rcm.Models.Asm.Parameter { StatusCode = 403, Type = "json" } } + }, + }, + AsmProduct, + nameof(TestSuspiciousAttackerBlocking)), + ((object)new AsmFeatures + { + Asm = new AsmFeature { Enabled = true }, + }, + "ASM_FEATURES", + nameof(TestSuspiciousAttackerBlocking)) + }; + + await agent.SetupRcmAndWait(Output, configurationInitial); + result = SubmitRequest(url, null, null, headers: headersAttackerScanner); + + var exclusions = "[{\"id\": \"exc-000-001\",\"on_match\": \"block_custom\",\"conditions\": [{\"operator\": \"ip_match\",\"parameters\": {\"data\": \"suspicious_ips_data_id\", \"inputs\": [{\"address\": \"http.client_ip\"}]}}]}]"; + var configuration = new[] + { + (new AppSec.Rcm.Models.Asm.Payload + { + Actions = new[] + { + new Action { Id = "block_custom", Type = BlockingAction.BlockRequestType, Parameters = new AppSec.Rcm.Models.Asm.Parameter { StatusCode = 405, Type = "auto" } } + }, + Exclusions = (JArray)JToken.Parse(exclusions) + }, + AsmProduct, + nameof(TestSuspiciousAttackerBlocking)), + ((object)new AppSec.Rcm.Models.AsmData.Payload + { + ExclusionsData = new[] + { + new AppSec.Rcm.Models.AsmData.RuleData { Id = "suspicious_ips_data_id", Type = "ip_with_expiration", Data = new AppSec.Rcm.Models.AsmData.Data[] { new() { Value = "34.65.27.85" } } } + }, + }, + "ASM_DATA", + nameof(TestSuspiciousAttackerBlocking)), + }; + + await agent.SetupRcmAndWait(Output, configuration); + result = SubmitRequest(url + "?a=3", null, null, headers: headersAttackerScanner); + result.Result.StatusCode.Should().Be(HttpStatusCode.MethodNotAllowed); + result = SubmitRequest(url + "?a=4", null, null, headers: headersRegularScanner); + result.Result.StatusCode.Should().Be(HttpStatusCode.Forbidden); + result = SubmitRequest(url + "?a=5", null, null, headers: headersAttackerArachni); + result.Result.StatusCode.Should().Be(HttpStatusCode.MethodNotAllowed); + result = SubmitRequest(url + "?a=6", null, null, headers: headersRegularArachni); + result.Result.StatusCode.Should().Be(HttpStatusCode.OK); + } +} +#endif From 2b5c0dc1277b6df59e33483a90b804728f2fff34 Mon Sep 17 00:00:00 2001 From: William Conti <58711692+wconti27@users.noreply.github.com> Date: Wed, 2 Oct 2024 13:59:15 -0400 Subject: [PATCH 24/38] chore: prefix system tests env vars (#6108) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary of changes Prefix system tests env vars to not conflict with local developer env vars ## Reason for change ## Implementation details ## Test coverage ## Other details --- .azure-pipelines/ultimate-pipeline.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.azure-pipelines/ultimate-pipeline.yml b/.azure-pipelines/ultimate-pipeline.yml index 6e567964e1a9..fc60ccc73798 100644 --- a/.azure-pipelines/ultimate-pipeline.yml +++ b/.azure-pipelines/ultimate-pipeline.yml @@ -5070,10 +5070,8 @@ stages: env: DD_API_KEY: $(SYSTEM_TESTS_DD_API_KEY) TEST_LIBRARY: dotnet - AWS_ACCESS_KEY_ID: $(SYSTEM_TESTS_IDM_AWS_ACCESS_KEY_ID) - AWS_SECRET_ACCESS_KEY: $(SYSTEM_TESTS_IDM_AWS_SECRET_ACCESS_KEY) - AWS_REGION: us-east-1 - AWS_DEFAULT_REGION: us-east-1 # AWS services should use `AWS_REGION`, but some still use the older `AWS_DEFAULT_REGION` + SYSTEM_TESTS_AWS_ACCESS_KEY_ID: $(SYSTEM_TESTS_IDM_AWS_ACCESS_KEY_ID) + SYSTEM_TESTS_AWS_SECRET_ACCESS_KEY: $(SYSTEM_TESTS_IDM_AWS_SECRET_ACCESS_KEY) - script: | mkdir -p all_logs From 6f1be329124d7e8ff1ef9eee44e1313b16f269ef Mon Sep 17 00:00:00 2001 From: Kevin Gosse Date: Thu, 3 Oct 2024 12:23:05 +0200 Subject: [PATCH 25/38] [Crashtracking] Implement support for Windows (#6088) ## Summary of changes Add crashtracking support for Windows. At the moment, only x64 is supported. Supporting x86 would require to include a 32-bit version of `dd-dotnet` in the package. ## Reason for change Until now, crashtracking only supported Linux. There are many crashes (due to either netfx or IIS) that only occur on Windows. ## Implementation details Crashtracking on Windows relies on [WER](https://learn.microsoft.com/en-us/windows/win32/wer/windows-error-reporting). When the native loader is loaded, we use `WerRegisterRuntimeExceptionModule` to register it as a crash handler. Note that .NET also registers a crash handler, and they're invoked by order of registration. To be invoked first, we unregister the .NET crash handler, register our own, then re-register the .NET crash handler. The .NET crash handler is in the DAC (mscordawks.dll for .NET Framework, mscordaccore.dll for .NET Core). The context argument is set to the address of `clr.dll`/`coreclr.dll` in memory (this is important because `WerUnregisterRuntimeExceptionModule` fails if the context value is different). When a crash occurs, Windows freezes the process and launches `WerFault.exe`. In WerFault, the crash handler is loaded and given a chance to inspect the crash (using the exported `OutOfProcessExceptionEventCallback` function). We invoke `dd-dotnet` to inspect the process and generate the crash report. The environment variables of the crashing process are not propagated to `WerFault.exe`. Because we need them (for instance, to know the location of the agent), we make a copy of all the `DD_*` environment variables and give the address of that copy as the "context" argument of `WerRegisterRuntimeExceptionModule`. When `OutOfProcessExceptionEventCallback` is invoked in `WerFault.exe`, we use `ReadProcessMemory` to read the saved environment variables from the crashing process, and set them for `dd-dotnet`. ## Test coverage Reusing the existing crashtracking tests. ## Other details More information on WER: https://minidump.net/windows-error-reporting/ --- .../CrashReportingLinux.cpp | 43 -- .../CrashReportingLinux.h | 1 - .../CrashReportingWindows.cpp | 256 +++++++++++ .../CrashReportingWindows.h | 32 ++ .../Datadog.Profiler.Native.Windows.vcxproj | 4 +- ...og.Profiler.Native.Windows.vcxproj.filters | 6 + .../Datadog.Profiler.Native.def | 3 +- .../CrashReporting.cpp | 47 +- .../Datadog.Profiler.Native/CrashReporting.h | 1 + .../Datadog.Trace.ClrProfiler.Native.vcxproj | 2 + ...g.Trace.ClrProfiler.Native.vcxproj.filters | 21 + .../crashhandler.cpp | 424 ++++++++++++++++++ .../crashhandler.h | 29 ++ .../dllmain.cpp | 115 +++-- tracer/build/_build/Build.Steps.cs | 1 + .../CreatedumpCommand.cs | 7 +- .../MockTracerAgent.cs | 28 +- .../ConsoleTestHelper.cs | 20 +- .../CreatedumpTests.cs | 151 +++++-- .../integrations/Samples.Console/Program.cs | 39 +- 20 files changed, 1076 insertions(+), 154 deletions(-) create mode 100644 profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/CrashReportingWindows.cpp create mode 100644 profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/CrashReportingWindows.h create mode 100644 shared/src/Datadog.Trace.ClrProfiler.Native/crashhandler.cpp create mode 100644 shared/src/Datadog.Trace.ClrProfiler.Native/crashhandler.h diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/CrashReportingLinux.cpp b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/CrashReportingLinux.cpp index 970b007dd3ad..7f3fb4254426 100644 --- a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/CrashReportingLinux.cpp +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/CrashReportingLinux.cpp @@ -266,49 +266,6 @@ std::vector CrashReportingLinux::GetThreadFrames(int32_t tid, Resolv return MergeFrames(frames, managedFrames); } -std::vector CrashReportingLinux::MergeFrames(const std::vector& nativeFrames, const std::vector& managedFrames) -{ - std::vector result; - result.reserve(std::max(nativeFrames.size(), managedFrames.size())); - - size_t i = 0, j = 0; - while (i < nativeFrames.size() && j < managedFrames.size()) - { - if (nativeFrames.at(i).sp < managedFrames.at(j).sp) - { - result.push_back(nativeFrames.at(i)); - ++i; - } - else if (managedFrames.at(j).sp < nativeFrames.at(i).sp) - { - result.push_back(managedFrames.at(j)); - ++j; - } - else - { // frames[i].sp == managedFrames[j].sp - // Prefer managedFrame when sp values are the same - result.push_back(managedFrames.at(j)); - ++i; - ++j; - } - } - - // Add any remaining frames that are left in either vector - while (i < nativeFrames.size()) - { - result.push_back(nativeFrames.at(i)); - ++i; - } - - while (j < managedFrames.size()) - { - result.push_back(managedFrames.at(j)); - ++j; - } - - return result; -} - std::string CrashReportingLinux::GetSignalInfo(int32_t signal) { auto signalInfo = strsignal(signal); diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/CrashReportingLinux.h b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/CrashReportingLinux.h index eace98610a74..39a2370a7f40 100644 --- a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/CrashReportingLinux.h +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/CrashReportingLinux.h @@ -31,7 +31,6 @@ class CrashReportingLinux : public CrashReporting std::pair FindModule(uintptr_t ip); std::vector GetModules(); std::string GetSignalInfo(int32_t signal) override; - std::vector MergeFrames(const std::vector& nativeFrames, const std::vector& managedFrames); std::string GetThreadName(int32_t tid); unw_addr_space_t _addressSpace; diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/CrashReportingWindows.cpp b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/CrashReportingWindows.cpp new file mode 100644 index 000000000000..3369325c56e0 --- /dev/null +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/CrashReportingWindows.cpp @@ -0,0 +1,256 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2022 Datadog, Inc. + +#include "windows.h" + +#include "CrashReportingWindows.h" +#include "TlHelp32.h" +#include "DbgHelp.h" +#include "Psapi.h" + +#include +#include + +#pragma comment(lib, "dbghelp.lib") + +CrashReporting* CrashReporting::Create(int32_t pid) +{ + auto crashReporting = new CrashReportingWindows(pid); + return (CrashReporting*)crashReporting; +} + +CrashReportingWindows::CrashReportingWindows(int32_t pid) + : CrashReporting(pid) + , _process(NULL) +{ +} + +CrashReportingWindows::~CrashReportingWindows() +{ + CloseHandle(_process); +} + +int32_t CrashReportingWindows::Initialize() +{ + auto result = CrashReporting::Initialize(); + + if (result == 0) + { + _process = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, _pid); + + if (_process == NULL) + { + return 1; + } + + SymInitialize(_process, nullptr, TRUE); + + _modules = GetModules(); + } + + return result; +} + +std::vector> CrashReportingWindows::GetThreads() +{ + std::vector> threads; + + auto threadSnapshot = CreateToolhelp32Snapshot(TH32CS_SNAPTHREAD, _pid); + + if (threadSnapshot == INVALID_HANDLE_VALUE) + { + return threads; + } + + THREADENTRY32 threadEntry = {}; + threadEntry.dwSize = sizeof(THREADENTRY32); + + if (Thread32First(threadSnapshot, &threadEntry)) + { + do + { + if (threadEntry.th32OwnerProcessID == _pid) + { + auto thread = OpenThread(THREAD_QUERY_INFORMATION, FALSE, threadEntry.th32ThreadID); + + if (thread) + { + std::string threadName; + PWSTR description; + + if (SUCCEEDED(GetThreadDescription(thread, &description))) + { + threadName = shared::ToString(description); + } + + threads.push_back({ threadEntry.th32ThreadID, threadName }); + + CloseHandle(thread); + } + } + } while (Thread32Next(threadSnapshot, &threadEntry)); + } + + CloseHandle(threadSnapshot); + + return threads; +} + +std::vector CrashReportingWindows::GetThreadFrames(int32_t tid, ResolveManagedCallstack resolveManagedCallstack, void* callbackContext) +{ + std::vector frames; + + // Get the managed callstack + ResolveMethodData* managedCallstack; + int32_t numberOfManagedFrames; + + auto resolved = resolveManagedCallstack(tid, callbackContext, &managedCallstack, &numberOfManagedFrames); + + std::vector managedFrames; + + if (resolved == 0 && numberOfManagedFrames > 0) + { + managedFrames.reserve(numberOfManagedFrames); + + for (int i = 0; i < numberOfManagedFrames; i++) + { + auto const& managedFrame = managedCallstack[i]; + + StackFrame stackFrame; + stackFrame.ip = managedFrame.ip; + stackFrame.sp = managedFrame.sp; + stackFrame.method = std::string(managedFrame.symbolName); + stackFrame.moduleAddress = managedFrame.moduleAddress; + stackFrame.symbolAddress = managedFrame.symbolAddress; + stackFrame.isSuspicious = managedFrame.isSuspicious; + + managedFrames.push_back(std::move(stackFrame)); + } + } + + CONTEXT context = {}; + context.ContextFlags = CONTEXT_FULL; + + auto thread = OpenThread(THREAD_GET_CONTEXT, FALSE, tid); + + if (thread == NULL) + { + return managedFrames; + } + + if (GetThreadContext(thread, &context)) + { + STACKFRAME_EX nativeStackFrame = {}; +#ifdef _M_X64 + int machineType = IMAGE_FILE_MACHINE_AMD64; + nativeStackFrame.AddrPC.Offset = context.Rip; + nativeStackFrame.AddrPC.Mode = AddrModeFlat; + nativeStackFrame.AddrFrame.Offset = context.Rsp; + nativeStackFrame.AddrFrame.Mode = AddrModeFlat; + nativeStackFrame.AddrStack.Offset = context.Rsp; + nativeStackFrame.AddrStack.Mode = AddrModeFlat; +#elif _M_IX86 + int machineType = IMAGE_FILE_MACHINE_I386; + nativeStackFrame.AddrPC.Offset = context.Eip; + nativeStackFrame.AddrPC.Mode = AddrModeFlat; + nativeStackFrame.AddrFrame.Offset = context.Ebp; + nativeStackFrame.AddrFrame.Mode = AddrModeFlat; + nativeStackFrame.AddrStack.Offset = context.Esp; + nativeStackFrame.AddrStack.Mode = AddrModeFlat; +#endif + + while (StackWalkEx(machineType, + _process, + thread, + &nativeStackFrame, + &context, + nullptr, + nullptr, + nullptr, + nullptr, + SYM_STKWALK_DEFAULT)) + { + auto module = FindModule(nativeStackFrame.AddrPC.Offset); + + StackFrame stackFrame; + stackFrame.ip = nativeStackFrame.AddrPC.Offset; + stackFrame.sp = nativeStackFrame.AddrStack.Offset; + stackFrame.isSuspicious = false; + stackFrame.moduleAddress = module.second; + stackFrame.symbolAddress = nativeStackFrame.AddrPC.Offset; + + std::ostringstream methodName; + methodName << module.first << "!+" << std::hex << (nativeStackFrame.AddrPC.Offset - module.second); + stackFrame.method = methodName.str(); + + std::filesystem::path modulePath(module.first); + + if (modulePath.has_filename()) + { + const auto moduleFilename = modulePath.stem().string(); + + if (moduleFilename.rfind("Datadog", 0) == 0 + || moduleFilename == "libdatadog" + || moduleFilename == "datadog" + || moduleFilename == "libddwaf" + || moduleFilename == "ddwaf") + { + stackFrame.isSuspicious = true; + } + } + + frames.push_back(std::move(stackFrame)); + } + } + + CloseHandle(thread); + + return MergeFrames(frames, managedFrames); +} + +std::string CrashReportingWindows::GetSignalInfo(int32_t signal) +{ + return std::string(); +} + +std::vector CrashReportingWindows::GetModules() +{ + std::vector modules; + + HMODULE hModules[1024]; + DWORD cbNeeded; + if (EnumProcessModules(_process, hModules, sizeof(hModules), &cbNeeded)) + { + for (unsigned int i = 0; i < (cbNeeded / sizeof(HMODULE)); ++i) + { + MODULEINFO moduleInfo = {}; + if (GetModuleInformation(_process, hModules[i], &moduleInfo, sizeof(moduleInfo))) + { + std::string resolvedModuleName = ""; + + char moduleName[MAX_PATH]; + if (GetModuleFileNameExA(_process, hModules[i], moduleName, sizeof(moduleName))) + { + resolvedModuleName = moduleName; + } + + modules.push_back({ (uintptr_t)moduleInfo.lpBaseOfDll, (uintptr_t)moduleInfo.lpBaseOfDll + moduleInfo.SizeOfImage, resolvedModuleName }); + } + } + } + + return modules; +} + +std::pair CrashReportingWindows::FindModule(uintptr_t ip) +{ + for (auto& module : _modules) + { + if (ip >= module.startAddress && ip < module.endAddress) + { + return std::make_pair(module.path, module.startAddress); + } + } + + return std::make_pair("", 0); +} diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/CrashReportingWindows.h b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/CrashReportingWindows.h new file mode 100644 index 000000000000..1ba17891bcdb --- /dev/null +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/CrashReportingWindows.h @@ -0,0 +1,32 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2022 Datadog, Inc. + +#pragma once +#include "CrashReporting.h" + +struct ModuleInfo +{ + uintptr_t startAddress; + uintptr_t endAddress; + std::string path; +}; + +class CrashReportingWindows : public CrashReporting +{ +public: + CrashReportingWindows(int32_t pid); + + ~CrashReportingWindows() override; + + int32_t STDMETHODCALLTYPE Initialize() override; + +private: + std::vector> GetThreads() override; + std::vector GetThreadFrames(int32_t tid, ResolveManagedCallstack resolveManagedCallstack, void* context) override; + std::string GetSignalInfo(int32_t signal) override; + std::vector GetModules(); + std::pair FindModule(uintptr_t ip); + + HANDLE _process; + std::vector _modules; +}; diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/Datadog.Profiler.Native.Windows.vcxproj b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/Datadog.Profiler.Native.Windows.vcxproj index 829fc0c3a059..fd04118092bb 100644 --- a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/Datadog.Profiler.Native.Windows.vcxproj +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/Datadog.Profiler.Native.Windows.vcxproj @@ -219,6 +219,7 @@ + @@ -236,6 +237,7 @@ + @@ -269,4 +271,4 @@ - + \ No newline at end of file diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/Datadog.Profiler.Native.Windows.vcxproj.filters b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/Datadog.Profiler.Native.Windows.vcxproj.filters index bdfe1208172a..f44652c84aec 100644 --- a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/Datadog.Profiler.Native.Windows.vcxproj.filters +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/Datadog.Profiler.Native.Windows.vcxproj.filters @@ -65,6 +65,9 @@ ETW + + Header Files + @@ -106,6 +109,9 @@ ETW + + Source Files + diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/Datadog.Profiler.Native.def b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/Datadog.Profiler.Native.def index 68a2870edc62..bbe0d7c6a653 100644 --- a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/Datadog.Profiler.Native.def +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/Datadog.Profiler.Native.def @@ -9,4 +9,5 @@ SetApplicationInfoForAppDomain PRIVATE SetEndpointForTrace PRIVATE SetGitMetadataForApplication PRIVATE - FlushProfile PRIVATE \ No newline at end of file + FlushProfile PRIVATE + CreateCrashReport PRIVATE \ No newline at end of file diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native/CrashReporting.cpp b/profiler/src/ProfilerEngine/Datadog.Profiler.Native/CrashReporting.cpp index 4f220c6f0cd4..33b2428a01f3 100644 --- a/profiler/src/ProfilerEngine/Datadog.Profiler.Native/CrashReporting.cpp +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native/CrashReporting.cpp @@ -18,13 +18,9 @@ extern "C" extern "C" IUnknown * STDMETHODCALLTYPE CreateCrashReport(int32_t pid) { -#ifdef _WIN32 - return nullptr; -#else auto instance = CrashReporting::Create(pid); instance->AddRef(); return (IUnknown*)instance; -#endif } CrashReporting::CrashReporting(int32_t pid) @@ -302,6 +298,49 @@ int32_t CrashReporting::ExportImpl(ddog_Endpoint* endpoint) return 0; } +std::vector CrashReporting::MergeFrames(const std::vector& nativeFrames, const std::vector& managedFrames) +{ + std::vector result; + result.reserve(std::max(nativeFrames.size(), managedFrames.size())); + + size_t i = 0, j = 0; + while (i < nativeFrames.size() && j < managedFrames.size()) + { + if (nativeFrames.at(i).sp < managedFrames.at(j).sp) + { + result.push_back(nativeFrames.at(i)); + ++i; + } + else if (managedFrames.at(j).sp < nativeFrames.at(i).sp) + { + result.push_back(managedFrames.at(j)); + ++j; + } + else + { // frames[i].sp == managedFrames[j].sp + // Prefer managedFrame when sp values are the same + result.push_back(managedFrames.at(j)); + ++i; + ++j; + } + } + + // Add any remaining frames that are left in either vector + while (i < nativeFrames.size()) + { + result.push_back(nativeFrames.at(i)); + ++i; + } + + while (j < managedFrames.size()) + { + result.push_back(managedFrames.at(j)); + ++j; + } + + return result; +} + int32_t CrashReporting::CrashProcess() { std::thread crashThread([]() diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native/CrashReporting.h b/profiler/src/ProfilerEngine/Datadog.Profiler.Native/CrashReporting.h index 3f3ff1cc47b6..fcf00423f49c 100644 --- a/profiler/src/ProfilerEngine/Datadog.Profiler.Native/CrashReporting.h +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native/CrashReporting.h @@ -102,6 +102,7 @@ class CrashReporting : public ICrashReporting virtual std::vector GetThreadFrames(int32_t tid, ResolveManagedCallstack resolveManagedCallstack, void* context) = 0; virtual std::string GetSignalInfo(int32_t signal) = 0; + static std::vector MergeFrames(const std::vector& nativeFrames, const std::vector& managedFrames); private: int32_t ExportImpl(ddog_Endpoint* endpoint); int32_t _refCount; diff --git a/shared/src/Datadog.Trace.ClrProfiler.Native/Datadog.Trace.ClrProfiler.Native.vcxproj b/shared/src/Datadog.Trace.ClrProfiler.Native/Datadog.Trace.ClrProfiler.Native.vcxproj index 79cc9eb6b28c..f6595e0d586d 100644 --- a/shared/src/Datadog.Trace.ClrProfiler.Native/Datadog.Trace.ClrProfiler.Native.vcxproj +++ b/shared/src/Datadog.Trace.ClrProfiler.Native/Datadog.Trace.ClrProfiler.Native.vcxproj @@ -318,6 +318,7 @@ + @@ -345,6 +346,7 @@ + diff --git a/shared/src/Datadog.Trace.ClrProfiler.Native/Datadog.Trace.ClrProfiler.Native.vcxproj.filters b/shared/src/Datadog.Trace.ClrProfiler.Native/Datadog.Trace.ClrProfiler.Native.vcxproj.filters index 7a5a769cbb06..e522bfbe26d9 100644 --- a/shared/src/Datadog.Trace.ClrProfiler.Native/Datadog.Trace.ClrProfiler.Native.vcxproj.filters +++ b/shared/src/Datadog.Trace.ClrProfiler.Native/Datadog.Trace.ClrProfiler.Native.vcxproj.filters @@ -84,6 +84,18 @@ Header Files + + Header Files + + + Header Files + + + Header Files + + + Header Files + @@ -137,6 +149,15 @@ shared + + Source Files + + + Source Files + + + Source Files + diff --git a/shared/src/Datadog.Trace.ClrProfiler.Native/crashhandler.cpp b/shared/src/Datadog.Trace.ClrProfiler.Native/crashhandler.cpp new file mode 100644 index 000000000000..abce9a477f1a --- /dev/null +++ b/shared/src/Datadog.Trace.ClrProfiler.Native/crashhandler.cpp @@ -0,0 +1,424 @@ +#include "crashhandler.h" +#include +#include "util.h" +#include "WerApi.h" +#include + +namespace datadog::shared::nativeloader +{ + // Get the path of the current module + std::wstring GetCurrentDllPath() + { + wchar_t path[MAX_PATH]; + HMODULE hm = NULL; + + if (GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | + GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, + (LPCWSTR)&GetCurrentDllPath, &hm) == 0) + { + Log::Warn("Crashtracking - Failed to get the current module handle: ", GetLastError()); + return std::wstring(); + } + + if (GetModuleFileNameW(hm, path, MAX_PATH) == 0) + { + Log::Warn("Crashtracking - Failed to get the current module filename: ", GetLastError()); + return std::wstring(); + } + + return std::wstring(path); + } + + bool RegistryValueExists(HKEY rootKey, LPCWSTR subKey, LPCWSTR valueName) + { + HKEY hKey; + auto result = RegOpenKeyEx(rootKey, subKey, 0, KEY_QUERY_VALUE, &hKey); + + if (result == ERROR_SUCCESS) + { + DWORD dataSize = 0; + DWORD valueType = 0; + result = RegQueryValueEx(hKey, valueName, NULL, &valueType, NULL, &dataSize); + RegCloseKey(hKey); + + return result == ERROR_SUCCESS; + } + + return false; + } + + // Capture all the environment variables starting with "DD_" + void GetDatadogEnvironmentBlock(WCHAR*& environmentVariables, int32_t& length) + { + auto envStrings = GetEnvironmentStrings(); + + std::wstring envBlock; + + if (envStrings == nullptr) + { + length = 2; + environmentVariables = new WCHAR[length]; + environmentVariables[0] = L'\0'; + environmentVariables[1] = L'\0'; + return; + } + + for (LPWCH env = envStrings; *env != L'\0'; env += wcslen(env) + 1) + { + if (wcsncmp(env, L"DD_", 3) == 0) + { + envBlock.append(env); + envBlock.push_back('\0'); + } + } + + // Environment block ends with a double null terminator + envBlock.push_back('\0'); + + if (envBlock.length() == 1) + { + // If the environment block was empty, we're still missing one null terminator + envBlock.push_back('\0'); + } + + length = static_cast(envBlock.length()); + environmentVariables = new WCHAR[length]; + memcpy(environmentVariables, envBlock.c_str(), length * sizeof(WCHAR)); + FreeEnvironmentStrings(envStrings); + } + + // Merge two environment blocks. The first one takes precedence. + std::vector MergeEnvironmentBlocks(LPCWSTR envBlock1, LPCWSTR envBlock2) + { + // Define a case-insensitive comparator for keys + struct CaseInsensitiveCompare + { + bool operator()(const std::wstring& lhs, const std::wstring& rhs) const + { + return _wcsicmp(lhs.c_str(), rhs.c_str()) < 0; + } + }; + + // Map to hold environment variables with case-insensitive keys + std::map envMap; + + // Helper lambda to parse an environment block into the map + auto parseEnvBlock = [&envMap](LPCWSTR envBlock) + { + if (envBlock) + { + LPCWSTR curr = envBlock; + while (*curr) + { + std::wstring entry = curr; + size_t pos = entry.find(L'='); + if (pos != std::wstring::npos) + { + std::wstring key = entry.substr(0, pos); + std::wstring value = entry.substr(pos + 1); + envMap[key] = std::move(value); // Insert or overwrite the key + } + // Move to the next null-terminated string + curr += entry.length() + 1; + } + } + }; + + // Parse envBlock2 first so that envBlock1 values take precedence + parseEnvBlock(envBlock2); + parseEnvBlock(envBlock1); + + // Reconstruct the combined environment block + std::vector result; + result.reserve(envMap.size()); + + for (const auto& kv : envMap) + { + std::wstring entry = kv.first + L"=" + kv.second; + result.insert(result.end(), entry.begin(), entry.end()); + result.push_back(L'\0'); // Null terminator for the entry + } + result.push_back(L'\0'); // Double null terminator at the end + + return result; + } + + std::unique_ptr CrashHandler::Create() + { + std::unique_ptr crashHandler(new CrashHandler()); + + if (crashHandler->Register()) + { + return crashHandler; + } + + return nullptr; + } + + CrashHandler::~CrashHandler() + { + if (!_crashHandler.empty()) + { + auto hr = WerUnregisterRuntimeExceptionModule(_crashHandler.c_str(), &_context); + Log::Debug("Crashtracking - Unregistering crash handler: ", hr); + _crashHandler.clear(); + } + + if (_context.Environ != nullptr) + { + delete[] _context.Environ; + } + + _context.Environ = nullptr; + _context.EnvironLength = 0; + } + + CrashHandler::CrashHandler() + : _context(), + _crashHandler() + { + } + + bool CrashHandler::Register() + { + if (!_crashHandler.empty()) + { + Log::Warn("Crashtracking - Trying to initialize the crash handler twice"); + return false; + } + + auto dllPath = GetCurrentDllPath(); + + if (dllPath.empty()) + { + Log::Warn("Crashtracking - Could not register the crash handler: error when retrieving the path of the DLL"); + return false; + } + + GetDatadogEnvironmentBlock(_context.Environ, _context.EnvironLength); + + // Register the crash handler in the registry + // Windows expects a DWORD value with the full path of the DLL as the name, + // in SOFTWARE\Microsoft\Windows\Windows Error Reporting\RuntimeExceptionHelperModules. + // The key can be located either in HKLM or HKCU. The MSI will create the value in HKLM, + // but if it's missing we add it to HKCU. + HKEY hKey; + LPCWSTR subKey = L"SOFTWARE\\Microsoft\\Windows\\Windows Error Reporting\\RuntimeExceptionHelperModules"; + DWORD value = 1; + + if (!RegistryValueExists(HKEY_LOCAL_MACHINE, subKey, dllPath.c_str())) + { + // Open the key + DWORD disposition; + auto result = RegCreateKeyEx(HKEY_CURRENT_USER, subKey, 0, NULL, 0, KEY_SET_VALUE, NULL, &hKey, &disposition); + + if (result != ERROR_SUCCESS) + { + // Failing to set the registry is not a fatal error: in the worst case scenario the crash handler just won't be invoked by WER + Log::Warn("Crashtracking - Failed to create registry key: ", GetLastError()); + } + else + { + // Set the value + result = RegSetValueEx(hKey, dllPath.c_str(), 0, REG_DWORD, reinterpret_cast(&value), sizeof(value)); + + if (result != ERROR_SUCCESS) + { + Log::Warn("Crashtracking - Failed to set registry value: ", GetLastError()); + } + + RegCloseKey(hKey); + } + } + + // The profiler API is not initialized yet, so we look for clr.dll/coreclr.dll + // to know if we're running with .NET Framework or .NET Core + bool isDotnetCore = true; + HMODULE hModule = GetModuleHandle(L"coreclr.dll"); + + if (hModule == NULL) + { + hModule = GetModuleHandle(L"clr.dll"); + isDotnetCore = false; + + if (hModule == NULL) + { + Log::Warn("Crashtracking - Failed to get module handle for coreclr.dll or clr.dll"); + return false; + } + } + + wchar_t buffer[MAX_PATH]; + + if (GetModuleFileNameW(hModule, buffer, MAX_PATH) == 0) + { + Log::Warn("Crashtracking - Failed to get module filename: ", GetLastError()); + return false; + } + + auto clrFileName = std::wstring(buffer); + + fs::path clrFileNamePath(clrFileName); + auto clrDirectory = clrFileNamePath.parent_path(); + + // Build the path to the DAC (mscordacwks.dll on .NET, mscordaccore.dll on .NET Core) + std::wstring dacFileName = isDotnetCore ? L"mscordaccore.dll" : L"mscordacwks.dll"; + fs::path dacFilePath = clrDirectory / dacFileName; + + // Unregister the .NET handler + Log::Debug("Crashtracking - Unregistering the .NET handler ", dacFilePath.c_str()); + auto unregisterDacHr = WerUnregisterRuntimeExceptionModule(dacFilePath.c_str(), (PVOID)hModule); + + if (FAILED(unregisterDacHr)) + { + Log::Warn("Crashtracking - Failed to unregister the DAC handler: ", unregisterDacHr); + } + + // Register our handler + Log::Debug("Crashtracking - Registering the crash handler ", dllPath.c_str()); + auto registrationHr = WerRegisterRuntimeExceptionModule(dllPath.c_str(), &_context); + + // If we successfully unregistered the .NET handler, put it back in place + if (SUCCEEDED(unregisterDacHr)) + { + auto registrationHr2 = WerRegisterRuntimeExceptionModule(dacFilePath.c_str(), (PVOID)hModule); + + if (FAILED(registrationHr2)) + { + Log::Warn("Crashtracking - Failed to re-register the DAC handler: ", registrationHr2); + } + } + + if (FAILED(registrationHr)) + { + Log::Warn("Crashtracking - Could not register the crash handler: ", registrationHr); + return false; + } + + _crashHandler = dllPath; + return true; + } + + extern "C" + { + HRESULT __declspec(dllexport) OutOfProcessExceptionEventCallback( + PVOID pContext, + const PWER_RUNTIME_EXCEPTION_INFORMATION pExceptionInformation, + BOOL* pbOwnershipClaimed, + PWSTR pwszEventName, + PDWORD pchSize, + PDWORD pdwSignatureCount + ) + { + Log::Debug("Crashtracking - OutOfProcessExceptionEventCallback"); + + if (pbOwnershipClaimed != nullptr) + { + // We don't claim the ownership of the crash. + // This way, the original crash handler registered by .NET will be invoked, + // and we don't affect the normal behavior. + *pbOwnershipClaimed = FALSE; + } + + // Get the pid and tid from the exception + auto pid = GetProcessId(pExceptionInformation->hProcess); + auto tid = GetThreadId(pExceptionInformation->hThread); + + // Read the environment variables saved in the crashing process + WerContext context{}; + + BOOL hasContext = ReadProcessMemory(pExceptionInformation->hProcess, pContext, &context, sizeof(context), nullptr); + BOOL hasEnviron = FALSE; + + std::vector envBlock; + + if (hasContext && context.EnvironLength > 0 && context.Environ != nullptr) + { + envBlock.resize(context.EnvironLength * sizeof(WCHAR)); + hasEnviron = ReadProcessMemory(pExceptionInformation->hProcess, context.Environ, envBlock.data(), context.EnvironLength * sizeof(WCHAR), nullptr); + + if (!hasEnviron) + { + Log::Warn("Crashtracking - Failed to read the environment block from crashing process"); + } + } + + // Merge them with the current environment variables + auto currentEnv = GetEnvironmentStrings(); + + if (hasEnviron) + { + envBlock = MergeEnvironmentBlocks((LPCWSTR)envBlock.data(), currentEnv); + } + else + { + envBlock.assign(currentEnv, currentEnv + envBlock.size()); + } + + FreeEnvironmentStrings(currentEnv); + + // Create the command-line for dd-dotnet + fs::path p(GetCurrentDllPath()); + auto directory = p.parent_path(); + auto ddDotnetPath = directory / "dd-dotnet.exe"; + + if (!fs::exists(ddDotnetPath)) + { + Log::Error("Crashtracking - dd-dotnet.exe not found at ", ddDotnetPath.c_str()); + return S_OK; + } + + std::wstringstream ss; + ss << ddDotnetPath << " createdump " << pid << " --crashthread " << tid; + auto commandLine = ss.str(); + + // Spawn dd-dotnet + STARTUPINFO si; + PROCESS_INFORMATION pi; + ZeroMemory(&si, sizeof(si)); + si.cb = sizeof(si); + ZeroMemory(&pi, sizeof(pi)); + + if (!CreateProcessW(NULL, &commandLine[0], NULL, NULL, FALSE, CREATE_NO_WINDOW | CREATE_UNICODE_ENVIRONMENT, envBlock.data(), NULL, &si, &pi)) + { + Log::Error("Crashtracking - Failed to spawn dd-dotnet.exe: ", GetLastError()); + return S_OK; + } + + // Wait for the process to exit + WaitForSingleObject(pi.hProcess, INFINITE); + + CloseHandle(pi.hProcess); + CloseHandle(pi.hThread); + + Log::Debug("Crashtracking - Crash report sent."); + + return S_OK; + } + + HRESULT __declspec(dllexport) OutOfProcessExceptionEventSignatureCallback( + PVOID pContext, + const PWER_RUNTIME_EXCEPTION_INFORMATION pExceptionInformation, + DWORD dwIndex, + PWSTR pwszName, + PDWORD pchName, + PWSTR pwszValue, + PDWORD pchValue + ) + { + return E_NOTIMPL; + } + + HRESULT __declspec(dllexport) OutOfProcessExceptionEventDebuggerLaunchCallback( + PVOID pContext, + const PWER_RUNTIME_EXCEPTION_INFORMATION pExceptionInformation, + PBOOL pbIsCustomDebugger, + PWSTR pwszDebuggerLaunch, + PDWORD pchDebuggerLaunch, + PBOOL pbIsDebuggerAutolaunch + ) + { + return E_NOTIMPL; + } + } +} \ No newline at end of file diff --git a/shared/src/Datadog.Trace.ClrProfiler.Native/crashhandler.h b/shared/src/Datadog.Trace.ClrProfiler.Native/crashhandler.h new file mode 100644 index 000000000000..479e33f4b7a9 --- /dev/null +++ b/shared/src/Datadog.Trace.ClrProfiler.Native/crashhandler.h @@ -0,0 +1,29 @@ +#pragma once + +#include "util.h" +#include "werapi.h" + +namespace datadog::shared::nativeloader +{ + // Be careful when updating the WerContext structure, it is read across processes using ReadProcessMemory + struct WerContext + { + WCHAR* Environ; + int32_t EnvironLength; + }; + + class CrashHandler + { + public: + static std::unique_ptr Create(); + + ~CrashHandler(); + + private: + CrashHandler(); + bool Register(); + + std::wstring _crashHandler; + WerContext _context; + }; +} \ No newline at end of file diff --git a/shared/src/Datadog.Trace.ClrProfiler.Native/dllmain.cpp b/shared/src/Datadog.Trace.ClrProfiler.Native/dllmain.cpp index f4e640700179..6f6a580d26d8 100644 --- a/shared/src/Datadog.Trace.ClrProfiler.Native/dllmain.cpp +++ b/shared/src/Datadog.Trace.ClrProfiler.Native/dllmain.cpp @@ -1,10 +1,22 @@ // dllmain.cpp : Defines the entry point for the DLL application. + +#if _WIN32 && AMD64 +#define CRASHTRACKING 1 +#endif + #include "cor_profiler_class_factory.h" #include "log.h" #include "dynamic_dispatcher.h" #include "util.h" +#if CRASHTRACKING +#include "crashhandler.h" +#endif + +#include "util.h" +#include + #ifndef _WIN32 #undef EXTERN_C #define EXTERN_C extern "C" __attribute__((visibility("default"))) @@ -14,67 +26,90 @@ using namespace datadog::shared::nativeloader; IDynamicDispatcher* dispatcher; +#if CRASHTRACKING +std::unique_ptr crashHandler; +#endif + EXTERN_C BOOL STDMETHODCALLTYPE DllMain(HMODULE hModule, DWORD ul_reason_for_call, LPVOID lpReserved) { // Perform actions based on the reason for calling. switch (ul_reason_for_call) { - case DLL_PROCESS_ATTACH: - { - // Initialize once for each new process. - // Return FALSE to fail DLL load. + case DLL_PROCESS_ATTACH: + { + // Initialize once for each new process. + // Return FALSE to fail DLL load. - constexpr const bool IsLogDebugEnabledDefault = false; - bool isLogDebugEnabled; + constexpr const bool IsLogDebugEnabledDefault = false; + bool isLogDebugEnabled; - shared::WSTRING isLogDebugEnabledStr = shared::GetEnvironmentValue(EnvironmentVariables::DebugLogEnabled); + shared::WSTRING isLogDebugEnabledStr = shared::GetEnvironmentValue(EnvironmentVariables::DebugLogEnabled); - // no environment variable set - if (isLogDebugEnabledStr.empty()) + // no environment variable set + if (isLogDebugEnabledStr.empty()) + { + Log::Info("No \"", EnvironmentVariables::DebugLogEnabled, "\" environment variable has been found.", + " Enable debug log = ", IsLogDebugEnabledDefault, " (default)."); + + isLogDebugEnabled = IsLogDebugEnabledDefault; + } + else + { + if (!shared::TryParseBooleanEnvironmentValue(isLogDebugEnabledStr, isLogDebugEnabled)) { - Log::Info("No \"", EnvironmentVariables::DebugLogEnabled, "\" environment variable has been found.", - " Enable debug log = ", IsLogDebugEnabledDefault, " (default)."); + // invalid value for environment variable + Log::Info("Non boolean value \"", isLogDebugEnabledStr, "\" for \"", + EnvironmentVariables::DebugLogEnabled, "\" environment variable.", + " Enable debug log = ", IsLogDebugEnabledDefault, " (default)."); isLogDebugEnabled = IsLogDebugEnabledDefault; } else { - if (!shared::TryParseBooleanEnvironmentValue(isLogDebugEnabledStr, isLogDebugEnabled)) - { - // invalid value for environment variable - Log::Info("Non boolean value \"", isLogDebugEnabledStr, "\" for \"", - EnvironmentVariables::DebugLogEnabled, "\" environment variable.", - " Enable debug log = ", IsLogDebugEnabledDefault, " (default)."); - - isLogDebugEnabled = IsLogDebugEnabledDefault; - } - else - { - // take environment variable into account - Log::Info("Enable debug log = ", isLogDebugEnabled, " from (", EnvironmentVariables::DebugLogEnabled, " environment variable)"); - } + // take environment variable into account + Log::Info("Enable debug log = ", isLogDebugEnabled, " from (", EnvironmentVariables::DebugLogEnabled, " environment variable)"); } + } - if (isLogDebugEnabled) - { - Log::EnableDebug(true); - } + if (isLogDebugEnabled) + { + Log::EnableDebug(true); + } - Log::Debug("DllMain: DLL_PROCESS_ATTACH"); - Log::Debug("DllMain: Pointer size: ", 8 * sizeof(void*), " bits."); +#if CRASHTRACKING + bool telemetry_enabled = true; + shared::TryParseBooleanEnvironmentValue(shared::GetEnvironmentValue(L"DD_INSTRUMENTATION_TELEMETRY_ENABLED"), telemetry_enabled); - dispatcher = new DynamicDispatcherImpl(); - dispatcher->LoadConfiguration(GetConfigurationFilePath()); + bool crashtracking_enabled = true; + shared::TryParseBooleanEnvironmentValue(shared::GetEnvironmentValue(L"DD_CRASHTRACKING_ENABLED"), crashtracking_enabled); - // ***************************************************************************************************************** - break; + if (telemetry_enabled && crashtracking_enabled) + { + Log::Info("Crashtracking - Registering unhandled exception filter."); + crashHandler = CrashHandler::Create(); + } + else + { + Log::Info("Crashtracking - Disabled by configuration."); } +#endif - case DLL_PROCESS_DETACH: - // Perform any necessary cleanup. - Log::Debug("DllMain: DLL_PROCESS_DETACH"); + dispatcher = new DynamicDispatcherImpl(); + dispatcher->LoadConfiguration(GetConfigurationFilePath()); + + // ***************************************************************************************************************** + break; + } + + case DLL_PROCESS_DETACH: + // Perform any necessary cleanup. + Log::Debug("DllMain: DLL_PROCESS_DETACH"); + +#if CRASHTRACKING + crashHandler = nullptr; +#endif - break; + break; } return TRUE; // Successful DLL_PROCESS_ATTACH. } @@ -84,7 +119,7 @@ EXTERN_C HRESULT STDMETHODCALLTYPE DllGetClassObject(REFCLSID rclsid, REFIID rii Log::Debug("DllGetClassObject"); // {846F5F1C-F9AE-4B07-969E-05C26BC060D8} - const GUID CLSID_CorProfiler = {0x846f5f1c, 0xf9ae, 0x4b07, {0x96, 0x9e, 0x5, 0xc2, 0x6b, 0xc0, 0x60, 0xd8}}; + const GUID CLSID_CorProfiler = { 0x846f5f1c, 0xf9ae, 0x4b07, {0x96, 0x9e, 0x5, 0xc2, 0x6b, 0xc0, 0x60, 0xd8} }; if (ppv == nullptr || rclsid != CLSID_CorProfiler) { diff --git a/tracer/build/_build/Build.Steps.cs b/tracer/build/_build/Build.Steps.cs index 3128e948e21b..96a5dc4c6ac8 100644 --- a/tracer/build/_build/Build.Steps.cs +++ b/tracer/build/_build/Build.Steps.cs @@ -2322,6 +2322,7 @@ var name when multiPackageProjects.Contains(name) => false, DotNetTest(config => config .SetProjectFile(project) + .SetDotnetPath(TargetPlatform) .SetConfiguration(BuildConfiguration) .SetFramework(Framework) .SetTestTargetPlatform(TargetPlatform) diff --git a/tracer/src/Datadog.Trace.Tools.dd_dotnet/CreatedumpCommand.cs b/tracer/src/Datadog.Trace.Tools.dd_dotnet/CreatedumpCommand.cs index aa545e83b91d..9fbdb6888f89 100644 --- a/tracer/src/Datadog.Trace.Tools.dd_dotnet/CreatedumpCommand.cs +++ b/tracer/src/Datadog.Trace.Tools.dd_dotnet/CreatedumpCommand.cs @@ -4,12 +4,14 @@ // using System; +using System.Collections; using System.Collections.Generic; using System.CommandLine; using System.CommandLine.Invocation; using System.ComponentModel; using System.IO; using System.Linq; +using System.Net.Http; using System.Runtime.InteropServices; using System.Text; using System.Text.RegularExpressions; @@ -421,7 +423,10 @@ private void Execute(InvocationContext context) private unsafe void GenerateCrashReport(int pid, int? signal, int? crashThread) { - var lib = NativeLibrary.Load(Path.Combine(AppContext.BaseDirectory, "Datadog.Profiler.Native.so")); + var extension = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "dll" : "so"; + var profilerLibrary = $"Datadog.Profiler.Native.{extension}"; + + var lib = NativeLibrary.Load(Path.Combine(AppContext.BaseDirectory, profilerLibrary)); var export = NativeLibrary.GetExport(lib, "CreateCrashReport"); diff --git a/tracer/test/Datadog.Trace.TestHelpers/MockTracerAgent.cs b/tracer/test/Datadog.Trace.TestHelpers/MockTracerAgent.cs index 55024bdb842c..cbe051949bca 100644 --- a/tracer/test/Datadog.Trace.TestHelpers/MockTracerAgent.cs +++ b/tracer/test/Datadog.Trace.TestHelpers/MockTracerAgent.cs @@ -38,9 +38,10 @@ public abstract class MockTracerAgent : IDisposable { private readonly CancellationTokenSource _cancellationTokenSource = new(); - protected MockTracerAgent(bool telemetryEnabled, TestTransports transport) + protected MockTracerAgent(bool telemetryEnabled, TestTransports transport, bool optionalTelemetryHeaders = false) { TelemetryEnabled = telemetryEnabled; + OptionalTelemetryHeaders = optionalTelemetryHeaders; TransportType = transport; } @@ -66,6 +67,8 @@ protected MockTracerAgent(bool telemetryEnabled, TestTransports transport) public bool TelemetryEnabled { get; } + public bool OptionalTelemetryHeaders { get; } + public Dictionary CustomResponses { get; } = new(); /// @@ -111,8 +114,8 @@ protected MockTracerAgent(bool telemetryEnabled, TestTransports transport) /// public bool ShouldDeserializeTraces { get; set; } = true; - public static TcpUdpAgent Create(ITestOutputHelper output, int? port = null, int retries = 5, bool useStatsd = false, bool doNotBindPorts = false, int? requestedStatsDPort = null, bool useTelemetry = false, AgentConfiguration agentConfiguration = null) - => new TcpUdpAgent(port, retries, useStatsd, doNotBindPorts, requestedStatsDPort, useTelemetry) { Output = output, Configuration = agentConfiguration ?? new() }; + public static TcpUdpAgent Create(ITestOutputHelper output, int? port = null, int retries = 5, bool useStatsd = false, bool doNotBindPorts = false, int? requestedStatsDPort = null, bool useTelemetry = false, bool optionalTelemetryHeaders = false, AgentConfiguration agentConfiguration = null) + => new TcpUdpAgent(port, retries, useStatsd, doNotBindPorts, requestedStatsDPort, useTelemetry, optionalTelemetryHeaders) { Output = output, Configuration = agentConfiguration ?? new() }; #if NETCOREAPP3_1_OR_GREATER public static UdsAgent Create(ITestOutputHelper output, UnixDomainSocketConfig config, AgentConfiguration agentConfiguration = null) => new UdsAgent(config) { Output = output, Configuration = agentConfiguration ?? new() }; @@ -597,10 +600,21 @@ private void HandlePotentialTelemetryData(MockHttpRequest request) { try { - var apiVersion = request.Headers.GetValue(TelemetryConstants.ApiVersionHeader); - var requestType = request.Headers.GetValue(TelemetryConstants.RequestTypeHeader); + request.Headers.TryGetValue(TelemetryConstants.ApiVersionHeader, out var apiVersion); + request.Headers.TryGetValue(TelemetryConstants.RequestTypeHeader, out var requestType); var body = request.ReadStreamBody(); + + if (OptionalTelemetryHeaders && (apiVersion == null || requestType == null)) + { + using var sr = new StreamReader(new MemoryStream(body)); + var text = sr.ReadToEnd(); + + var json = JObject.Parse(text); + apiVersion = json["api_version"].Value(); + requestType = json["request_type"].Value(); + } + using var stream = new MemoryStream(body); var telemetry = MockTelemetryAgent.DeserializeResponse(stream, apiVersion, requestType); @@ -1039,8 +1053,8 @@ public class TcpUdpAgent : MockTracerAgent private readonly Task _tracesListenerTask; private readonly Task _statsdTask; - public TcpUdpAgent(int? port, int retries, bool useStatsd, bool doNotBindPorts, int? requestedStatsDPort, bool useTelemetry) - : base(useTelemetry, TestTransports.Tcp) + public TcpUdpAgent(int? port, int retries, bool useStatsd, bool doNotBindPorts, int? requestedStatsDPort, bool useTelemetry, bool optionalTelemetryHeaders) + : base(useTelemetry, TestTransports.Tcp, optionalTelemetryHeaders) { port ??= TcpPortProvider.GetOpenPort(); if (doNotBindPorts) diff --git a/tracer/test/Datadog.Trace.Tools.dd_dotnet.ArtifactTests/ConsoleTestHelper.cs b/tracer/test/Datadog.Trace.Tools.dd_dotnet.ArtifactTests/ConsoleTestHelper.cs index 2127323fa1bc..859d68d44256 100644 --- a/tracer/test/Datadog.Trace.Tools.dd_dotnet.ArtifactTests/ConsoleTestHelper.cs +++ b/tracer/test/Datadog.Trace.Tools.dd_dotnet.ArtifactTests/ConsoleTestHelper.cs @@ -21,12 +21,12 @@ protected ConsoleTestHelper(ITestOutputHelper output) { } - protected Task StartConsole(bool enableProfiler, params (string Key, string Value)[] environmentVariables) + protected Task StartConsole(bool enableProfiler, params (string Key, string Value)[] environmentVariables) { return StartConsole(EnvironmentHelper, enableProfiler, "wait", environmentVariables); } - protected Task StartConsoleWithArgs(string args, bool enableProfiler, params (string Key, string Value)[] environmentVariables) + protected Task StartConsoleWithArgs(string args, bool enableProfiler, params (string Key, string Value)[] environmentVariables) { return StartConsole(EnvironmentHelper, enableProfiler, args, environmentVariables); } @@ -49,7 +49,7 @@ protected Task StartConsoleWithArgs(string args, bool enableProfi return (executable, args); } - protected Task StartConsole(EnvironmentHelper environmentHelper, bool enableProfiler, string args, params (string Key, string Value)[] environmentVariables) + protected Task StartConsole(EnvironmentHelper environmentHelper, bool enableProfiler, string args, params (string Key, string Value)[] environmentVariables) { var (executable, baseArgs) = PrepareSampleApp(environmentHelper); args = $"{baseArgs} {args}"; @@ -57,7 +57,7 @@ protected Task StartConsole(EnvironmentHelper environmentHelper, return StartConsole(executable, args, environmentHelper, enableProfiler, environmentVariables); } - protected async Task StartConsole(string executable, string args, EnvironmentHelper environmentHelper, bool enableProfiler, params (string Key, string Value)[] environmentVariables) + protected async Task StartConsole(string executable, string args, EnvironmentHelper environmentHelper, bool enableProfiler, params (string Key, string Value)[] environmentVariables) { var processStart = new ProcessStartInfo(executable, args) { UseShellExecute = false, RedirectStandardError = true, RedirectStandardOutput = true }; @@ -65,7 +65,7 @@ protected async Task StartConsole(string executable, string args, if (enableProfiler) { - agent = MockTracerAgent.Create(Output); + agent = MockTracerAgent.Create(Output, useTelemetry: true, optionalTelemetryHeaders: true); environmentHelper.SetEnvironmentVariables( agent, @@ -129,20 +129,20 @@ protected async Task StartConsole(string executable, string args, throw new TimeoutException("Timeout when waiting for the target process to start"); } - private class CustomProcessHelper : ProcessHelper + public class CustomProcessHelper : ProcessHelper { - private readonly MockTracerAgent? _agent; - public CustomProcessHelper(MockTracerAgent? agent, Process process, Action? onDataReceived = null) : base(process, onDataReceived) { - _agent = agent; + Agent = agent; } + public MockTracerAgent? Agent { get; } + public override void Dispose() { base.Dispose(); - _agent?.Dispose(); + Agent?.Dispose(); } } } diff --git a/tracer/test/Datadog.Trace.Tools.dd_dotnet.ArtifactTests/CreatedumpTests.cs b/tracer/test/Datadog.Trace.Tools.dd_dotnet.ArtifactTests/CreatedumpTests.cs index 5b69778a652c..16ff5a735ab7 100644 --- a/tracer/test/Datadog.Trace.Tools.dd_dotnet.ArtifactTests/CreatedumpTests.cs +++ b/tracer/test/Datadog.Trace.Tools.dd_dotnet.ArtifactTests/CreatedumpTests.cs @@ -3,12 +3,11 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. // -#if !NETFRAMEWORK - using System; using System.Collections.Generic; using System.IO; using System.Linq; +using System.Runtime.InteropServices; using System.Text.RegularExpressions; using System.Threading.Tasks; using Datadog.Trace.Telemetry; @@ -24,7 +23,9 @@ namespace Datadog.Trace.Tools.dd_dotnet.ArtifactTests; public class CreatedumpTests : ConsoleTestHelper { +#if !NETFRAMEWORK // Createdump is not supported on .NET Framework private const string CreatedumpExpectedOutput = "Writing minidump with heap to file /dev/null"; +#endif private const string CrashReportExpectedOutput = "The crash may have been caused by automatic instrumentation"; public CreatedumpTests(ITestOutputHelper output) @@ -42,7 +43,7 @@ private static (string Key, string Value) LdPreloadConfig { var path = Utils.GetApiWrapperPath(); - if (!File.Exists(path)) + if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) && !File.Exists(path)) { throw new FileNotFoundException($"LD wrapper not found at path {path}. Ensure you have built the profiler home directory using BuildProfilerHome"); } @@ -53,6 +54,7 @@ private static (string Key, string Value) LdPreloadConfig private static (string Key, string Value)[] CreatedumpConfig => [("COMPlus_DbgEnableMiniDump", "1"), ("COMPlus_DbgMiniDumpName", "/dev/null")]; +#if !NETFRAMEWORK [SkippableTheory] [InlineData("1", true)] [InlineData("0", false)] @@ -68,12 +70,13 @@ public async Task Passthrough(string passthrough, bool shouldCallCreatedump) // "was COMPlus_DbgEnableMiniDump set?" check. SkipOn.Platform(SkipOn.PlatformValue.MacOs); + SkipOn.Platform(SkipOn.PlatformValue.Windows); // This test is not needed on Windows because we don't hook createdump using var reportFile = new TemporaryFile(); (string, string)[] args = [LdPreloadConfig, .. CreatedumpConfig, ("DD_INTERNAL_CRASHTRACKING_PASSTHROUGH", passthrough), CrashReportConfig(reportFile)]; - using var helper = await StartConsoleWithArgs("crash-datadog", false, args); + using var helper = await StartConsoleWithArgs("crash-datadog", enableProfiler: true, args); await helper.Task; @@ -82,6 +85,7 @@ public async Task Passthrough(string passthrough, bool shouldCallCreatedump) assertionScope.AddReportable("stderr", helper.ErrorOutput); helper.StandardOutput.Should().Contain(CrashReportExpectedOutput); + File.Exists(reportFile.Path).Should().BeTrue(); if (shouldCallCreatedump) @@ -128,6 +132,7 @@ public async Task BashScript() helper.StandardOutput.Should().Contain(CreatedumpExpectedOutput); } +#endif [SkippableTheory] [InlineData(true)] @@ -136,16 +141,22 @@ public async Task DoNothingIfNotEnabled(bool enableCrashDumps) { SkipOn.Platform(SkipOn.PlatformValue.MacOs); + if (enableCrashDumps) + { + // Crashtracking is not supported on Windows x86 + SkipOn.PlatformAndArchitecture(SkipOn.PlatformValue.Windows, SkipOn.ArchitectureValue.X86); + } + using var reportFile = new TemporaryFile(); - (string, string)[] args = [LdPreloadConfig, ("DD_CRASHTRACKING_ENABLED", "0")]; + (string, string)[] args = [LdPreloadConfig, CrashReportConfig(reportFile), ("DD_CRASHTRACKING_ENABLED", "0")]; if (enableCrashDumps) { args = [.. args, .. CreatedumpConfig]; } - using var helper = await StartConsoleWithArgs("crash-datadog", false, args); + using var helper = await StartConsoleWithArgs("crash-datadog", enableProfiler: true, args); await helper.Task; @@ -153,8 +164,12 @@ public async Task DoNothingIfNotEnabled(bool enableCrashDumps) assertionScope.AddReportable("stdout", helper.StandardOutput); assertionScope.AddReportable("stderr", helper.ErrorOutput); - helper.StandardOutput.Should().NotContain(CrashReportExpectedOutput); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + helper.StandardOutput.Should().NotContain(CrashReportExpectedOutput); + } +#if !NETFRAMEWORK if (enableCrashDumps) { helper.StandardOutput.Should().Contain(CreatedumpExpectedOutput); @@ -163,6 +178,7 @@ public async Task DoNothingIfNotEnabled(bool enableCrashDumps) { helper.StandardOutput.Should().NotContain(CreatedumpExpectedOutput); } +#endif File.Exists(reportFile.Path).Should().BeFalse(); } @@ -174,6 +190,7 @@ public async Task DoNothingIfNotEnabled(bool enableCrashDumps) public async Task DisableTelemetry(bool telemetryEnabled, bool crashdumpEnabled) { SkipOn.Platform(SkipOn.PlatformValue.MacOs); + SkipOn.PlatformAndArchitecture(SkipOn.PlatformValue.Windows, SkipOn.ArchitectureValue.X86); using var reportFile = new TemporaryFile(); @@ -186,7 +203,7 @@ public async Task DisableTelemetry(bool telemetryEnabled, bool crashdumpEnabled) args = [.. args, ("DD_INSTRUMENTATION_TELEMETRY_ENABLED", telemetryEnabled ? "1" : "0")]; - using var helper = await StartConsoleWithArgs("crash-datadog", false, args); + using var helper = await StartConsoleWithArgs("crash-datadog", enableProfiler: true, args); await helper.Task; @@ -194,6 +211,7 @@ public async Task DisableTelemetry(bool telemetryEnabled, bool crashdumpEnabled) assertionScope.AddReportable("stdout", helper.StandardOutput); assertionScope.AddReportable("stderr", helper.ErrorOutput); +#if !NETFRAMEWORK if (crashdumpEnabled) { helper.StandardOutput.Should().Contain(CreatedumpExpectedOutput); @@ -202,15 +220,24 @@ public async Task DisableTelemetry(bool telemetryEnabled, bool crashdumpEnabled) { helper.StandardOutput.Should().NotContain(CreatedumpExpectedOutput); } +#endif if (telemetryEnabled) { - helper.StandardOutput.Should().Contain(CrashReportExpectedOutput); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + helper.StandardOutput.Should().Contain(CrashReportExpectedOutput); + } + File.Exists(reportFile.Path).Should().BeTrue(); } else { - helper.StandardOutput.Should().NotContain(CrashReportExpectedOutput); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + helper.StandardOutput.Should().NotContain(CrashReportExpectedOutput); + } + File.Exists(reportFile.Path).Should().BeFalse(); } } @@ -219,12 +246,13 @@ public async Task DisableTelemetry(bool telemetryEnabled, bool crashdumpEnabled) public async Task WriteCrashReport() { SkipOn.Platform(SkipOn.PlatformValue.MacOs); + SkipOn.PlatformAndArchitecture(SkipOn.PlatformValue.Windows, SkipOn.ArchitectureValue.X86); using var reportFile = new TemporaryFile(); using var helper = await StartConsoleWithArgs( "crash-datadog", - false, + enableProfiler: true, [LdPreloadConfig, CrashReportConfig(reportFile)]); await helper.Task; @@ -233,7 +261,10 @@ public async Task WriteCrashReport() assertionScope.AddReportable("stdout", helper.StandardOutput); assertionScope.AddReportable("stderr", helper.ErrorOutput); - helper.StandardOutput.Should().Contain(CrashReportExpectedOutput); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + helper.StandardOutput.Should().Contain(CrashReportExpectedOutput); + } File.Exists(reportFile.Path).Should().BeTrue(); @@ -246,7 +277,11 @@ public async Task WriteCrashReport() .FirstOrDefault(t => t.StartsWith("exception:")); exception.Should().NotBeNull().And.StartWith("exception:Type: System.BadImageFormatException\nMessage: Expected\nStack Trace:\n"); - report["siginfo"]!["signum"]!.Value().Should().Be("6"); + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + report["siginfo"]!["signum"]!.Value().Should().Be("6"); + } } #if !NETFRAMEWORK @@ -277,7 +312,7 @@ public async Task WorksFromDifferentFolders(string subFolder) using var helper = await StartConsoleWithArgs( "crash-datadog", - false, + enableProfiler: true, [("LD_PRELOAD", newApiWrapperPath), CrashReportConfig(reportFile)]); await helper.Task; @@ -286,7 +321,10 @@ public async Task WorksFromDifferentFolders(string subFolder) assertionScope.AddReportable("stdout", helper.StandardOutput); assertionScope.AddReportable("stderr", helper.ErrorOutput); - helper.StandardOutput.Should().Contain(CrashReportExpectedOutput); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + helper.StandardOutput.Should().Contain(CrashReportExpectedOutput); + } File.Exists(reportFile.Path).Should().BeTrue(); } @@ -301,6 +339,7 @@ public async Task WorksFromDifferentFolders(string subFolder) public async Task NativeCrash() { SkipOn.Platform(SkipOn.PlatformValue.MacOs); + SkipOn.PlatformAndArchitecture(SkipOn.PlatformValue.Windows, SkipOn.ArchitectureValue.X86); if (Utils.IsAlpine()) { @@ -311,7 +350,7 @@ public async Task NativeCrash() using var helper = await StartConsoleWithArgs( "crash-native", - true, + enableProfiler: true, [LdPreloadConfig, CrashReportConfig(reportFile)]); await helper.Task; @@ -320,7 +359,10 @@ public async Task NativeCrash() assertionScope.AddReportable("stdout", helper.StandardOutput); assertionScope.AddReportable("stderr", helper.ErrorOutput); - helper.StandardOutput.Should().Contain(CrashReportExpectedOutput); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + helper.StandardOutput.Should().Contain(CrashReportExpectedOutput); + } File.Exists(reportFile.Path).Should().BeTrue(); } @@ -331,12 +373,13 @@ public async Task CheckThreadName() // Test that threads prefixed with DD_ are marked as suspicious even if they have nothing of Datadog in the stacktrace SkipOn.Platform(SkipOn.PlatformValue.MacOs); + SkipOn.PlatformAndArchitecture(SkipOn.PlatformValue.Windows, SkipOn.ArchitectureValue.X86); using var reportFile = new TemporaryFile(); using var helper = await StartConsoleWithArgs( "crash-thread", - true, + enableProfiler: true, [LdPreloadConfig, CrashReportConfig(reportFile)]); await helper.Task; @@ -345,7 +388,10 @@ public async Task CheckThreadName() assertionScope.AddReportable("stdout", helper.StandardOutput); assertionScope.AddReportable("stderr", helper.ErrorOutput); - helper.StandardOutput.Should().Contain(CrashReportExpectedOutput); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + helper.StandardOutput.Should().Contain(CrashReportExpectedOutput); + } File.Exists(reportFile.Path).Should().BeTrue(); } @@ -354,13 +400,12 @@ public async Task CheckThreadName() public async Task SendReportThroughTelemetry() { SkipOn.Platform(SkipOn.PlatformValue.MacOs); - - using var agent = new MockTelemetryAgent(TcpPortProvider.GetOpenPort()) { OptionalHeaders = true }; + SkipOn.PlatformAndArchitecture(SkipOn.PlatformValue.Windows, SkipOn.ArchitectureValue.X86); using var helper = await StartConsoleWithArgs( "crash-datadog", - false, - [LdPreloadConfig, ("DD_TRACE_AGENT_URL", $"http://localhost:{agent.Port}")]); + enableProfiler: true, + [LdPreloadConfig]); await helper.Task; @@ -368,35 +413,58 @@ public async Task SendReportThroughTelemetry() assertionScope.AddReportable("stdout", helper.StandardOutput); assertionScope.AddReportable("stderr", helper.ErrorOutput); - helper.StandardOutput.Should().Contain(CrashReportExpectedOutput); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + helper.StandardOutput.Should().Contain(CrashReportExpectedOutput); + } - var data = agent.WaitForLatestTelemetry(d => d.IsRequestType(TelemetryRequestTypes.RedactedErrorLogs)); - data.Should().NotBeNull(); + bool IsCrashReport(object payload) + { + if (payload is not TelemetryData data) + { + return false; + } + + if (data.TryGetPayload(TelemetryRequestTypes.RedactedErrorLogs) is not { } log) + { + return false; + } - var log = (LogsPayload)data.Payload; - log.Logs.Should().HaveCount(1); - var report = JObject.Parse(log.Logs[0].Message); + if (log.Logs.Count != 1) + { + return false; + } - report["additional_stacktraces"].Should().NotBeNull(); + var report = JObject.Parse(log.Logs[0].Message); + return report["additional_stacktraces"] != null; + } + + var agent = helper.Agent; + + agent.WaitForLatestTelemetry(IsCrashReport).Should().NotBeNull(); } [SkippableFact] public async Task IgnoreNonDatadogCrashes() { SkipOn.Platform(SkipOn.PlatformValue.MacOs); + SkipOn.PlatformAndArchitecture(SkipOn.PlatformValue.Windows, SkipOn.ArchitectureValue.X86); using var reportFile = new TemporaryFile(); using var helper = await StartConsoleWithArgs( "crash", - false, + enableProfiler: true, [LdPreloadConfig, CrashReportConfig(reportFile)]); await helper.Task; - helper.StandardOutput.Should() + if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + helper.StandardOutput.Should() .NotContain(CrashReportExpectedOutput) .And.EndWith("Crashing...\n"); // Making sure there is no additional output + } File.Exists(reportFile.Path).Should().BeFalse(); } @@ -405,12 +473,13 @@ public async Task IgnoreNonDatadogCrashes() public async Task ReportedStacktrace() { SkipOn.Platform(SkipOn.PlatformValue.MacOs); + SkipOn.PlatformAndArchitecture(SkipOn.PlatformValue.Windows, SkipOn.ArchitectureValue.X86); using var reportFile = new TemporaryFile(); using var helper = await StartConsoleWithArgs( "crash-datadog", - false, + enableProfiler: true, [LdPreloadConfig, CrashReportConfig(reportFile)]); await helper.Task; @@ -419,18 +488,18 @@ public async Task ReportedStacktrace() var reader = new StringReader(helper.StandardOutput); - int? expectedPid = null; + int? mainThreadId = null; var expectedCallstack = new List(); - var pidRegex = "PID: (?[0-9]+)"; + var tidRegex = "Main thread: (?[0-9]+)"; while (reader.ReadLine() is { } line) { - var pidMatch = Regex.Match(line, pidRegex); + var tidMatch = Regex.Match(line, tidRegex); - if (pidMatch.Success) + if (tidMatch.Success) { - expectedPid = int.Parse(pidMatch.Groups["pid"].Value); + mainThreadId = int.Parse(tidMatch.Groups["tid"].Value); continue; } @@ -440,7 +509,7 @@ public async Task ReportedStacktrace() } } - expectedPid.Should().NotBeNull(); + mainThreadId.Should().NotBeNull(); expectedCallstack.Should().HaveCountGreaterOrEqualTo(2); var report = JObject.Parse(reportFile.GetContent()); @@ -449,7 +518,7 @@ public async Task ReportedStacktrace() assertionScope.AddReportable("Report", report.ToString()); ValidateStacktrace(report["stacktrace"]); - ValidateStacktrace(report["additional_stacktraces"][expectedPid.Value.ToString()]); + ValidateStacktrace(report["additional_stacktraces"][mainThreadId.Value.ToString()]); void ValidateStacktrace(JToken callstack) { @@ -526,5 +595,3 @@ public void Dispose() } } } - -#endif diff --git a/tracer/test/test-applications/integrations/Samples.Console/Program.cs b/tracer/test/test-applications/integrations/Samples.Console/Program.cs index 7ba48e19a24f..05116fc5e15f 100644 --- a/tracer/test/test-applications/integrations/Samples.Console/Program.cs +++ b/tracer/test/test-applications/integrations/Samples.Console/Program.cs @@ -2,9 +2,11 @@ using System.ComponentModel; using System.Diagnostics; using System.IO; +using System.Linq; using System.Net; using System.Reflection; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; @@ -38,7 +40,7 @@ void DoCrash() var thread = new Thread( () => { - Thread.CurrentThread.Name = "DD_thread"; + SetCurrentThreadName("DD_thread"); DoCrash(); }); @@ -69,6 +71,16 @@ void DoCrash() } } + private static void SetCurrentThreadName(string name) + { +#if NETFRAMEWORK + // .NET Framework doesn't set the thread name at the OS level + SetThreadDescription(GetCurrentThread(), name); +#else + Thread.CurrentThread.Name = name; +#endif + } + // Can't use a "real" async Main because it messes up the callstack for the crash-report tests private static async Task AsyncMain(string[] args) { @@ -139,7 +151,7 @@ private static void DumpCallstackAndThrow(Exception exception) private static void Ready() { - Console.WriteLine($"Waiting - PID: {Process.GetCurrentProcess().Id} - Profiler attached: {SampleHelpers.IsProfilerAttached()}"); + Console.WriteLine($"Waiting - PID: {Process.GetCurrentProcess().Id} - Main thread: {GetMainThreadId()} - Profiler attached: {SampleHelpers.IsProfilerAttached()}"); } private static unsafe void NativeCrash() @@ -177,8 +189,14 @@ private static unsafe IntPtr CreateCrashReport(int pid) var nativeLibraryType = Type.GetType("Datadog.Trace.AppSec.Waf.NativeBindings.NativeLibrary, Datadog.Trace", throwOnError: true); var tryLoad = nativeLibraryType.GetMethod("TryLoad", BindingFlags.NonPublic | BindingFlags.Static); var getExport = nativeLibraryType.GetMethod("GetExport", BindingFlags.NonPublic | BindingFlags.Static); - - var folder = Path.GetDirectoryName(Environment.GetEnvironmentVariable("CORECLR_PROFILER_PATH")); + +#if NETFRAMEWORK + const string profilerPathEnvironmentVariable = "COR_PROFILER_PATH"; +#else + const string profilerPathEnvironmentVariable = "CORECLR_PROFILER_PATH"; +#endif + + var folder = Path.GetDirectoryName(Environment.GetEnvironmentVariable(profilerPathEnvironmentVariable)); var profilerPath = Path.Combine(folder, "Datadog.Profiler.Native" + (Environment.OSVersion.Platform == PlatformID.Win32NT ? ".dll" : ".so")); var arguments = new object[] { profilerPath, null }; @@ -191,6 +209,19 @@ private static unsafe IntPtr CreateCrashReport(int pid) return ((delegate* unmanaged[Stdcall])createCrashReport)(pid); } +#if NETFRAMEWORK + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool SetThreadDescription(IntPtr hThread, [MarshalAs(UnmanagedType.LPWStr)] string lpThreadDescription); + + [DllImport("kernel32.dll", SetLastError = true)] + internal static extern IntPtr GetCurrentThread(); +#endif + + private static int GetMainThreadId() + { + return Process.GetCurrentProcess().Threads.Cast().First().Id; + } + private class DummySynchronizationContext : SynchronizationContext { public override void Post(SendOrPostCallback d, object state) => d(state); From 587426a0c4754599ca9fa69fa9f486d914b985de Mon Sep 17 00:00:00 2001 From: Kevin Gosse Date: Thu, 3 Oct 2024 12:23:20 +0200 Subject: [PATCH 26/38] [Crashtracking] Add registry key to installer (#6106) ## Summary of changes Register `Datadog.Trace.ClrProfiler.Native.dll` in the `HKLM\Software\Microsoft\Windows\Windows Error Reporting\RuntimeExceptionHelperModules` key. ## Reason for change This is needed by crashtracking. If the value is absent, the tracer will create it in HKCU (the instrumented application is unlikely to have permission to write to HKLM). However, registering the crash handler in HKCU is only supported in somewhat versions of Windows (since Windows 10 2004). So we should rely on HKLM whenever possible. ## Other details Depends on https://github.com/DataDog/dd-trace-dotnet/pull/6088 --------- Co-authored-by: Andrew Lock --- shared/src/msi-installer/Product.wxs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/shared/src/msi-installer/Product.wxs b/shared/src/msi-installer/Product.wxs index 0bcf7fd89aad..700ddfe93865 100644 --- a/shared/src/msi-installer/Product.wxs +++ b/shared/src/msi-installer/Product.wxs @@ -129,6 +129,9 @@ + + + From 0ec8f0132f205c2962c22726eac5ec05a44e604a Mon Sep 17 00:00:00 2001 From: Kevin Gosse Date: Thu, 3 Oct 2024 12:23:47 +0200 Subject: [PATCH 27/38] [Crashtracking] Remove exception types that are too broad (#6109) ## Summary of changes Remove `System.MissingMethodException` and `System.Security.VerificationException` from the suspicious exception types. ## Reason for change They're too broad and are going to cause false-negatives. A crash report will still be generated if one of those exceptions cause a crash *and* we're in the callstack. --- tracer/src/Datadog.Trace.Tools.dd_dotnet/CreatedumpCommand.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/tracer/src/Datadog.Trace.Tools.dd_dotnet/CreatedumpCommand.cs b/tracer/src/Datadog.Trace.Tools.dd_dotnet/CreatedumpCommand.cs index 9fbdb6888f89..d2af2025cb40 100644 --- a/tracer/src/Datadog.Trace.Tools.dd_dotnet/CreatedumpCommand.cs +++ b/tracer/src/Datadog.Trace.Tools.dd_dotnet/CreatedumpCommand.cs @@ -522,8 +522,6 @@ private unsafe void GenerateCrashReport(int pid, int? signal, int? crashThread) var suspiciousExceptionTypes = new[] { "System.InvalidProgramException", - "System.Security.VerificationException", - "System.MissingMethodException", "System.MissingFieldException", "System.MissingMemberException", "System.BadImageFormatException", From a1838fa7714b39c6af9bc0bada3bbc778dcec2a1 Mon Sep 17 00:00:00 2001 From: Anna Date: Thu, 3 Oct 2024 14:14:51 +0200 Subject: [PATCH 28/38] [ASM] Activate api sec by default (#6043) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary of changes Activate api security by default ## Reason for change https://docs.google.com/document/d/1BbF-ZQHG9seaaik578WFCb1PEs-SmKq34PHg5sG3q6s/edit#heading=h.d3npy38hknxa ## Implementation details Had to ignore the tags in the system tests as Gzip bytes results in different values under linux/windows Maybe future: implement in the test agent some normalization for api security tags. ## Test coverage ## Other details --- .azure-pipelines/ultimate-pipeline.yml | 2 +- docker-compose.windows.yml | 2 +- docker-compose.yml | 2 +- tracer/build/smoke_test_snapshots/smoke_test_snapshots.json | 4 ++++ .../smoke_test_snapshots/smoke_test_snapshots_2_1.json | 3 +++ tracer/src/Datadog.Trace/AppSec/SecuritySettings.cs | 2 +- .../Datadog.Trace/Configuration/ConfigurationKeys.AppSec.cs | 2 +- .../ApiSecurity/AspNetCoreApiSecurity.cs | 5 +---- .../ApiSecurity/AspNetFxWebApiApiSecurity.cs | 6 +----- .../ApiSecurity/AspNetMvc5ApiSecurity.cs | 6 +----- .../Datadog.Trace.Security.IntegrationTests/AspNetBase.cs | 1 + .../AspNetCore5AutoUserEvents.cs | 1 + .../AspNetCoreBare.cs | 1 + .../AspNetCoreBase.cs | 2 +- .../Datadog.Trace.Security.IntegrationTests/AspNetMvc5.cs | 1 + .../Datadog.Trace.Security.IntegrationTests/Rcm/RcmBase.cs | 1 + 16 files changed, 21 insertions(+), 20 deletions(-) diff --git a/.azure-pipelines/ultimate-pipeline.yml b/.azure-pipelines/ultimate-pipeline.yml index fc60ccc73798..e7cde930012e 100644 --- a/.azure-pipelines/ultimate-pipeline.yml +++ b/.azure-pipelines/ultimate-pipeline.yml @@ -6341,7 +6341,7 @@ stages: SNAPSHOT_DIR: $(System.DefaultWorkingDirectory)/tracer/build/smoke_test_snapshots SNAPSHOT_CI: 1 # ignoring 'http.client_ip' and 'network.client.ip' because it's set to '::1' here instead of expected '127.0.0.1' - SNAPSHOT_IGNORED_ATTRS: span_id,trace_id,parent_id,duration,start,metrics.system.pid,meta.runtime-id,metrics.process_id,meta._dd.p.dm,meta._dd.p.tid,meta._dd.git.commit.sha,meta._dd.git.repository_url,meta.http.client_ip,meta.network.client.ip + SNAPSHOT_IGNORED_ATTRS: span_id,trace_id,parent_id,duration,start,metrics.system.pid,meta.runtime-id,metrics.process_id,meta._dd.p.dm,meta._dd.p.tid,meta._dd.git.commit.sha,meta._dd.git.repository_url,meta.http.client_ip,meta.network.client.ip,meta._dd.appsec.s.req.params,meta._dd.appsec.s.res.body,meta._dd.appsec.s.req.headers,meta._dd.appsec.s.res.headers #api-security attrs are unfortunately ignored because gzip compression generates different bytes per platform windows/linux - script: | # Based on variables set in smoke.dotnet-tool.nuget.dockerfile diff --git a/docker-compose.windows.yml b/docker-compose.windows.yml index f7a835f8860b..02c92ea5f21a 100644 --- a/docker-compose.windows.yml +++ b/docker-compose.windows.yml @@ -23,7 +23,7 @@ services: - "8126:8126" environment: - SNAPSHOT_CI=1 - - SNAPSHOT_IGNORED_ATTRS=span_id,trace_id,parent_id,duration,start,metrics.system.pid,meta.runtime-id,metrics.process_id,meta.http.client_ip,meta.network.client.ip,meta._dd.p.dm,meta._dd.p.tid,meta._dd.parent_id + - SNAPSHOT_IGNORED_ATTRS=span_id,trace_id,parent_id,duration,start,metrics.system.pid,meta.runtime-id,metrics.process_id,meta.http.client_ip,meta.network.client.ip,meta._dd.p.dm,meta._dd.p.tid,meta._dd.parent_id,meta._dd.appsec.s.req.params,meta._dd.appsec.s.res.body,meta._dd.appsec.s.req.headers,meta._dd.appsec.s.res.headers #api-security attrs are unfortunately ignored because gzip compression generates different bytes per platform windows/linux smoke-tests.windows: build: diff --git a/docker-compose.yml b/docker-compose.yml index c169b3b1e65b..2b73089d2613 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -871,7 +871,7 @@ services: environment: - ENABLED_CHECKS=trace_count_header,meta_tracer_version_header,trace_content_length - SNAPSHOT_CI=1 - - SNAPSHOT_IGNORED_ATTRS=span_id,trace_id,parent_id,duration,start,metrics.system.pid,meta.runtime-id,metrics.process_id,meta._dd.p.dm,meta._dd.p.tid,meta._dd.parent_id + - SNAPSHOT_IGNORED_ATTRS=span_id,trace_id,parent_id,duration,start,metrics.system.pid,meta.runtime-id,metrics.process_id,meta._dd.p.dm,meta._dd.p.tid,meta._dd.parent_id,meta._dd.appsec.s.req.params,meta._dd.appsec.s.res.body,meta._dd.appsec.s.req.headers,meta._dd.appsec.s.res.headers #api-security attrs are unfortunately ignored because gzip compression generates different bytes per platform windows/linux smoke-tests: build: diff --git a/tracer/build/smoke_test_snapshots/smoke_test_snapshots.json b/tracer/build/smoke_test_snapshots/smoke_test_snapshots.json index 9563c41a1a03..429e6cd2450a 100644 --- a/tracer/build/smoke_test_snapshots/smoke_test_snapshots.json +++ b/tracer/build/smoke_test_snapshots/smoke_test_snapshots.json @@ -40,6 +40,10 @@ "meta": { "_dd.appsec.waf.version": "1.19.1", "_dd.runtime_family": "dotnet", + "_dd.appsec.s.req.params": "H4sIAAAAAAAAA4uuVkrOzyspys/JSS1Ssoq2iNVRSkwuyczPA3NqYwH+CR9jIQAAAA==", + "_dd.appsec.s.res.body": "H4sIAAAAAAAAA4u2iAUA8YntnQMAAAA=", + "_dd.appsec.s.req.headers": "H4sIAAAAAAAAA4WOMQ6AIBAE/3I1FHaGrxCKixAkQSBwhYbwdzUWNhDqnd1ZWeHkGgl1tDxhNoG40yCklKtSrII3AcTSFAPKuJlCSKYb/zOEtkyQgkfyLjzK7GJ2dI2N36mZ8iVHx/dYev2mbkdocaj9AAAA", + "_dd.appsec.s.res.headers": "H4sIAAAAAAAAA4uuVkrOzytJzSvRLaksSFWyio6OtoiN1alWyknNU7IyrI2tjQUAcaAU2yQAAAA=", "aspnet_core.endpoint": "AspNetCoreSmokeTest.ValuesController.Get (AspNetCoreSmokeTest)", "aspnet_core.route": "api/values", "component": "aspnet_core", diff --git a/tracer/build/smoke_test_snapshots/smoke_test_snapshots_2_1.json b/tracer/build/smoke_test_snapshots/smoke_test_snapshots_2_1.json index 8263bcdbb341..ee830e108a3b 100644 --- a/tracer/build/smoke_test_snapshots/smoke_test_snapshots_2_1.json +++ b/tracer/build/smoke_test_snapshots/smoke_test_snapshots_2_1.json @@ -40,6 +40,9 @@ "meta": { "_dd.appsec.waf.version": "1.19.1", "_dd.runtime_family": "dotnet", + "_dd.appsec.s.res.body": "H4sIAAAAAAAAA4u2iAUA8YntnQMAAAA=", + "_dd.appsec.s.req.headers": "H4sIAAAAAAAAA4WOMQrAIBDA/uKsQ7fiVw6Ho4oVrIp3Q4v491JcLc4JJNAEVzwcMbITGgB2Y2QT0SWht27kwAWrSzzlt7LIaLNXjJ4WCuFVYkhelRpyDfws/NFVwa7S3+SfdmaarXfzAg6PMlH9AAAA", + "_dd.appsec.s.res.headers": "H4sIAAAAAAAAA4uuVkrOzytJzSvRLaksSFWyio6OtoiN1alWyknNU7IyrI2tjQUAcaAU2yQAAAA=", "aspnet_core.route": "api/values", "component": "aspnet_core", "http.client_ip": "127.0.0.1", diff --git a/tracer/src/Datadog.Trace/AppSec/SecuritySettings.cs b/tracer/src/Datadog.Trace/AppSec/SecuritySettings.cs index 3d03095a7448..6db6208ee3aa 100644 --- a/tracer/src/Datadog.Trace/AppSec/SecuritySettings.cs +++ b/tracer/src/Datadog.Trace/AppSec/SecuritySettings.cs @@ -124,7 +124,7 @@ public SecuritySettings(IConfigurationSource? source, IConfigurationTelemetry te } ApiSecurityEnabled = config.WithKeys(ConfigurationKeys.AppSec.ApiSecurityEnabled, "DD_EXPERIMENTAL_API_SECURITY_ENABLED") - .AsBool(false); + .AsBool(true); ApiSecuritySampleDelay = config.WithKeys(ConfigurationKeys.AppSec.ApiSecuritySampleDelay) .AsDouble(30.0, val => val >= 0.0) diff --git a/tracer/src/Datadog.Trace/Configuration/ConfigurationKeys.AppSec.cs b/tracer/src/Datadog.Trace/Configuration/ConfigurationKeys.AppSec.cs index e3ff01f312fd..536969887552 100644 --- a/tracer/src/Datadog.Trace/Configuration/ConfigurationKeys.AppSec.cs +++ b/tracer/src/Datadog.Trace/Configuration/ConfigurationKeys.AppSec.cs @@ -112,7 +112,7 @@ internal class AppSec internal const string UserEventsAutoInstrumentationMode = "DD_APPSEC_AUTO_USER_INSTRUMENTATION_MODE"; /// - /// Unless set to true or 1, tracers don’t collect schemas. After the experiment, the environment variable will be removed and schema collection will be enabled only when ASM is enabled + /// When ASM is enabled, collects in spans endpoints apis schemas analyzed by the waf, default value is true. /// internal const string ApiSecurityEnabled = "DD_API_SECURITY_ENABLED"; diff --git a/tracer/test/Datadog.Trace.Security.IntegrationTests/ApiSecurity/AspNetCoreApiSecurity.cs b/tracer/test/Datadog.Trace.Security.IntegrationTests/ApiSecurity/AspNetCoreApiSecurity.cs index c15d5901b6d9..2369fe0ca12b 100644 --- a/tracer/test/Datadog.Trace.Security.IntegrationTests/ApiSecurity/AspNetCoreApiSecurity.cs +++ b/tracer/test/Datadog.Trace.Security.IntegrationTests/ApiSecurity/AspNetCoreApiSecurity.cs @@ -35,10 +35,7 @@ protected AspNetCoreApiSecurity(AspNetCoreTestFixture fixture, ITestOutputHelper // necessary as the developer middleware prevents the right blocking response EnvironmentHelper.CustomEnvironmentVariables.Add("ASPNETCORE_ENVIRONMENT", "Production"); SetEnvironmentVariable(ConfigurationKeys.LogDirectory, LogDirectory); - if (enableApiSecurity) - { - EnvironmentHelper.CustomEnvironmentVariables.Add(ConfigurationKeys.AppSec.ApiSecurityEnabled, "true"); - } + SetEnvironmentVariable(ConfigurationKeys.AppSec.ApiSecurityEnabled, enableApiSecurity.ToString()); } public override void Dispose() diff --git a/tracer/test/Datadog.Trace.Security.IntegrationTests/ApiSecurity/AspNetFxWebApiApiSecurity.cs b/tracer/test/Datadog.Trace.Security.IntegrationTests/ApiSecurity/AspNetFxWebApiApiSecurity.cs index fff92263332a..9ce0c10ca2cb 100644 --- a/tracer/test/Datadog.Trace.Security.IntegrationTests/ApiSecurity/AspNetFxWebApiApiSecurity.cs +++ b/tracer/test/Datadog.Trace.Security.IntegrationTests/ApiSecurity/AspNetFxWebApiApiSecurity.cs @@ -34,11 +34,7 @@ internal AspNetFxWebApiApiSecurity(IisFixture iisFixture, ITestOutputHelper outp { SetSecurity(true); EnvironmentHelper.CustomEnvironmentVariables.Add(ConfigurationKeys.AppSec.Rules, "ApiSecurity\\ruleset-with-block.json"); - - if (enableApiSecurity) - { - EnvironmentHelper.CustomEnvironmentVariables.Add(ConfigurationKeys.AppSec.ApiSecurityEnabled, "true"); - } + SetEnvironmentVariable(ConfigurationKeys.AppSec.ApiSecurityEnabled, enableApiSecurity.ToString()); _iisFixture = iisFixture; AddCookies(new Dictionary { { "cookie-key", "cookie-value" } }); diff --git a/tracer/test/Datadog.Trace.Security.IntegrationTests/ApiSecurity/AspNetMvc5ApiSecurity.cs b/tracer/test/Datadog.Trace.Security.IntegrationTests/ApiSecurity/AspNetMvc5ApiSecurity.cs index a782d2c36413..999757271992 100644 --- a/tracer/test/Datadog.Trace.Security.IntegrationTests/ApiSecurity/AspNetMvc5ApiSecurity.cs +++ b/tracer/test/Datadog.Trace.Security.IntegrationTests/ApiSecurity/AspNetMvc5ApiSecurity.cs @@ -47,11 +47,7 @@ internal AspNetMvc5ApiSecurity(IisFixture iisIisFixture, ITestOutputHelper outpu { SetSecurity(true); EnvironmentHelper.CustomEnvironmentVariables.Add(ConfigurationKeys.AppSec.Rules, "ApiSecurity\\ruleset-with-block.json"); - if (enableApiSecurity) - { - EnvironmentHelper.CustomEnvironmentVariables.Add(ConfigurationKeys.AppSec.ApiSecurityEnabled, "true"); - } - + SetEnvironmentVariable(ConfigurationKeys.AppSec.ApiSecurityEnabled, enableApiSecurity.ToString()); AddCookies(new Dictionary { { "cookie-key", "cookie-value" } }); _iisFixture = iisIisFixture; _testName = "Security." + nameof(AspNetMvc5ApiSecurity) + ".enableApiSecurity=" + enableApiSecurity; diff --git a/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetBase.cs b/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetBase.cs index 5d2112756a28..339b404c956a 100644 --- a/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetBase.cs +++ b/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetBase.cs @@ -83,6 +83,7 @@ public AspNetBase(string sampleName, ITestOutputHelper outputHelper, string shut _jsonSerializerSettingsOrderProperty = new JsonSerializerSettings { ContractResolver = new OrderedContractResolver() }; _clearMetaStruct = clearMetaStruct; + SetEnvironmentVariable(ConfigurationKeys.AppSec.ApiSecurityEnabled, "false"); } protected bool IncludeAllHttpSpans { get; set; } = false; diff --git a/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetCore5AutoUserEvents.cs b/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetCore5AutoUserEvents.cs index 1163446fadc7..4e7283a331c3 100644 --- a/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetCore5AutoUserEvents.cs +++ b/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetCore5AutoUserEvents.cs @@ -8,6 +8,7 @@ #pragma warning disable SA1649 // File name must match first type name using System.Threading.Tasks; +using Datadog.Trace.Configuration; using Datadog.Trace.TestHelpers; using Xunit; using Xunit.Abstractions; diff --git a/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetCoreBare.cs b/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetCoreBare.cs index e4fcb19f7099..04cf596eb6d5 100644 --- a/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetCoreBare.cs +++ b/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetCoreBare.cs @@ -8,6 +8,7 @@ using System; using System.Net; using System.Threading.Tasks; +using Datadog.Trace.Configuration; using Datadog.Trace.TestHelpers; using Xunit; using Xunit.Abstractions; diff --git a/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetCoreBase.cs b/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetCoreBase.cs index 0b88c41ae770..80cc36a35c63 100644 --- a/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetCoreBase.cs +++ b/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetCoreBase.cs @@ -8,8 +8,8 @@ using System.Net; using System.Threading.Tasks; using Datadog.Trace.AppSec; +using Datadog.Trace.Configuration; using Datadog.Trace.TestHelpers; -using Datadog.Trace.Vendors.Newtonsoft.Json; using Xunit; using Xunit.Abstractions; diff --git a/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetMvc5.cs b/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetMvc5.cs index 301f226b7cb1..c70329c02f84 100644 --- a/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetMvc5.cs +++ b/tracer/test/Datadog.Trace.Security.IntegrationTests/AspNetMvc5.cs @@ -65,6 +65,7 @@ public AspNetMvc5(IisFixture iisFixture, ITestOutputHelper output, bool classicM SetSecurity(enableSecurity); SetEnvironmentVariable(Configuration.ConfigurationKeys.AppSec.Rules, DefaultRuleFile); SetEnvironmentVariable(Configuration.ConfigurationKeys.DebugEnabled, "1"); + SetEnvironmentVariable(Configuration.ConfigurationKeys.AppSec.ApiSecurityEnabled, "false"); _classicMode = classicMode; _iisFixture = iisFixture; diff --git a/tracer/test/Datadog.Trace.Security.IntegrationTests/Rcm/RcmBase.cs b/tracer/test/Datadog.Trace.Security.IntegrationTests/Rcm/RcmBase.cs index bee5eeca14d4..ad9a88bade75 100644 --- a/tracer/test/Datadog.Trace.Security.IntegrationTests/Rcm/RcmBase.cs +++ b/tracer/test/Datadog.Trace.Security.IntegrationTests/Rcm/RcmBase.cs @@ -25,6 +25,7 @@ protected RcmBase(AspNetCoreTestFixture fixture, ITestOutputHelper outputHelper, Fixture = fixture; EnableSecurity = enableSecurity; SetEnvironmentVariable(ConfigurationKeys.Rcm.PollInterval, "0.5"); + SetEnvironmentVariable(ConfigurationKeys.AppSec.ApiSecurityEnabled, "false"); } protected AspNetCoreTestFixture Fixture { get; } From 43b2292aa16ec2adeeff08ba630be8b031222a2a Mon Sep 17 00:00:00 2001 From: Tony Redondo Date: Thu, 3 Oct 2024 14:39:59 +0200 Subject: [PATCH 29/38] [CI Visibility] Fix code coverage enabled tag behavior (#6111) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary of changes This PR fixes the current behavior of the `test.code_coverage.enabled` tag to only set it to true when Intelligent Test Runner is enabled and collecting coverage data per test ## Reason for change .NET implementation was setting this value also on global code coverage reporting, being different to other tracers implementations ## Implementation details Remove the set tag when reporting the global code coverage from third party libraries. ## Test coverage Updated the code coverage reporting test to check we don't set the tag anymore. ## Other details --- tracer/src/Datadog.Trace/Ci/TestSession.cs | 1 - .../AutoInstrumentation/Testing/DotnetTest/DotnetCommon.cs | 1 - .../CiRunCommandTests.cs | 2 +- 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/tracer/src/Datadog.Trace/Ci/TestSession.cs b/tracer/src/Datadog.Trace/Ci/TestSession.cs index a2a86647d4af..0d0c91926532 100644 --- a/tracer/src/Datadog.Trace/Ci/TestSession.cs +++ b/tracer/src/Datadog.Trace/Ci/TestSession.cs @@ -533,7 +533,6 @@ private void OnIpcMessageReceived(object message) CIVisibility.Log.Information("TestSession.ReceiveMessage (code coverage): {Value}", codeCoverageMessage.Value); // Adds the global code coverage percentage to the session - SetTag(CodeCoverageTags.Enabled, "true"); SetTag(CodeCoverageTags.PercentageOfTotalLines, codeCoverageMessage.Value); } } diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Testing/DotnetTest/DotnetCommon.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Testing/DotnetTest/DotnetCommon.cs index fd5cc529d420..d4d305f4f9c0 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Testing/DotnetTest/DotnetCommon.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Testing/DotnetTest/DotnetCommon.cs @@ -155,7 +155,6 @@ internal static void FinalizeSession(TestSession? session, int exitCode, Excepti { if (TryGetCoveragePercentageFromXml(extCodeCoverageFilePath, out var coveragePercentage)) { - session.SetTag(CodeCoverageTags.Enabled, "true"); session.SetTag(CodeCoverageTags.PercentageOfTotalLines, coveragePercentage); } else diff --git a/tracer/test/Datadog.Trace.Tools.Runner.IntegrationTests/CiRunCommandTests.cs b/tracer/test/Datadog.Trace.Tools.Runner.IntegrationTests/CiRunCommandTests.cs index 3c2b26fe5864..a977dfb78bce 100644 --- a/tracer/test/Datadog.Trace.Tools.Runner.IntegrationTests/CiRunCommandTests.cs +++ b/tracer/test/Datadog.Trace.Tools.Runner.IntegrationTests/CiRunCommandTests.cs @@ -118,7 +118,7 @@ private void RunExternalCoverageTest(string filePath) environmentVariables.Should().NotBeNull(); testSession.Should().NotBeNull(); - testSession.Meta.Should().Contain(new KeyValuePair(CodeCoverageTags.Enabled, "true")); + testSession.Meta.Should().NotContain(new KeyValuePair(CodeCoverageTags.Enabled, "true")); testSession.Metrics.Should().Contain(new KeyValuePair(CodeCoverageTags.PercentageOfTotalLines, 83.33)); } } From a4b13011aadb8aeba77e2604492390d8ceb3103e Mon Sep 17 00:00:00 2001 From: Gregory LEOCADIE Date: Thu, 3 Oct 2024 15:26:11 +0200 Subject: [PATCH 30/38] [Profiler] Bump libdatadog 13.1 (#6112) --- build/cmake/FindLibdatadog.cmake | 10 +++++----- .../Datadog.Profiler.Native.Windows.vcxproj | 4 ++-- .../Datadog.Profiler.Native.Windows/packages.config | 2 +- .../Datadog.Profiler.Native.vcxproj | 4 ++-- .../Datadog.Profiler.Native/packages.config | 2 +- .../Datadog.Profiler.Native.Tests.vcxproj | 4 ++-- .../test/Datadog.Profiler.Native.Tests/packages.config | 2 +- 7 files changed, 14 insertions(+), 14 deletions(-) diff --git a/build/cmake/FindLibdatadog.cmake b/build/cmake/FindLibdatadog.cmake index 2a96950b3231..46a839460547 100644 --- a/build/cmake/FindLibdatadog.cmake +++ b/build/cmake/FindLibdatadog.cmake @@ -4,22 +4,22 @@ endif() include(FetchContent) -set(LIBDATADOG_VERSION "v13.0.0" CACHE STRING "libdatadog version") +set(LIBDATADOG_VERSION "v13.1.0" CACHE STRING "libdatadog version") if (CMAKE_SYSTEM_PROCESSOR STREQUAL aarch64 OR CMAKE_SYSTEM_PROCESSOR STREQUAL arm64) if (DEFINED ENV{IsAlpine} AND "$ENV{IsAlpine}" MATCHES "true") - set(SHA256_LIBDATADOG "13a0e9966f2174ea17b1203f4174c9dae36c482d522c31e67bca3283efd0b550" CACHE STRING "libdatadog sha256") + set(SHA256_LIBDATADOG "9cddbc9ece4c2fe9a1f0ab5a7cfed218d617c5154f318e0bce9a6102b265c989" CACHE STRING "libdatadog sha256") set(FILE_TO_DOWNLOAD libdatadog-aarch64-alpine-linux-musl.tar.gz) else() - set(SHA256_LIBDATADOG "84f2006f2b00b018979f1e74f628cb8043413c1e20d19d610f95fceb7e14a8c1" CACHE STRING "libdatadog sha256") + set(SHA256_LIBDATADOG "db17a5873d82ef772f969582949b272dcd04044a0cd08b196d3820172a19814d" CACHE STRING "libdatadog sha256") set(FILE_TO_DOWNLOAD libdatadog-aarch64-unknown-linux-gnu.tar.gz) endif() else() if (DEFINED ENV{IsAlpine} AND "$ENV{IsAlpine}" MATCHES "true") - set(SHA256_LIBDATADOG "2d7a1926772a9079facfdbfe096881910f2053fe08e59189a6fed89e2b3f4c4d" CACHE STRING "libdatadog sha256") + set(SHA256_LIBDATADOG "46d0e6445fa1b0fbe8d079e6fa997fa10a4fef4084fe10f4b5886c92effc7be8" CACHE STRING "libdatadog sha256") set(FILE_TO_DOWNLOAD libdatadog-${CMAKE_SYSTEM_PROCESSOR}-alpine-linux-musl.tar.gz) else() - set(SHA256_LIBDATADOG "c0e94fcff4f8129a8e9a3c3805791e189c08ee7a64e803086f33dfec2a767b0d" CACHE STRING "libdatadog sha256") + set(SHA256_LIBDATADOG "adaf79470fd0b06ce6d63ae8f231e555fa12b70d5bf82565a96a25f59ea8071d" CACHE STRING "libdatadog sha256") set(FILE_TO_DOWNLOAD libdatadog-${CMAKE_SYSTEM_PROCESSOR}-unknown-linux-gnu.tar.gz) endif() endif() diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/Datadog.Profiler.Native.Windows.vcxproj b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/Datadog.Profiler.Native.Windows.vcxproj index fd04118092bb..6318b439fbd9 100644 --- a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/Datadog.Profiler.Native.Windows.vcxproj +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/Datadog.Profiler.Native.Windows.vcxproj @@ -1,6 +1,6 @@ - + Debug @@ -269,6 +269,6 @@ This project references NuGet package(s) that are missing on this computer. Use NuGet Package Restore to download them. For more information, see http://go.microsoft.com/fwlink/?LinkID=322105. The missing file is {0}. - + \ No newline at end of file diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/packages.config b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/packages.config index 5a1c0ff95227..8e9f505e1dc5 100644 --- a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/packages.config +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Windows/packages.config @@ -1,4 +1,4 @@  - + \ No newline at end of file diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native/Datadog.Profiler.Native.vcxproj b/profiler/src/ProfilerEngine/Datadog.Profiler.Native/Datadog.Profiler.Native.vcxproj index d1ac45d00950..c1e12cbfb5f9 100644 --- a/profiler/src/ProfilerEngine/Datadog.Profiler.Native/Datadog.Profiler.Native.vcxproj +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native/Datadog.Profiler.Native.vcxproj @@ -1,6 +1,6 @@  - + Debug @@ -469,6 +469,6 @@ This project references NuGet package(s) that are missing on this computer. Use NuGet Package Restore to download them. For more information, see http://go.microsoft.com/fwlink/?LinkID=322105. The missing file is {0}. - + \ No newline at end of file diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native/packages.config b/profiler/src/ProfilerEngine/Datadog.Profiler.Native/packages.config index 5a1c0ff95227..8e9f505e1dc5 100644 --- a/profiler/src/ProfilerEngine/Datadog.Profiler.Native/packages.config +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native/packages.config @@ -1,4 +1,4 @@  - + \ No newline at end of file diff --git a/profiler/test/Datadog.Profiler.Native.Tests/Datadog.Profiler.Native.Tests.vcxproj b/profiler/test/Datadog.Profiler.Native.Tests/Datadog.Profiler.Native.Tests.vcxproj index 7d57d28fa33b..c560696e3e8c 100644 --- a/profiler/test/Datadog.Profiler.Native.Tests/Datadog.Profiler.Native.Tests.vcxproj +++ b/profiler/test/Datadog.Profiler.Native.Tests/Datadog.Profiler.Native.Tests.vcxproj @@ -1,6 +1,6 @@  - + Debug @@ -210,6 +210,6 @@ - + \ No newline at end of file diff --git a/profiler/test/Datadog.Profiler.Native.Tests/packages.config b/profiler/test/Datadog.Profiler.Native.Tests/packages.config index deb4c676ab56..7e01b95979ee 100644 --- a/profiler/test/Datadog.Profiler.Native.Tests/packages.config +++ b/profiler/test/Datadog.Profiler.Native.Tests/packages.config @@ -1,6 +1,6 @@  - + \ No newline at end of file From 992259df381589dc078622d554496cb7338588a8 Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Tue, 8 Oct 2024 09:15:02 -0700 Subject: [PATCH 31/38] Update integration test snapshots --- .../snapshots/AwsLambdaTests.verified.txt | 173 +++++++++++++++--- 1 file changed, 145 insertions(+), 28 deletions(-) diff --git a/tracer/test/snapshots/AwsLambdaTests.verified.txt b/tracer/test/snapshots/AwsLambdaTests.verified.txt index 373d510ed821..7a480cedcbcf 100644 --- a/tracer/test/snapshots/AwsLambdaTests.verified.txt +++ b/tracer/test/snapshots/AwsLambdaTests.verified.txt @@ -577,10 +577,7 @@ runtime-id: Guid_10 }, Metrics: { - process_id: 0, - _dd.top_level: 1.0, - _dd.tracer_kr: 1.0, - _sampling_priority_v1: 1.0 + _dd.top_level: 1.0 } }, { @@ -595,10 +592,7 @@ runtime-id: Guid_11 }, Metrics: { - process_id: 0, - _dd.top_level: 1.0, - _dd.tracer_kr: 1.0, - _sampling_priority_v1: 1.0 + _dd.top_level: 1.0 } }, { @@ -613,10 +607,7 @@ runtime-id: Guid_12 }, Metrics: { - process_id: 0, - _dd.top_level: 1.0, - _dd.tracer_kr: 1.0, - _sampling_priority_v1: 1.0 + _dd.top_level: 1.0 } }, { @@ -667,10 +658,7 @@ runtime-id: Guid_15 }, Metrics: { - process_id: 0, - _dd.top_level: 1.0, - _dd.tracer_kr: 1.0, - _sampling_priority_v1: 1.0 + _dd.top_level: 1.0 } }, { @@ -883,10 +871,7 @@ runtime-id: Guid_27 }, Metrics: { - process_id: 0, - _dd.top_level: 1.0, - _dd.tracer_kr: 1.0, - _sampling_priority_v1: 1.0 + _dd.top_level: 1.0 } }, { @@ -901,10 +886,7 @@ runtime-id: Guid_28 }, Metrics: { - process_id: 0, - _dd.top_level: 1.0, - _dd.tracer_kr: 1.0, - _sampling_priority_v1: 1.0 + _dd.top_level: 1.0 } }, { @@ -919,10 +901,7 @@ runtime-id: Guid_29 }, Metrics: { - process_id: 0, - _dd.top_level: 1.0, - _dd.tracer_kr: 1.0, - _sampling_priority_v1: 1.0 + _dd.top_level: 1.0 } }, { @@ -1978,5 +1957,143 @@ Metrics: { _dd.top_level: 1.0 } + }, + { + TraceId: Id_19, + SpanId: Id_20, + Name: placeholder-operation, + Resource: placeholder-operation, + Service: placeholder-service, + Tags: { + language: dotnet, + runtime-id: Guid_10, + _dd.base_service: Samples.AWS.Lambda + }, + Metrics: { + process_id: 0, + _dd.top_level: 1.0, + _dd.tracer_kr: 1.0, + _sampling_priority_v1: 1.0 + } + }, + { + TraceId: Id_21, + SpanId: Id_22, + Name: placeholder-operation, + Resource: placeholder-operation, + Service: placeholder-service, + Tags: { + language: dotnet, + runtime-id: Guid_11, + _dd.base_service: Samples.AWS.Lambda + }, + Metrics: { + process_id: 0, + _dd.top_level: 1.0, + _dd.tracer_kr: 1.0, + _sampling_priority_v1: 1.0 + } + }, + { + TraceId: Id_23, + SpanId: Id_24, + Name: placeholder-operation, + Resource: placeholder-operation, + Service: placeholder-service, + Tags: { + language: dotnet, + runtime-id: Guid_12, + _dd.base_service: Samples.AWS.Lambda + }, + Metrics: { + process_id: 0, + _dd.top_level: 1.0, + _dd.tracer_kr: 1.0, + _sampling_priority_v1: 1.0 + } + }, + { + TraceId: Id_29, + SpanId: Id_30, + Name: placeholder-operation, + Resource: placeholder-operation, + Service: placeholder-service, + Tags: { + language: dotnet, + runtime-id: Guid_15, + _dd.base_service: Samples.AWS.Lambda + }, + Metrics: { + process_id: 0, + _dd.top_level: 1.0, + _dd.tracer_kr: 1.0, + _sampling_priority_v1: 1.0 + } + }, + { + TraceId: Id_53, + SpanId: Id_54, + Name: placeholder-operation, + Resource: placeholder-operation, + Service: placeholder-service, + Error: 1, + Tags: { + error.msg: Cannot assign requested address, + error.stack: Cannot assign requested address (SocketException), + error.type: System.Net.WebException, + language: dotnet, + runtime-id: Guid_27, + _dd.base_service: Samples.AWS.Lambda + }, + Metrics: { + process_id: 0, + _dd.top_level: 1.0, + _dd.tracer_kr: 1.0, + _sampling_priority_v1: 1.0 + } + }, + { + TraceId: Id_55, + SpanId: Id_56, + Name: placeholder-operation, + Resource: placeholder-operation, + Service: placeholder-service, + Error: 1, + Tags: { + error.msg: Cannot assign requested address, + error.stack: Cannot assign requested address (SocketException), + error.type: System.Net.WebException, + language: dotnet, + runtime-id: Guid_28, + _dd.base_service: Samples.AWS.Lambda + }, + Metrics: { + process_id: 0, + _dd.top_level: 1.0, + _dd.tracer_kr: 1.0, + _sampling_priority_v1: 1.0 + } + }, + { + TraceId: Id_57, + SpanId: Id_58, + Name: placeholder-operation, + Resource: placeholder-operation, + Service: placeholder-service, + Error: 1, + Tags: { + error.msg: Cannot assign requested address, + error.stack: Cannot assign requested address (SocketException), + error.type: System.Net.WebException, + language: dotnet, + runtime-id: Guid_29, + _dd.base_service: Samples.AWS.Lambda + }, + Metrics: { + process_id: 0, + _dd.top_level: 1.0, + _dd.tracer_kr: 1.0, + _sampling_priority_v1: 1.0 + } } ] \ No newline at end of file From f710538fe5b2732149cb7155fb9e1ba0922988b4 Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Wed, 9 Oct 2024 12:00:42 -0700 Subject: [PATCH 32/38] force ci From 1d962d7dfae6f2bfa7191e9c33d8318cef5cf8cf Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Wed, 16 Oct 2024 09:19:46 -0700 Subject: [PATCH 33/38] Revert "CreatePlaceholderScope takes WebHeaderCollection" This reverts commit ed98cb18c93ae291087793e1aede3a8df19cadc7. --- .../ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs index d08fe1962563..bff68c5f6a3d 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs @@ -58,6 +58,7 @@ internal static Scope SendStartInvocation(ILambdaExtensionRequest requestBuilder var tracer = Tracer.Instance; return CreatePlaceholderScope(tracer, headers); + } internal static void SendEndInvocation(ILambdaExtensionRequest requestBuilder, Scope scope, bool isError, string data) From 2363539e6566fb44f5cf7e9ccb8d5873a4fed2bc Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Wed, 16 Oct 2024 09:20:00 -0700 Subject: [PATCH 34/38] Revert "Reorganize LambdaCommon a little bit" This reverts commit 83362d6cab79b732308133ce81f7dd665b6d71eb. --- .../AWS/Lambda/LambdaCommon.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs index bff68c5f6a3d..b5b4aae06bd5 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs @@ -61,6 +61,21 @@ internal static Scope SendStartInvocation(ILambdaExtensionRequest requestBuilder } + internal static Scope CreatePlaceholderScope(Tracer tracer, Dictionary myHeaders) + { + var spanContext = SpanContextPropagator.Instance.Extract(myHeaders); + TelemetryFactory.Metrics.RecordCountSpanCreated(MetricTags.IntegrationName.AwsLambda); + + if (spanContext != null) + { + // The problem is, we're not setting span.SamplingPriority + return tracer.StartActiveInternal(operationName: PlaceholderOperationName, parent: spanContext, serviceName: PlaceholderServiceName); + } + + var span = tracer.StartSpan(PlaceholderOperationName, tags: null, serviceName: PlaceholderServiceName, addToTraceContext: false); + return tracer.TracerManager.ScopeManager.Activate(span, false); + } + internal static void SendEndInvocation(ILambdaExtensionRequest requestBuilder, Scope scope, bool isError, string data) { var request = requestBuilder.GetEndInvocationRequest(scope, isError); From c6377c1444cff2c085a9dcab62019afd893ade2c Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Wed, 16 Oct 2024 09:20:22 -0700 Subject: [PATCH 35/38] Revert "Cherry pick commits from v3" This reverts commit d2a6d929846196ba6257c007b3a6ba86bde751d7. --- .../AWS/Lambda/LambdaCommon.cs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs index b5b4aae06bd5..d08fe1962563 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs @@ -58,22 +58,6 @@ internal static Scope SendStartInvocation(ILambdaExtensionRequest requestBuilder var tracer = Tracer.Instance; return CreatePlaceholderScope(tracer, headers); - - } - - internal static Scope CreatePlaceholderScope(Tracer tracer, Dictionary myHeaders) - { - var spanContext = SpanContextPropagator.Instance.Extract(myHeaders); - TelemetryFactory.Metrics.RecordCountSpanCreated(MetricTags.IntegrationName.AwsLambda); - - if (spanContext != null) - { - // The problem is, we're not setting span.SamplingPriority - return tracer.StartActiveInternal(operationName: PlaceholderOperationName, parent: spanContext, serviceName: PlaceholderServiceName); - } - - var span = tracer.StartSpan(PlaceholderOperationName, tags: null, serviceName: PlaceholderServiceName, addToTraceContext: false); - return tracer.TracerManager.ScopeManager.Activate(span, false); } internal static void SendEndInvocation(ILambdaExtensionRequest requestBuilder, Scope scope, bool isError, string data) From f6ef838fee918c0cbd53278d65e1001730868f10 Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Wed, 16 Oct 2024 21:51:02 -0700 Subject: [PATCH 36/38] CreatePlaceholderScope takes NameValueHeadersCollection --- .../AWS/Lambda/LambdaCommon.cs | 14 ++++---------- .../test/Datadog.Trace.Tests/LambdaCommonTests.cs | 15 ++++++++------- .../LambdaRequestBuilderTests.cs | 11 ++++++----- 3 files changed, 18 insertions(+), 22 deletions(-) diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs index d08fe1962563..e633d1167515 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs @@ -8,6 +8,7 @@ using System.Net; using System.Text; using System.Threading.Tasks; +using Datadog.Trace.Headers; using Datadog.Trace.Propagators; using Datadog.Trace.Telemetry; using Datadog.Trace.Telemetry.Metrics; @@ -22,9 +23,9 @@ internal abstract class LambdaCommon private const double ServerlessMaxWaitingFlushTime = 3; private const string LogLevelEnvName = "DD_LOG_LEVEL"; - internal static Scope CreatePlaceholderScope(Tracer tracer, WebHeaderCollection headers) + internal static Scope CreatePlaceholderScope(Tracer tracer, NameValueHeadersCollection headers) { - var spanContext = SpanContextPropagator.Instance.Extract(headers, SafeGetValues); + var spanContext = SpanContextPropagator.Instance.Extract(headers); TelemetryFactory.Metrics.RecordCountSpanCreated(MetricTags.IntegrationName.AwsLambda); if (spanContext != null) @@ -36,13 +37,6 @@ internal static Scope CreatePlaceholderScope(Tracer tracer, WebHeaderCollection return tracer.TracerManager.ScopeManager.Activate(span, false); } - // parseUtility.ParseUInt64 falls over and dies when it tries to parse a null collection of values, and WebHeadersCollection.GetValues will return null if the header does not exist. - private static string[] SafeGetValues(WebHeaderCollection headers, string name) - { - var values = headers.GetValues(name); - return values ?? []; - } - internal static Scope SendStartInvocation(ILambdaExtensionRequest requestBuilder, string data, IDictionary context) { var request = requestBuilder.GetStartInvocationRequest(); @@ -50,7 +44,7 @@ internal static Scope SendStartInvocation(ILambdaExtensionRequest requestBuilder WriteRequestHeaders(request, context); var response = (HttpWebResponse)request.GetResponse(); - var headers = response.Headers; + var headers = response.Headers.Wrap(); if (!ValidateOkStatus(response)) { return null; diff --git a/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs b/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs index a17905d4b473..e2cb37ddb074 100644 --- a/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs +++ b/tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs @@ -9,6 +9,7 @@ using System.Net; using System.Threading.Tasks; using Datadog.Trace.ClrProfiler.AutoInstrumentation.AWS.Lambda; +using Datadog.Trace.ExtensionMethods; using Datadog.Trace.TestHelpers; using FluentAssertions; @@ -28,7 +29,7 @@ public async Task TestCreatePlaceholderScopeSuccessWithTraceIdOnly() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" } }; + var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" } }.Wrap(); var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); scope.Should().NotBeNull(); scope.Span.TraceId128.Should().Be((TraceId)1234); @@ -40,7 +41,7 @@ public async Task TestCreatePlaceholderScopeSuccessWithTraceIdOnly() public async Task TestCreatePlaceholderScopeSuccessWith64BitTraceIdContext() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; + var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }.Wrap(); var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); scope.Should().NotBeNull(); @@ -54,7 +55,7 @@ public async Task TestCreatePlaceholderScopeSuccessWith64BitTraceIdContext() public async Task TestCreatePlaceholderScopeSuccessWith128BitTraceIdContext() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "5744042798732701615" }, { HttpHeaderNames.SamplingPriority, "-1" }, { HttpHeaderNames.PropagatedTags, "_dd.p.tid=1914fe7789eb32be" } }; + var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "5744042798732701615" }, { HttpHeaderNames.SamplingPriority, "-1" }, { HttpHeaderNames.PropagatedTags, "_dd.p.tid=1914fe7789eb32be" } }.Wrap(); var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); scope.Should().NotBeNull(); @@ -68,7 +69,7 @@ public async Task TestCreatePlaceholderScopeSuccessWith128BitTraceIdContext() public async Task TestCreatePlaceholderScopeSuccessWithoutContext() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var scope = LambdaCommon.CreatePlaceholderScope(tracer, new WebHeaderCollection()); + var scope = LambdaCommon.CreatePlaceholderScope(tracer, new WebHeaderCollection().Wrap()); scope.Should().NotBeNull(); scope.Span.TraceId128.Should().BeGreaterThan((TraceId.Zero)); @@ -136,7 +137,7 @@ public void TestSendStartInvocationSuccess() public async Task TestSendEndInvocationFailure() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; + var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }.Wrap(); var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); var response = new Mock(MockBehavior.Loose); @@ -157,7 +158,7 @@ public async Task TestSendEndInvocationFailure() public async Task TestSendEndInvocationSuccess() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; + var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }.Wrap(); var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); var response = new Mock(MockBehavior.Loose); @@ -181,7 +182,7 @@ public async Task TestSendEndInvocationSuccess() public async Task TestSendEndInvocationFalse() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; + var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }.Wrap(); var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); var response = new Mock(MockBehavior.Loose); diff --git a/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs b/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs index 313183dd6b8e..ebb86437d365 100644 --- a/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs +++ b/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs @@ -6,6 +6,7 @@ using System.Net; using System.Threading.Tasks; using Datadog.Trace.ClrProfiler.AutoInstrumentation.AWS.Lambda; +using Datadog.Trace.ExtensionMethods; using Datadog.Trace.TestHelpers; using FluentAssertions; @@ -23,7 +24,7 @@ public class LambdaRequestBuilderTests public async Task TestGetEndInvocationRequestWithError() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var headers = new WebHeaderCollection(); + var headers = new WebHeaderCollection().Wrap(); var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder(); @@ -39,7 +40,7 @@ public async Task TestGetEndInvocationRequestWithError() public async Task TestGetEndInvocationRequestWithoutError() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var headers = new WebHeaderCollection(); + var headers = new WebHeaderCollection().Wrap(); var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder(); @@ -55,7 +56,7 @@ public async Task TestGetEndInvocationRequestWithoutError() public async Task TestGetEndInvocationRequestWithScope() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }; + var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }.Wrap(); var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder(); @@ -83,7 +84,7 @@ public void TestGetEndInvocationRequestWithoutScope() public async Task TestGetEndInvocationRequestWithErrorTags() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var headers = new WebHeaderCollection(); + var headers = new WebHeaderCollection().Wrap(); var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); scope.Span.SetTag("error.msg", "Exception"); scope.Span.SetTag("error.type", "Exception"); @@ -105,7 +106,7 @@ public async Task TestGetEndInvocationRequestWithErrorTags() public async Task TestGetEndInvocationRequestWithoutErrorTags() { await using var tracer = TracerHelper.CreateWithFakeAgent(); - var headers = new WebHeaderCollection(); + var headers = new WebHeaderCollection().Wrap(); var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers); ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder(); From a898b31151a100d15b3858c2878752596857481a Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Mon, 21 Oct 2024 10:38:58 -0700 Subject: [PATCH 37/38] Use StartSpan instead of StartActiveInternal in CreatePlaceholderScope --- .../AutoInstrumentation/AWS/Lambda/LambdaCommon.cs | 7 ++----- .../test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs | 2 -- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs index e633d1167515..c7a86ea6c1d5 100644 --- a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs +++ b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/Lambda/LambdaCommon.cs @@ -28,12 +28,9 @@ internal static Scope CreatePlaceholderScope(Tracer tracer, NameValueHeadersColl var spanContext = SpanContextPropagator.Instance.Extract(headers); TelemetryFactory.Metrics.RecordCountSpanCreated(MetricTags.IntegrationName.AwsLambda); - if (spanContext != null) - { - return tracer.StartActiveInternal(operationName: PlaceholderOperationName, parent: spanContext, serviceName: PlaceholderServiceName); - } + var span = spanContext != null ? tracer.StartSpan(PlaceholderOperationName, tags: null, parent: spanContext, serviceName: PlaceholderServiceName, addToTraceContext: false) : tracer.StartSpan(PlaceholderOperationName, tags: null, serviceName: PlaceholderServiceName, addToTraceContext: false); - var span = tracer.StartSpan(PlaceholderOperationName, tags: null, serviceName: PlaceholderServiceName, addToTraceContext: false); + TelemetryFactory.Metrics.RecordCountSpanCreated(MetricTags.IntegrationName.AwsLambda); return tracer.TracerManager.ScopeManager.Activate(span, false); } diff --git a/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs b/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs index ebb86437d365..dffadf7660cb 100644 --- a/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs +++ b/tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs @@ -15,8 +15,6 @@ namespace Datadog.Trace.Tests { - extern alias DatadogTraceManual; - [Collection(nameof(WebRequestCollection))] public class LambdaRequestBuilderTests { From 83201d8c85c93f29f0c5e9bd537b3ef06ec3941b Mon Sep 17 00:00:00 2001 From: "chris.agocs" Date: Mon, 21 Oct 2024 11:54:02 -0700 Subject: [PATCH 38/38] Update integration test snapshots --- .../snapshots/AwsLambdaTests.verified.txt | 173 +++--------------- 1 file changed, 28 insertions(+), 145 deletions(-) diff --git a/tracer/test/snapshots/AwsLambdaTests.verified.txt b/tracer/test/snapshots/AwsLambdaTests.verified.txt index 7a480cedcbcf..373d510ed821 100644 --- a/tracer/test/snapshots/AwsLambdaTests.verified.txt +++ b/tracer/test/snapshots/AwsLambdaTests.verified.txt @@ -577,7 +577,10 @@ runtime-id: Guid_10 }, Metrics: { - _dd.top_level: 1.0 + process_id: 0, + _dd.top_level: 1.0, + _dd.tracer_kr: 1.0, + _sampling_priority_v1: 1.0 } }, { @@ -592,7 +595,10 @@ runtime-id: Guid_11 }, Metrics: { - _dd.top_level: 1.0 + process_id: 0, + _dd.top_level: 1.0, + _dd.tracer_kr: 1.0, + _sampling_priority_v1: 1.0 } }, { @@ -607,7 +613,10 @@ runtime-id: Guid_12 }, Metrics: { - _dd.top_level: 1.0 + process_id: 0, + _dd.top_level: 1.0, + _dd.tracer_kr: 1.0, + _sampling_priority_v1: 1.0 } }, { @@ -658,7 +667,10 @@ runtime-id: Guid_15 }, Metrics: { - _dd.top_level: 1.0 + process_id: 0, + _dd.top_level: 1.0, + _dd.tracer_kr: 1.0, + _sampling_priority_v1: 1.0 } }, { @@ -871,7 +883,10 @@ runtime-id: Guid_27 }, Metrics: { - _dd.top_level: 1.0 + process_id: 0, + _dd.top_level: 1.0, + _dd.tracer_kr: 1.0, + _sampling_priority_v1: 1.0 } }, { @@ -886,7 +901,10 @@ runtime-id: Guid_28 }, Metrics: { - _dd.top_level: 1.0 + process_id: 0, + _dd.top_level: 1.0, + _dd.tracer_kr: 1.0, + _sampling_priority_v1: 1.0 } }, { @@ -901,7 +919,10 @@ runtime-id: Guid_29 }, Metrics: { - _dd.top_level: 1.0 + process_id: 0, + _dd.top_level: 1.0, + _dd.tracer_kr: 1.0, + _sampling_priority_v1: 1.0 } }, { @@ -1957,143 +1978,5 @@ Metrics: { _dd.top_level: 1.0 } - }, - { - TraceId: Id_19, - SpanId: Id_20, - Name: placeholder-operation, - Resource: placeholder-operation, - Service: placeholder-service, - Tags: { - language: dotnet, - runtime-id: Guid_10, - _dd.base_service: Samples.AWS.Lambda - }, - Metrics: { - process_id: 0, - _dd.top_level: 1.0, - _dd.tracer_kr: 1.0, - _sampling_priority_v1: 1.0 - } - }, - { - TraceId: Id_21, - SpanId: Id_22, - Name: placeholder-operation, - Resource: placeholder-operation, - Service: placeholder-service, - Tags: { - language: dotnet, - runtime-id: Guid_11, - _dd.base_service: Samples.AWS.Lambda - }, - Metrics: { - process_id: 0, - _dd.top_level: 1.0, - _dd.tracer_kr: 1.0, - _sampling_priority_v1: 1.0 - } - }, - { - TraceId: Id_23, - SpanId: Id_24, - Name: placeholder-operation, - Resource: placeholder-operation, - Service: placeholder-service, - Tags: { - language: dotnet, - runtime-id: Guid_12, - _dd.base_service: Samples.AWS.Lambda - }, - Metrics: { - process_id: 0, - _dd.top_level: 1.0, - _dd.tracer_kr: 1.0, - _sampling_priority_v1: 1.0 - } - }, - { - TraceId: Id_29, - SpanId: Id_30, - Name: placeholder-operation, - Resource: placeholder-operation, - Service: placeholder-service, - Tags: { - language: dotnet, - runtime-id: Guid_15, - _dd.base_service: Samples.AWS.Lambda - }, - Metrics: { - process_id: 0, - _dd.top_level: 1.0, - _dd.tracer_kr: 1.0, - _sampling_priority_v1: 1.0 - } - }, - { - TraceId: Id_53, - SpanId: Id_54, - Name: placeholder-operation, - Resource: placeholder-operation, - Service: placeholder-service, - Error: 1, - Tags: { - error.msg: Cannot assign requested address, - error.stack: Cannot assign requested address (SocketException), - error.type: System.Net.WebException, - language: dotnet, - runtime-id: Guid_27, - _dd.base_service: Samples.AWS.Lambda - }, - Metrics: { - process_id: 0, - _dd.top_level: 1.0, - _dd.tracer_kr: 1.0, - _sampling_priority_v1: 1.0 - } - }, - { - TraceId: Id_55, - SpanId: Id_56, - Name: placeholder-operation, - Resource: placeholder-operation, - Service: placeholder-service, - Error: 1, - Tags: { - error.msg: Cannot assign requested address, - error.stack: Cannot assign requested address (SocketException), - error.type: System.Net.WebException, - language: dotnet, - runtime-id: Guid_28, - _dd.base_service: Samples.AWS.Lambda - }, - Metrics: { - process_id: 0, - _dd.top_level: 1.0, - _dd.tracer_kr: 1.0, - _sampling_priority_v1: 1.0 - } - }, - { - TraceId: Id_57, - SpanId: Id_58, - Name: placeholder-operation, - Resource: placeholder-operation, - Service: placeholder-service, - Error: 1, - Tags: { - error.msg: Cannot assign requested address, - error.stack: Cannot assign requested address (SocketException), - error.type: System.Net.WebException, - language: dotnet, - runtime-id: Guid_29, - _dd.base_service: Samples.AWS.Lambda - }, - Metrics: { - process_id: 0, - _dd.top_level: 1.0, - _dd.tracer_kr: 1.0, - _sampling_priority_v1: 1.0 - } } ] \ No newline at end of file