aws-observability / aws-otel-dotnet-instrumentation

Apache License 2.0
0 stars 6 forks source link

Fixed the route parsing logic by caching the httpContext object #130

Closed AsakerMohd closed 6 days ago

AsakerMohd commented 1 week ago

Description of changes: Fixed the route parsing logic by storing a weak reference of the httpContext to be accessed later by processors. Weak References allow the garbage collectorto reclaim memory if the object is no longer used. We are storing references due to the following:

  1. When a request is received, an activity starts immediately and in that phase, the routing middleware hasn't executed and thus the routing data isn't available yet
  2. Once the routing middleware is executed, and the request is matched to the route template, we are certain the routing data is avaialble when any children activities are started.
  3. We then use this HttpContext object to access the now available route data.
  4. In AwsSpanProcessingUtil, we extract the HttpContext object and attempt to get the route data. If it's not available, we fallback to the original logic to compute using the path. This is the case when a request is made that doesn't match to any route pattern.

Testing: Added unit tests for the extra if condition.

Also tested by running a sample app with a new route /outgoing/test/{id} and making a call to /outgoing/test/2 and these are the resulting activities (pay special attention to aws.local.operation since that's the whole point of the fix):

app-1           | Activity.TraceId:            483a7eed5b59dab4092c753ce10f49d6
app-1           | Activity.SpanId:             1de8a7025accab5e
app-1           | Activity.TraceFlags:         Recorded
app-1           | Activity.ParentSpanId:       49f97bc5a9a50083
app-1           | Activity.ActivitySourceName: System.Net.Http
app-1           | Activity.DisplayName:        GET
app-1           | Activity.Kind:               Client
app-1           | Activity.StartTime:          2024-10-28T21:44:21.3935591Z
app-1           | Activity.Duration:           00:00:00.2968578
app-1           | Activity.Tags:
app-1           |     aws.local.operation: GET outgoing-http-call/test/{id}
app-1           |     http.request.method: GET
app-1           |     server.address: aws.amazon.com
app-1           |     server.port: 443
app-1           |     url.full: https://aws.amazon.com/
app-1           |     network.protocol.version: 1.1
app-1           |     http.response.status_code: 200
app-1           |     aws.local.service: aws-otel-integ-test
app-1           |     aws.remote.service: aws.amazon.com:443
app-1           |     aws.remote.operation: GET /
app-1           |     aws.span.kind: CLIENT
app-1           | Resource associated with Activity:
app-1           |     telemetry.distro.name: aws-otel-dotnet-instrumentation
app-1           |     telemetry.distro.version: 1.3.2.dev0-aws
app-1           |     host.name: 7be0db790823
app-1           |     process.owner: root
app-1           |     process.pid: 1
app-1           |     process.runtime.description: .NET 8.0.10
app-1           |     process.runtime.name: .NET
app-1           |     process.runtime.version: 8.0.10
app-1           |     container.id: 7be0db7908233c976cd1767a97a36afab03ad08c1739a53982bbbc2750143e86
app-1           |     telemetry.sdk.name: opentelemetry
app-1           |     telemetry.sdk.language: dotnet
app-1           |     telemetry.sdk.version: 1.9.0
app-1           |     service.name: aws-otel-integ-test
app-1           | 
app-1           | Activity.TraceId:            483a7eed5b59dab4092c753ce10f49d6
app-1           | Activity.SpanId:             49f97bc5a9a50083
app-1           | Activity.TraceFlags:         Recorded
app-1           | Activity.ActivitySourceName: Microsoft.AspNetCore
app-1           | Activity.DisplayName:        GET outgoing-http-call/test/{id}
app-1           | Activity.Kind:               Server
app-1           | Activity.StartTime:          2024-10-28T21:44:21.3123446Z
app-1           | Activity.Duration:           00:00:00.4471396
app-1           | Activity.Tags:
app-1           |     server.address: localhost
app-1           |     server.port: 8080
app-1           |     http.request.method: GET
app-1           |     url.scheme: http
app-1           |     url.path: /outgoing-http-call/test/2
app-1           |     network.protocol.version: 1.1
app-1           |     user_agent.original: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:131.0) Gecko/20100101 Firefox/131.0
app-1           |     http.route: outgoing-http-call/test/{id}
app-1           |     http.response.status_code: 200
app-1           |     aws.local.service: aws-otel-integ-test
app-1           |     aws.local.operation: GET outgoing-http-call/test/{id}
app-1           |     aws.span.kind: LOCAL_ROOT
app-1           | Resource associated with Activity:
app-1           |     telemetry.distro.name: aws-otel-dotnet-instrumentation
app-1           |     telemetry.distro.version: 1.3.2.dev0-aws
app-1           |     host.name: 7be0db790823
app-1           |     process.owner: root
app-1           |     process.pid: 1
app-1           |     process.runtime.description: .NET 8.0.10
app-1           |     process.runtime.name: .NET
app-1           |     process.runtime.version: 8.0.10
app-1           |     container.id: 7be0db7908233c976cd1767a97a36afab03ad08c1739a53982bbbc2750143e86
app-1           |     telemetry.sdk.name: opentelemetry
app-1           |     telemetry.sdk.language: dotnet
app-1           |     telemetry.sdk.version: 1.9.0
app-1           |     service.name: aws-otel-integ-test

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

vastin commented 1 week ago

Please fix contract test for local operation change. https://github.com/aws-observability/aws-otel-dotnet-instrumentation/actions/runs/11563162142/job/32185885662?pr=130