Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract Upper64 bit trace ID from extension response #6041

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
69b6163
Extract Upper64 bit trace ID from extension response
agocs Sep 16, 2024
42c242a
Use SpanContextPropagator.Instance.Extract()
agocs Sep 24, 2024
94d9f24
replace CreateNewPlaceholderScope with NewCreatePlaceholderScope in t…
agocs Sep 25, 2024
00c5475
Remove createPlaceholderScope
agocs Sep 25, 2024
55245cd
Rename NewCreatePlaceholderScope to CreatePlaceholderscope
agocs Sep 25, 2024
ad19246
Merge branch 'master' into chris.agocs/parse_128_bit_trace_id_from_la…
agocs Sep 25, 2024
cee09c0
Savepoint
agocs Sep 30, 2024
ddd0fb6
Fix test for 128 bit trace IDs
agocs Sep 30, 2024
b300797
Reorganize LambdaCommon a little bit
agocs Sep 30, 2024
7b9bb9f
Merge branch 'master' into chris.agocs/parse_128_bit_trace_id_from_la…
agocs Sep 30, 2024
3fd8cd6
CreatePlaceholderScope takes WebHeaderCollection
agocs Oct 1, 2024
357f279
Merge branch 'chris.agocs/parse_128_bit_trace_id_from_lambda_extensio…
agocs Oct 1, 2024
9fb8fcd
Get empty string array from WebHeadersCollection instead of null
agocs Oct 2, 2024
ad30105
Update tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/L…
agocs Oct 3, 2024
a56874d
Merge branch 'master' into chris.agocs/parse_128_bit_trace_id_from_la…
agocs Oct 3, 2024
d2a6d92
Cherry pick commits from v3
agocs Sep 30, 2024
274d538
Fix test for 128 bit trace IDs
agocs Sep 30, 2024
83362d6
Reorganize LambdaCommon a little bit
agocs Sep 30, 2024
ed98cb1
CreatePlaceholderScope takes WebHeaderCollection
agocs Oct 1, 2024
52ccc64
Get empty string array from WebHeadersCollection instead of null
agocs Oct 2, 2024
60ccc82
Update tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AWS/L…
agocs Oct 3, 2024
bf5a010
Minor build fixes (#6099)
andrewlock Sep 30, 2024
7376508
[IAST] Fix for VS Edit and Continue (#6097)
daniel-romano-DD Oct 1, 2024
c0c88f4
Remove warning about non-serializable test data (#6102)
bouwkast Oct 1, 2024
1bf0873
[ASM][RCM] Read waf actions per file and remove per file (#6072)
anna-git Oct 2, 2024
758301f
Run CallTargetNativeTests on Windows (#5754)
andrewlock Oct 2, 2024
6e7966a
[ASM] Suspicious attacker blocking (#6057)
NachoEchevarria Oct 2, 2024
2b5c0dc
chore: prefix system tests env vars (#6108)
wconti27 Oct 2, 2024
6f1be32
[Crashtracking] Implement support for Windows (#6088)
kevingosse Oct 3, 2024
587426a
[Crashtracking] Add registry key to installer (#6106)
kevingosse Oct 3, 2024
0ec8f01
[Crashtracking] Remove exception types that are too broad (#6109)
kevingosse Oct 3, 2024
a1838fa
[ASM] Activate api sec by default (#6043)
anna-git Oct 3, 2024
43b2292
[CI Visibility] Fix code coverage enabled tag behavior (#6111)
tonyredondo Oct 3, 2024
a4b1301
[Profiler] Bump libdatadog 13.1 (#6112)
gleocadie Oct 3, 2024
992259d
Update integration test snapshots
agocs Oct 8, 2024
ce40a26
Merge branch 'chris.agocs/parse_128_bit_trace_id_from_lambda_extensio…
agocs Oct 8, 2024
f710538
force ci
agocs Oct 9, 2024
1d962d7
Revert "CreatePlaceholderScope takes WebHeaderCollection"
agocs Oct 16, 2024
2363539
Revert "Reorganize LambdaCommon a little bit"
agocs Oct 16, 2024
c6377c1
Revert "Cherry pick commits from v3"
agocs Oct 16, 2024
f6ef838
CreatePlaceholderScope takes NameValueHeadersCollection
agocs Oct 17, 2024
a898b31
Use StartSpan instead of StartActiveInternal in CreatePlaceholderScope
agocs Oct 21, 2024
83201d8
Update integration test snapshots
agocs Oct 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
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;
Expand All @@ -21,31 +23,12 @@ 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, NameValueHeadersCollection headers)
{
Span span;
var spanContext = SpanContextPropagator.Instance.Extract(headers);
TelemetryFactory.Metrics.RecordCountSpanCreated(MetricTags.IntegrationName.AwsLambda);

if (traceId == null)
{
Log("traceId not found");
span = tracer.StartSpan(PlaceholderOperationName, tags: null, serviceName: PlaceholderServiceName, addToTraceContext: false);
}
else
{
Log($"creating the placeholder traceId = {traceId}");
span = tracer.StartSpan(PlaceholderOperationName, tags: null, serviceName: PlaceholderServiceName, traceId: (TraceId)Convert.ToUInt64(traceId), 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);
}
var span = spanContext != null ? tracer.StartSpan(PlaceholderOperationName, tags: null, parent: spanContext, serviceName: PlaceholderServiceName, addToTraceContext: false) : tracer.StartSpan(PlaceholderOperationName, tags: null, serviceName: PlaceholderServiceName, addToTraceContext: false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick

Suggested change
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, parent: spanContext, serviceName: PlaceholderServiceName, addToTraceContext: false);

I don't think the spanContext != null is necessary as we'll always be calling the same StartSpan function and parent can be set to null - that is the default value.


TelemetryFactory.Metrics.RecordCountSpanCreated(MetricTags.IntegrationName.AwsLambda);
return tracer.TracerManager.ScopeManager.Activate(span, false);
Expand All @@ -57,14 +40,15 @@ 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 samplingPriority = response.Headers.Get(HttpHeaderNames.SamplingPriority);
if (ValidateOkStatus(response))

var headers = response.Headers.Wrap();
if (!ValidateOkStatus(response))
{
return CreatePlaceholderScope(Tracer.Instance, traceId, samplingPriority);
return null;
}

return null;
var tracer = Tracer.Instance;
return CreatePlaceholderScope(tracer, headers);
}

internal static void SendEndInvocation(ILambdaExtensionRequest requestBuilder, Scope scope, bool isError, string data)
Expand Down
48 changes: 21 additions & 27 deletions tracer/test/Datadog.Trace.Tests/LambdaCommonTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -17,6 +18,8 @@

namespace Datadog.Trace.Tests
{
extern alias DatadogTraceManual;

public class LambdaCommonTests
{
private readonly Mock<ILambdaExtensionRequest> _lambdaRequestMock = new();
Expand All @@ -25,36 +28,38 @@ public class LambdaCommonTests
public async Task TestCreatePlaceholderScopeSuccessWithTraceIdOnly()
{
await using var tracer = TracerHelper.CreateWithFakeAgent();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, "1234", null);

var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" } }.Wrap();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers);
scope.Should().NotBeNull();
scope.Span.TraceId128.Should().Be((TraceId)1234);
((ISpan)scope.Span).TraceId.Should().Be(1234);
scope.Span.SpanId.Should().BeGreaterThan(0);
}

[Fact]
public async Task TestCreatePlaceholderScopeSuccessWithSamplingPriorityOnly()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wound up removing this test since dd-trace doesn't seem to allow a distributed trace with no trace ID.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right. A trace id is required when extracting trace context from headers, otherwise we ignore all the other data. But what changed here? Are we now trying to propagate trace context without a trace id?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But what changed here?

Oh, I see. The null in LambdaCommon.CreatePlaceholderScope(tracer, null, "-1") below was the trace id, and it was handled differently since the previous code was not using SpanContextPropagator.

public async Task TestCreatePlaceholderScopeSuccessWith64BitTraceIdContext()
{
await using var tracer = TracerHelper.CreateWithFakeAgent();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, null, "-1");
var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }.Wrap();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers);

scope.Should().NotBeNull();
scope.Span.TraceId128.Should().BeGreaterThan(TraceId.Zero);
((ISpan)scope.Span).TraceId.Should().BeGreaterThan(0);
scope.Span.TraceId128.Should().Be((TraceId)1234);
((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 TestCreatePlaceholderScopeSuccessWithFullContext()
public async Task TestCreatePlaceholderScopeSuccessWith128BitTraceIdContext()
{
await using var tracer = TracerHelper.CreateWithFakeAgent();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, "1234", "-1");
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();
scope.Span.TraceId128.Should().Be((TraceId)1234);
((ISpan)scope.Span).TraceId.Should().Be(1234);
scope.Span.TraceId128.ToString().Should().Be("1914fe7789eb32be4fb6f07e011a6faf");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value agrees with dd-trace-java

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed that 1914fe7789eb32be4fb6f07e011a6faf has

  • 1914fe7789eb32be in the upper 64 bits
  • 4fb6f07e011a6faf (5744042798732701615) in the lower 64 bits

scope.Span.SpanId.Should().BeGreaterThan(0);
scope.Span.Context.TraceContext.SamplingPriority.Should().Be(-1);
}
Expand All @@ -64,28 +69,14 @@ public async Task TestCreatePlaceholderScopeSuccessWithFullContext()
public async Task TestCreatePlaceholderScopeSuccessWithoutContext()
{
await using var tracer = TracerHelper.CreateWithFakeAgent();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, null, null);
var scope = LambdaCommon.CreatePlaceholderScope(tracer, new WebHeaderCollection().Wrap());

scope.Should().NotBeNull();
scope.Span.TraceId128.Should().BeGreaterThan((TraceId.Zero));
((ISpan)scope.Span).TraceId.Should().BeGreaterThan(0);
scope.Span.SpanId.Should().BeGreaterThan(0);
}

[Fact]
public async Task TestCreatePlaceholderScopeInvalidTraceId()
{
await using var tracer = TracerHelper.CreateWithFakeAgent();
Assert.Throws<FormatException>(() => LambdaCommon.CreatePlaceholderScope(tracer, "invalid-trace-id", "-1"));
}

[Fact]
public async Task TestCreatePlaceholderScopeInvalidSamplingPriority()
{
await using var tracer = TracerHelper.CreateWithFakeAgent();
Assert.Throws<FormatException>(() => LambdaCommon.CreatePlaceholderScope(tracer, "1234", "invalid-sampling-priority"));
}

[Fact]
[Trait("Category", "ArmUnsupported")]
public void TestSendStartInvocationThrow()
Expand Down Expand Up @@ -146,7 +137,8 @@ public void TestSendStartInvocationSuccess()
public async Task TestSendEndInvocationFailure()
{
await using var tracer = TracerHelper.CreateWithFakeAgent();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, "1234", "-1");
var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }.Wrap();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers);

var response = new Mock<HttpWebResponse>(MockBehavior.Loose);
var responseStream = new Mock<Stream>(MockBehavior.Loose);
Expand All @@ -166,7 +158,8 @@ public async Task TestSendEndInvocationFailure()
public async Task TestSendEndInvocationSuccess()
{
await using var tracer = TracerHelper.CreateWithFakeAgent();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, "1234", "-1");
var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }.Wrap();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers);

var response = new Mock<HttpWebResponse>(MockBehavior.Loose);
var responseStream = new Mock<Stream>(MockBehavior.Loose);
Expand All @@ -189,7 +182,8 @@ public async Task TestSendEndInvocationSuccess()
public async Task TestSendEndInvocationFalse()
{
await using var tracer = TracerHelper.CreateWithFakeAgent();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, "1234", "-1");
var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }.Wrap();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers);

var response = new Mock<HttpWebResponse>(MockBehavior.Loose);
var responseStream = new Mock<Stream>(MockBehavior.Loose);
Expand Down
17 changes: 12 additions & 5 deletions tracer/test/Datadog.Trace.Tests/LambdaRequestBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>
#if NET6_0_OR_GREATER
using System.Net;
using System.Threading.Tasks;
using Datadog.Trace.ClrProfiler.AutoInstrumentation.AWS.Lambda;
using Datadog.Trace.ExtensionMethods;
using Datadog.Trace.TestHelpers;

using FluentAssertions;
Expand All @@ -20,7 +22,8 @@ public class LambdaRequestBuilderTests
public async Task TestGetEndInvocationRequestWithError()
{
await using var tracer = TracerHelper.CreateWithFakeAgent();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, traceId: null, samplingPriority: null);
var headers = new WebHeaderCollection().Wrap();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers);

ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder();
var request = requestBuilder.GetEndInvocationRequest(scope, isError: true);
Expand All @@ -35,7 +38,8 @@ public async Task TestGetEndInvocationRequestWithError()
public async Task TestGetEndInvocationRequestWithoutError()
{
await using var tracer = TracerHelper.CreateWithFakeAgent();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, traceId: null, samplingPriority: null);
var headers = new WebHeaderCollection().Wrap();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers);

ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder();
var request = requestBuilder.GetEndInvocationRequest(scope, isError: false);
Expand All @@ -50,7 +54,8 @@ public async Task TestGetEndInvocationRequestWithoutError()
public async Task TestGetEndInvocationRequestWithScope()
{
await using var tracer = TracerHelper.CreateWithFakeAgent();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, traceId: "1234", samplingPriority: "-1");
var headers = new WebHeaderCollection { { HttpHeaderNames.TraceId, "1234" }, { HttpHeaderNames.SamplingPriority, "-1" } }.Wrap();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers);

ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder();
var request = requestBuilder.GetEndInvocationRequest(scope, isError: false);
Expand All @@ -77,7 +82,8 @@ public void TestGetEndInvocationRequestWithoutScope()
public async Task TestGetEndInvocationRequestWithErrorTags()
{
await using var tracer = TracerHelper.CreateWithFakeAgent();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, traceId: null, samplingPriority: null);
var headers = new WebHeaderCollection().Wrap();
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");
Expand All @@ -98,7 +104,8 @@ public async Task TestGetEndInvocationRequestWithErrorTags()
public async Task TestGetEndInvocationRequestWithoutErrorTags()
{
await using var tracer = TracerHelper.CreateWithFakeAgent();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, null, null);
var headers = new WebHeaderCollection().Wrap();
var scope = LambdaCommon.CreatePlaceholderScope(tracer, headers);

ILambdaExtensionRequest requestBuilder = new LambdaRequestBuilder();
var request = requestBuilder.GetEndInvocationRequest(scope, true);
Expand Down
Loading