Skip to content

Commit fa6192b

Browse files
authored
Swap disposal ordering of inferred span (#7293)
## Summary of changes Swaps the ordering when disposing the API Gateway inferred span to be after the "root" span. Realistically the API Gateway span will be the parent of the root span for the ASP NET Core instrumentation. ## Reason for change If the proxy scope is the parent of the `ASPNET Core` scope AND that `ASPNET Core` scope is active this was hitting the following [piece of code](https://github.com/DataDog/dd-trace-dotnet/blob/1752abdc32c02c702b7c8374bfd1bae8b381d7db/tracer/src/Datadog.Trace/AsyncLocalScopeManager.cs#L44-L48) in `Async ```csharp if (current == null || current != scope) { // This is not the current scope for this context, bail out return; } ``` This would cause the scope for the proxy to remain open 😢 ## Implementation details Swapped the ordering of them. ## Test coverage - Identified and covered by existing tests and was caught in [this](#7280) PR when updating xUnit ## Other details <!-- Fixes #{issue} --> Cherry picked out and making it's own PR instead of bundling into the [xUnit update one ](#7284) because it passes and works without that. <!-- ⚠️ Note: where possible, please obtain 2 approvals prior to merging. Unless CODEOWNERS specifies otherwise, for external teams it is typically best to have one review from a team member, and one review from apm-dotnet. Trivial changes do not require 2 reviews. -->
1 parent 6e3c6ba commit fa6192b

6 files changed

+154
-141
lines changed

tracer/src/Datadog.Trace/PlatformHelpers/AspNetCoreHttpRequestHandler.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,9 @@ public void StopAspNetCorePipelineScope(Tracer tracer, Security security, Scope
201201
}
202202

203203
CoreHttpContextStore.Instance.Remove();
204-
proxyScope?.Dispose();
204+
205205
rootScope.Dispose();
206+
proxyScope?.Dispose();
206207
}
207208
}
208209

tracer/test/snapshots/AspNetCoreMvc31InferredProxySpansTests.Enabled.__path=__statusCode=200_spanCount=3.verified.txt

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -18,33 +18,6 @@
1818
version: 1.0.0
1919
}
2020
},
21-
{
22-
TraceId: Id_1,
23-
SpanId: Id_4,
24-
Name: aws.apigateway,
25-
Resource: GET /api/test/?,
26-
Service: test.api.com,
27-
Type: web,
28-
ParentId: Id_5,
29-
Tags: {
30-
component: aws-apigateway,
31-
env: integration_tests,
32-
http.method: GET,
33-
http.response.headers.sample_correlation_identifier: 0000-0000-0000,
34-
http.response.headers.server: Kestrel,
35-
http.route: /api/test/?,
36-
http.status_code: 200,
37-
http.url: test.api.com/api/test/1,
38-
language: dotnet,
39-
span.kind: internal,
40-
stage: prod,
41-
_dd.base_service: Samples.AspNetCoreMvc31
42-
},
43-
Metrics: {
44-
_dd.inferred_span: 1.0,
45-
_sampling_priority_v1: 1.0
46-
}
47-
},
4821
{
4922
TraceId: Id_1,
5023
SpanId: Id_3,
@@ -76,5 +49,35 @@
7649
Metrics: {
7750
_dd.top_level: 1.0
7851
}
52+
},
53+
{
54+
TraceId: Id_1,
55+
SpanId: Id_4,
56+
Name: aws.apigateway,
57+
Resource: GET /api/test/?,
58+
Service: test.api.com,
59+
Type: web,
60+
Tags: {
61+
component: aws-apigateway,
62+
env: integration_tests,
63+
http.method: GET,
64+
http.response.headers.sample_correlation_identifier: 0000-0000-0000,
65+
http.response.headers.server: Kestrel,
66+
http.route: /api/test/?,
67+
http.status_code: 200,
68+
http.url: test.api.com/api/test/1,
69+
language: dotnet,
70+
runtime-id: Guid_1,
71+
span.kind: internal,
72+
stage: prod,
73+
_dd.base_service: Samples.AspNetCoreMvc31
74+
},
75+
Metrics: {
76+
process_id: 0,
77+
_dd.inferred_span: 1.0,
78+
_dd.top_level: 1.0,
79+
_dd.tracer_kr: 1.0,
80+
_sampling_priority_v1: 1.0
81+
}
7982
}
8083
]

tracer/test/snapshots/AspNetCoreMvc31InferredProxySpansTests.Enabled.__path=_bad-request_statusCode=500_spanCount=3.verified.txt

Lines changed: 33 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -20,71 +20,74 @@
2020
},
2121
{
2222
TraceId: Id_1,
23-
SpanId: Id_4,
24-
Name: aws.apigateway,
25-
Resource: GET /api/test/?,
26-
Service: test.api.com,
23+
SpanId: Id_3,
24+
Name: aspnet_core.request,
25+
Resource: GET /bad-request,
26+
Service: Samples.AspNetCoreMvc31,
2727
Type: web,
28-
ParentId: Id_5,
28+
ParentId: Id_4,
2929
Error: 1,
3030
Tags: {
31-
component: aws-apigateway,
31+
aspnet_core.endpoint: Samples.AspNetCoreMvc.Controllers.HomeController.ThrowException (Samples.AspNetCoreMvc31),
32+
aspnet_core.route: bad-request,
33+
component: aspnet_core,
34+
datadog-header-tag: asp-net-core,
3235
env: integration_tests,
3336
error.msg: This was a bad request.,
3437
error.stack:
3538
System.Exception: This was a bad request.
3639
at Samples.AspNetCoreMvc.Controllers.HomeController.ThrowException(),
3740
error.type: System.Exception,
3841
http.method: GET,
42+
http.request.headers.host: localhost:00000,
43+
http.request.headers.sample_correlation_identifier: 0000-0000-0000,
3944
http.response.headers.server: Kestrel,
40-
http.route: /api/test/?,
45+
http.route: bad-request,
4146
http.status_code: 500,
42-
http.url: test.api.com/api/test/1,
47+
http.url: http://localhost:00000/bad-request,
48+
http.useragent: testhelper,
4349
language: dotnet,
44-
span.kind: internal,
45-
stage: prod,
46-
_dd.base_service: Samples.AspNetCoreMvc31
50+
runtime-id: Guid_1,
51+
span.kind: server,
52+
version: 1.0.0
4753
},
4854
Metrics: {
49-
_dd.inferred_span: 1.0,
50-
_sampling_priority_v1: 1.0
55+
_dd.top_level: 1.0
5156
}
5257
},
5358
{
5459
TraceId: Id_1,
55-
SpanId: Id_3,
56-
Name: aspnet_core.request,
57-
Resource: GET /bad-request,
58-
Service: Samples.AspNetCoreMvc31,
60+
SpanId: Id_4,
61+
Name: aws.apigateway,
62+
Resource: GET /api/test/?,
63+
Service: test.api.com,
5964
Type: web,
60-
ParentId: Id_4,
6165
Error: 1,
6266
Tags: {
63-
aspnet_core.endpoint: Samples.AspNetCoreMvc.Controllers.HomeController.ThrowException (Samples.AspNetCoreMvc31),
64-
aspnet_core.route: bad-request,
65-
component: aspnet_core,
66-
datadog-header-tag: asp-net-core,
67+
component: aws-apigateway,
6768
env: integration_tests,
6869
error.msg: This was a bad request.,
6970
error.stack:
7071
System.Exception: This was a bad request.
7172
at Samples.AspNetCoreMvc.Controllers.HomeController.ThrowException(),
7273
error.type: System.Exception,
7374
http.method: GET,
74-
http.request.headers.host: localhost:00000,
75-
http.request.headers.sample_correlation_identifier: 0000-0000-0000,
7675
http.response.headers.server: Kestrel,
77-
http.route: bad-request,
76+
http.route: /api/test/?,
7877
http.status_code: 500,
79-
http.url: http://localhost:00000/bad-request,
80-
http.useragent: testhelper,
78+
http.url: test.api.com/api/test/1,
8179
language: dotnet,
8280
runtime-id: Guid_1,
83-
span.kind: server,
84-
version: 1.0.0
81+
span.kind: internal,
82+
stage: prod,
83+
_dd.base_service: Samples.AspNetCoreMvc31
8584
},
8685
Metrics: {
87-
_dd.top_level: 1.0
86+
process_id: 0,
87+
_dd.inferred_span: 1.0,
88+
_dd.top_level: 1.0,
89+
_dd.tracer_kr: 1.0,
90+
_sampling_priority_v1: 1.0
8891
}
8992
}
9093
]

tracer/test/snapshots/AspNetCoreMvc31InferredProxySpansTests.Enabled.__path=_handled-exception_statusCode=500_spanCount=3.verified.txt

Lines changed: 32 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -24,35 +24,6 @@ System.Exception: Exception of type 'System.Exception' was thrown.
2424
version: 1.0.0
2525
}
2626
},
27-
{
28-
TraceId: Id_1,
29-
SpanId: Id_4,
30-
Name: aws.apigateway,
31-
Resource: GET /api/test/?,
32-
Service: test.api.com,
33-
Type: web,
34-
ParentId: Id_5,
35-
Error: 1,
36-
Tags: {
37-
component: aws-apigateway,
38-
env: integration_tests,
39-
error.msg: The HTTP response has status code 500.,
40-
http.method: GET,
41-
http.response.headers.sample_correlation_identifier: 0000-0000-0000,
42-
http.response.headers.server: Kestrel,
43-
http.route: /api/test/?,
44-
http.status_code: 500,
45-
http.url: test.api.com/api/test/1,
46-
language: dotnet,
47-
span.kind: internal,
48-
stage: prod,
49-
_dd.base_service: Samples.AspNetCoreMvc31
50-
},
51-
Metrics: {
52-
_dd.inferred_span: 1.0,
53-
_sampling_priority_v1: 1.0
54-
}
55-
},
5627
{
5728
TraceId: Id_1,
5829
SpanId: Id_3,
@@ -86,5 +57,37 @@ System.Exception: Exception of type 'System.Exception' was thrown.
8657
Metrics: {
8758
_dd.top_level: 1.0
8859
}
60+
},
61+
{
62+
TraceId: Id_1,
63+
SpanId: Id_4,
64+
Name: aws.apigateway,
65+
Resource: GET /api/test/?,
66+
Service: test.api.com,
67+
Type: web,
68+
Error: 1,
69+
Tags: {
70+
component: aws-apigateway,
71+
env: integration_tests,
72+
error.msg: The HTTP response has status code 500.,
73+
http.method: GET,
74+
http.response.headers.sample_correlation_identifier: 0000-0000-0000,
75+
http.response.headers.server: Kestrel,
76+
http.route: /api/test/?,
77+
http.status_code: 500,
78+
http.url: test.api.com/api/test/1,
79+
language: dotnet,
80+
runtime-id: Guid_1,
81+
span.kind: internal,
82+
stage: prod,
83+
_dd.base_service: Samples.AspNetCoreMvc31
84+
},
85+
Metrics: {
86+
process_id: 0,
87+
_dd.inferred_span: 1.0,
88+
_dd.top_level: 1.0,
89+
_dd.tracer_kr: 1.0,
90+
_sampling_priority_v1: 1.0
91+
}
8992
}
9093
]

tracer/test/snapshots/AspNetCoreMvc31InferredProxySpansTests.Enabled.__path=_not-found_statusCode=404_spanCount=2.verified.txt

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,58 +2,58 @@
22
{
33
TraceId: Id_1,
44
SpanId: Id_2,
5-
Name: aws.apigateway,
6-
Resource: GET /api/test/?,
7-
Service: test.api.com,
5+
Name: aspnet_core.request,
6+
Resource: GET /not-found,
7+
Service: Samples.AspNetCoreMvc31,
88
Type: web,
9+
ParentId: Id_3,
910
Tags: {
10-
component: aws-apigateway,
11+
component: aspnet_core,
12+
datadog-header-tag: asp-net-core,
1113
env: integration_tests,
1214
http.method: GET,
15+
http.request.headers.host: localhost:00000,
16+
http.request.headers.sample_correlation_identifier: 0000-0000-0000,
1317
http.response.headers.server: Kestrel,
14-
http.route: /api/test/?,
1518
http.status_code: 404,
16-
http.url: test.api.com/api/test/1,
19+
http.url: http://localhost:00000/not-found,
20+
http.useragent: testhelper,
1721
language: dotnet,
1822
runtime-id: Guid_1,
19-
span.kind: internal,
20-
stage: prod,
21-
_dd.base_service: Samples.AspNetCoreMvc31
23+
span.kind: server,
24+
version: 1.0.0
2225
},
2326
Metrics: {
24-
process_id: 0,
25-
_dd.inferred_span: 1.0,
26-
_dd.top_level: 1.0,
27-
_dd.tracer_kr: 1.0,
28-
_sampling_priority_v1: 1.0
27+
_dd.top_level: 1.0
2928
}
3029
},
3130
{
3231
TraceId: Id_1,
3332
SpanId: Id_3,
34-
Name: aspnet_core.request,
35-
Resource: GET /not-found,
36-
Service: Samples.AspNetCoreMvc31,
33+
Name: aws.apigateway,
34+
Resource: GET /api/test/?,
35+
Service: test.api.com,
3736
Type: web,
38-
ParentId: Id_2,
3937
Tags: {
40-
component: aspnet_core,
41-
datadog-header-tag: asp-net-core,
38+
component: aws-apigateway,
4239
env: integration_tests,
4340
http.method: GET,
44-
http.request.headers.host: localhost:00000,
45-
http.request.headers.sample_correlation_identifier: 0000-0000-0000,
4641
http.response.headers.server: Kestrel,
42+
http.route: /api/test/?,
4743
http.status_code: 404,
48-
http.url: http://localhost:00000/not-found,
49-
http.useragent: testhelper,
44+
http.url: test.api.com/api/test/1,
5045
language: dotnet,
5146
runtime-id: Guid_1,
52-
span.kind: server,
53-
version: 1.0.0
47+
span.kind: internal,
48+
stage: prod,
49+
_dd.base_service: Samples.AspNetCoreMvc31
5450
},
5551
Metrics: {
56-
_dd.top_level: 1.0
52+
process_id: 0,
53+
_dd.inferred_span: 1.0,
54+
_dd.top_level: 1.0,
55+
_dd.tracer_kr: 1.0,
56+
_sampling_priority_v1: 1.0
5757
}
5858
}
5959
]

0 commit comments

Comments
 (0)