Skip to content

Commit 98430b8

Browse files
AsakerMohdvastin
AsakerMohd
and
vastin
authored
Fixed the route parsing logic by caching the httpContext object (#130)
Co-authored-by: Vastin <[email protected]>
1 parent e7dd03d commit 98430b8

File tree

7 files changed

+181
-7
lines changed

7 files changed

+181
-7
lines changed

‎src/AWS.Distro.OpenTelemetry.AutoInstrumentation/AwsSpanProcessingUtil.cs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
using System.Diagnostics;
5+
#if !NETFRAMEWORK
6+
using Microsoft.AspNetCore.Diagnostics;
7+
using Microsoft.AspNetCore.Http;
8+
using Microsoft.AspNetCore.Routing;
9+
#endif
510
using Newtonsoft.Json.Linq;
611
using OpenTelemetry;
712
using static AWS.Distro.OpenTelemetry.AutoInstrumentation.AwsAttributeKeys;
@@ -106,6 +111,28 @@ internal static string GetIngressOperation(Activity span)
106111
operation = InternalOperation;
107112
}
108113

114+
#if !NETFRAMEWORK
115+
// Access the HttpContext object to get the route data.
116+
else if (span.GetCustomProperty("HttpContextWeakRef") is WeakReference<HttpContext> httpContextWeakRef &&
117+
httpContextWeakRef.TryGetTarget(out var httpContext))
118+
{
119+
// This is copied from upstream to maintain the same retrieval logic
120+
// https://github.com/open-telemetry/opentelemetry-dotnet-contrib/blob/main/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs#L246C13-L247C83
121+
var routePattern = (httpContext.Features.Get<IExceptionHandlerPathFeature>()?.Endpoint as RouteEndpoint ??
122+
httpContext.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;
123+
124+
if (!string.IsNullOrEmpty(routePattern))
125+
{
126+
string? httpMethod = (string?)span.GetTagItem(AttributeHttpRequestMethod);
127+
operation = httpMethod + " " + routePattern;
128+
}
129+
else
130+
{
131+
operation = GenerateIngressOperation(span);
132+
}
133+
}
134+
#endif
135+
109136
// workaround for now so that both Server and Consumer spans have same operation
110137
// TODO: Update this and other languages so that all of them set the operation during propagation.
111138
else if (!IsValidOperation(span, operation) || (IsKeyPresent(span, AttributeUrlPath) && HttpRouteDataParsingEnabled == "false"))

‎src/AWS.Distro.OpenTelemetry.AutoInstrumentation/Plugin.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.Diagnostics;
55
using System.Diagnostics.Metrics;
66
using Amazon.Runtime;
7+
using Microsoft.AspNetCore.Http;
78
using Microsoft.Extensions.Logging;
89
using OpenTelemetry;
910
using OpenTelemetry.Context.Propagation;
@@ -282,6 +283,16 @@ public void ConfigureTracesOptions(AspNetCoreTraceInstrumentationOptions options
282283
{
283284
options.EnrichWithHttpRequest = (activity, request) =>
284285
{
286+
// Storing a weak reference of the httpContext to be accessed later by processors. Weak References allow the garbage collector
287+
// to reclaim memory if the object is no longer used.
288+
// We are storing references due to the following:
289+
// 1. When a request is received, an activity starts immediately and in that phase,
290+
// the routing middleware hasn't executed and thus the routing data isn't available yet
291+
// 2. Once the routing middleware is executed, and the request is matched to the route template,
292+
// we are certain the routing data is avaialble when any children activities are started.
293+
// 3. We then use this HttpContext object to access the now available route data.
294+
activity.SetCustomProperty("HttpContextWeakRef", new WeakReference<HttpContext>(request.HttpContext));
295+
285296
if (this.sampler != null && this.sampler.GetType() == typeof(AWSXRayRemoteSampler))
286297
{
287298
this.ShouldSampleParent(activity);

‎test/AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests/AwsSpanProcessingUtilTest.cs‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@
99

1010
namespace AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests;
1111

12+
using Microsoft.AspNetCore.Diagnostics;
13+
using Microsoft.AspNetCore.Http;
14+
using Microsoft.AspNetCore.Http.Features;
15+
using Microsoft.AspNetCore.Routing;
16+
using Microsoft.AspNetCore.Routing.Patterns;
17+
using Moq;
1218
using Xunit;
1319

1420
public class AwsSpanProcessingUtilTest
@@ -110,6 +116,87 @@ public void TestGetIngressOperationInvalidNameAndValidTargetAndMethod()
110116
Assert.Equal(validMethod + " " + validTarget, actualOperation);
111117
}
112118

119+
#if !NETFRAMEWORK
120+
[Fact]
121+
public void TestGetIngressOperationInvalidNameWithHttpContextPresentAndRouteDataAvailable()
122+
{
123+
var routeTemplate = "/test/{id}";
124+
string invalidName = string.Empty;
125+
string validTarget = "/";
126+
string validMethod = "GET";
127+
128+
// Arrange
129+
var contextMock = new Mock<HttpContext>();
130+
var featuresMock = new Mock<IFeatureCollection>();
131+
var exceptionHandlerFeatureMock = new Mock<IExceptionHandlerPathFeature>();
132+
133+
// Set up the RoutePattern
134+
var routePattern = RoutePatternFactory.Parse(routeTemplate);
135+
var routeEndpoint = new RouteEndpoint(
136+
requestDelegate: (context) => Task.CompletedTask,
137+
routePattern: routePattern,
138+
order: 0,
139+
metadata: new EndpointMetadataCollection(),
140+
displayName: "RouteTemplateTest");
141+
142+
// Configure the exception handler feature to return the route endpoint
143+
exceptionHandlerFeatureMock.Setup(f => f.Endpoint).Returns(routeEndpoint);
144+
145+
// Set up the Features collection
146+
featuresMock.Setup(f => f.Get<IExceptionHandlerPathFeature>()).Returns(exceptionHandlerFeatureMock.Object);
147+
contextMock.Setup(c => c.Features).Returns(featuresMock.Object);
148+
149+
// Act
150+
var context = contextMock.Object;
151+
152+
Activity? spanDataMock = this.testSource.StartActivity("test", ActivityKind.Server);
153+
if (spanDataMock != null)
154+
{
155+
spanDataMock.DisplayName = invalidName;
156+
spanDataMock.SetTag(AttributeHttpRequestMethod, validMethod);
157+
spanDataMock.SetTag(AttributeUrlPath, validTarget);
158+
spanDataMock.SetCustomProperty("HttpContextWeakRef", new WeakReference<HttpContext>(context));
159+
spanDataMock.Start();
160+
161+
string actualOperation = GetIngressOperation(spanDataMock);
162+
Assert.Equal(validMethod + " " + routeTemplate, actualOperation);
163+
}
164+
}
165+
#endif
166+
167+
[Fact]
168+
public void TestGetIngressOperationInvalidNameWithHttpContextPresentAndRouteDataNotAvailable()
169+
{
170+
string invalidName = string.Empty;
171+
string validTarget = "/test";
172+
string validMethod = "GET";
173+
174+
var contextMock = new Mock<HttpContext>();
175+
var featuresMock = new Mock<IFeatureCollection>();
176+
var exceptionHandlerFeatureMock = new Mock<IExceptionHandlerPathFeature>();
177+
178+
RouteEndpoint? routeEndpoint = null;
179+
180+
exceptionHandlerFeatureMock.Setup(f => f.Endpoint).Returns(routeEndpoint);
181+
182+
featuresMock.Setup(f => f.Get<IExceptionHandlerPathFeature>()).Returns(exceptionHandlerFeatureMock.Object);
183+
contextMock.Setup(c => c.Features).Returns(featuresMock.Object);
184+
var context = contextMock.Object;
185+
186+
Activity? spanDataMock = this.testSource.StartActivity("test", ActivityKind.Server);
187+
if (spanDataMock != null)
188+
{
189+
spanDataMock.DisplayName = invalidName;
190+
spanDataMock.SetTag(AttributeHttpRequestMethod, validMethod);
191+
spanDataMock.SetTag(AttributeUrlPath, validTarget);
192+
spanDataMock.SetCustomProperty("HttpContextWeakRef", new WeakReference<HttpContext>(context));
193+
spanDataMock.Start();
194+
195+
string actualOperation = GetIngressOperation(spanDataMock);
196+
Assert.Equal(validMethod + " " + validTarget, actualOperation);
197+
}
198+
}
199+
113200
[Fact]
114201
public void TestGetEgressOperationUseInternalOperation()
115202
{

‎test/AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests/GlobalSuppressions.cs‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// This file is used by Code Analysis to maintain SuppressMessage
2+
// attributes that are applied to this project.
3+
// Project-level suppressions either have no target or are given
4+
// a specific target and scoped to a namespace, type, member, etc.
5+
6+
using System.Diagnostics.CodeAnalysis;
7+
8+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.#ctor")]
9+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestExtractAPIPathValidPath")]
10+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestExtractAPIPathValueEmptyTarget")]
11+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestExtractAPIPathValueNoSlash")]
12+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestExtractAPIPathValueNullTarget")]
13+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestExtractAPIPathValueOnlySlash")]
14+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestExtractAPIPathValueOnlySlashAtEnd")]
15+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestGet")]
16+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestGetEgressOperationUseInternalOperation")]
17+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestGetEgressOperationUseLocalOperation")]
18+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestGetIngressOperationEmptyNameAndNoFallback")]
19+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestGetIngressOperationHttpMethodNameAndNoFallback")]
20+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestGetIngressOperationInvalidNameAndValidTarget")]
21+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestGetIngressOperationInvalidNameAndValidTargetAndMethod")]
22+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestGetIngressOperationUnknownNameAndNoFallback")]
23+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestGetIngressOperationValidName")]
24+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestGetIngressOperationWithnotServer")]
25+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestIsAwsSpanFalse")]
26+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestIsAwsSpanTrue")]
27+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestIsConsumerProcessSpanFalse")]
28+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestIsDBSpanFalse")]
29+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestIsDBSpanTrue")]
30+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestIsKeyPresentKeyAbsent")]
31+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestIsKeyPresentKeyPresent")]
32+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestIsLocalRoot")]
33+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestMetricAttributesGeneratedForOtherInstrumentationSqsConsumerSpan")]
34+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestNoMetricAttributesForAwsSdkSqsConsumerProcessSpan")]
35+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestNoMetricAttributesForSqsConsumerSpanAwsSdk")]
36+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestShouldGenerateDependencyMetricAttributes")]
37+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestShouldGenerateServiceMetricAttributes")]
38+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestShouldUseInternalOperationFalse")]
39+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestSqlDialectKeywordsMaxLength")]
40+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "member", Target = "~M:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest.TestSqlDialectKeywordsOrder")]
41+
[assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:Elements should be documented", Justification = "Reviewed", Scope = "type", Target = "~T:AWS.Distro.OpenTelemetry.AutoInstrumentation.Tests.AwsSpanProcessingUtilTest")]

‎test/contract-tests/tests/test/amazon/awssdk/awssdk_test.py‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,15 @@ def _assert_array_value_ddb_table_name(self, attributes_dict: Dict[str, AnyValue
582582
self.assertEqual(attributes_dict[key].string_value, expect_values[0])
583583

584584
def _filter_bedrock_metrics(self, target_metrics: List[Metric]):
585-
bedrock_calls = {"GET /agents", "GET /guardrails", "GET /knowledgebases", "POST /agents", "POST /model", "POST /knowledgebases" }
585+
bedrock_calls = {
586+
"GET agents/test-agent",
587+
"GET guardrails/test-guardrail",
588+
"GET knowledgebases/test-knowledge-base",
589+
"GET knowledgebases/test-knowledge-base/datasources/test-data-source",
590+
"POST agents/test-agent/agentAliases/test-agent-alias/sessions/test-session/text",
591+
"POST model/test-model/invoke",
592+
"POST knowledgebases/test-knowledge-base/retrieve"
593+
}
586594
for metric in target_metrics:
587595
for dp in metric.exponential_histogram.data_points:
588596
# remove dp generated from manual response

‎test/contract-tests/tests/test/amazon/efcore/efcore_test.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,15 @@ def test_route(self) -> None:
4343
0,
4444
0,
4545
request_method="GET",
46-
local_operation="GET /blogs"
46+
local_operation="GET /blogs/{id}"
4747
)
4848

4949
def test_delete_success(self) -> None:
5050
self.do_test_requests(
51-
"/blogs/1", "DELETE", 200, 0, 0, request_method="DELETE", local_operation="DELETE /blogs"
51+
"/blogs/1", "DELETE", 200, 0, 0, request_method="DELETE", local_operation="DELETE /blogs/{id}"
5252
)
5353
def test_error(self) -> None:
54-
self.do_test_requests("/blogs/100", "GET", 404, 1, 0, request_method="GET", local_operation="GET /blogs")
54+
self.do_test_requests("/blogs/100", "GET", 404, 1, 0, request_method="GET", local_operation="GET /blogs/{id}")
5555

5656
@override
5757
def _assert_aws_span_attributes(self, resource_scope_spans: List[ResourceScopeSpan], path: str, **kwargs) -> None:

0 commit comments

Comments
 (0)