From 56af8c7119a978d94073b7e524b9d00331f1d507 Mon Sep 17 00:00:00 2001 From: Muhammad Othman Date: Wed, 2 Oct 2024 16:03:08 -0400 Subject: [PATCH 1/3] [Instrumentation.AWS] Move adding request and response tags to AWSTracingPipelineHandler --- .../AWSPropagatorPipelineHandler.cs | 179 +-------------- .../Implementation/AWSSemanticConventions.cs | 1 + .../AWSTracingPipelineCustomizer.cs | 11 +- .../AWSTracingPipelineHandler.cs | 213 ++++++++++++++++++ 4 files changed, 226 insertions(+), 178 deletions(-) create mode 100644 src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs diff --git a/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSPropagatorPipelineHandler.cs b/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSPropagatorPipelineHandler.cs index 1e97c34c88..20818ba0ff 100644 --- a/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSPropagatorPipelineHandler.cs +++ b/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSPropagatorPipelineHandler.cs @@ -7,7 +7,6 @@ using Amazon.Runtime.Telemetry; using OpenTelemetry.Context.Propagation; using OpenTelemetry.Extensions.AWS.Trace; -using OpenTelemetry.Trace; namespace OpenTelemetry.Instrumentation.AWS.Implementation; @@ -28,191 +27,22 @@ internal class AWSPropagatorPipelineHandler : PipelineHandler carrier[name] = value; }; - private readonly AWSClientInstrumentationOptions options; - - public AWSPropagatorPipelineHandler(AWSClientInstrumentationOptions options) - { - this.options = options; - } - public override void InvokeSync(IExecutionContext executionContext) { - this.ProcessBeginRequest(executionContext); + ProcessBeginRequest(executionContext); base.InvokeSync(executionContext); - - ProcessEndRequest(executionContext); } public override async Task InvokeAsync(IExecutionContext executionContext) { - T? ret = null; - - this.ProcessBeginRequest(executionContext); - - ret = await base.InvokeAsync(executionContext).ConfigureAwait(false); - - ProcessEndRequest(executionContext); - - return ret; - } - -#if NET - [System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessage( - "Trimming", - "IL2075", - Justification = "The reflected properties were already used by the AWS SDK's marshallers so the properties could not have been trimmed.")] -#endif - private static void AddRequestSpecificInformation(Activity activity, IRequestContext requestContext) - { - var service = requestContext.ServiceMetaData.ServiceId; - - if (AWSServiceHelper.ServiceRequestParameterMap.TryGetValue(service, out var parameters)) - { - AmazonWebServiceRequest request = requestContext.OriginalRequest; - - foreach (var parameter in parameters) - { - try - { - // for bedrock agent, we only extract one attribute based on the operation. - if (AWSServiceType.IsBedrockAgentService(service)) - { - if (AWSServiceHelper.OperationNameToResourceMap()[AWSServiceHelper.GetAWSOperationName(requestContext)] != parameter) - { - continue; - } - } - - var property = request.GetType().GetProperty(parameter); - if (property != null) - { - if (AWSServiceHelper.ParameterAttributeMap.TryGetValue(parameter, out var attribute)) - { - activity.SetTag(attribute, property.GetValue(request)); - } - } - } - catch (Exception) - { - // Guard against any reflection-related exceptions when running in AoT. - // See https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/1543#issuecomment-1907667722. - } - } - } + ProcessBeginRequest(executionContext); - if (AWSServiceType.IsDynamoDbService(service)) - { - activity.SetTag(SemanticConventions.AttributeDbSystem, AWSSemanticConventions.AttributeValueDynamoDb); - } - else if (AWSServiceType.IsSqsService(service)) - { - SqsRequestContextHelper.AddAttributes( - requestContext, AWSMessagingUtils.InjectIntoDictionary(new PropagationContext(activity.Context, Baggage.Current))); - } - else if (AWSServiceType.IsSnsService(service)) - { - SnsRequestContextHelper.AddAttributes( - requestContext, AWSMessagingUtils.InjectIntoDictionary(new PropagationContext(activity.Context, Baggage.Current))); - } - else if (AWSServiceType.IsBedrockRuntimeService(service)) - { - activity.SetTag(AWSSemanticConventions.AttributeGenAiSystem, "aws_bedrock"); - } + return await base.InvokeAsync(executionContext).ConfigureAwait(false); } -#if NET - [System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessage( - "Trimming", - "IL2075", - Justification = "The reflected properties were already used by the AWS SDK's marshallers so the properties could not have been trimmed.")] -#endif - private static void AddResponseSpecificInformation(Activity activity, IExecutionContext executionContext) + private static void ProcessBeginRequest(IExecutionContext executionContext) { - var service = executionContext.RequestContext.ServiceMetaData.ServiceId; - var responseContext = executionContext.ResponseContext; - - if (AWSServiceHelper.ServiceResponseParameterMap.TryGetValue(service, out var parameters)) - { - AmazonWebServiceResponse response = responseContext.Response; - - foreach (var parameter in parameters) - { - try - { - // for bedrock agent, extract attribute from object in response. - if (AWSServiceType.IsBedrockAgentService(service)) - { - var operationName = Utils.RemoveSuffix(response.GetType().Name, "Response"); - if (AWSServiceHelper.OperationNameToResourceMap()[operationName] == parameter) - { - AddBedrockAgentResponseAttribute(activity, response, parameter); - } - } - - var property = response.GetType().GetProperty(parameter); - if (property != null) - { - if (AWSServiceHelper.ParameterAttributeMap.TryGetValue(parameter, out var attribute)) - { - activity.SetTag(attribute, property.GetValue(response)); - } - } - } - catch (Exception) - { - // Guard against any reflection-related exceptions when running in AoT. - // See https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/1543#issuecomment-1907667722. - } - } - } - } - -#if NET - [System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessage( - "Trimming", - "IL2075", - Justification = "The reflected properties were already used by the AWS SDK's marshallers so the properties could not have been trimmed.")] -#endif - private static void AddBedrockAgentResponseAttribute(Activity activity, AmazonWebServiceResponse response, string parameter) - { - var responseObject = response.GetType().GetProperty(Utils.RemoveSuffix(parameter, "Id")); - if (responseObject != null) - { - var attributeObject = responseObject.GetValue(response); - if (attributeObject != null) - { - var property = attributeObject.GetType().GetProperty(parameter); - if (property != null) - { - if (AWSServiceHelper.ParameterAttributeMap.TryGetValue(parameter, out var attribute)) - { - activity.SetTag(attribute, property.GetValue(attributeObject)); - } - } - } - } - } - - private static void ProcessEndRequest(IExecutionContext executionContext) - { - var currentActivity = Activity.Current; - - if (currentActivity == null || !currentActivity.Source.Name.StartsWith(TelemetryConstants.TelemetryScopePrefix, StringComparison.Ordinal)) - { - return; - } - - AddResponseSpecificInformation(currentActivity, executionContext); - } - - private void ProcessBeginRequest(IExecutionContext executionContext) - { - if (this.options.SuppressDownstreamInstrumentation) - { - SuppressInstrumentationScope.Enter(); - } - var currentActivity = Activity.Current; // Propagate the current activity if it was created by the AWS SDK @@ -221,7 +51,6 @@ private void ProcessBeginRequest(IExecutionContext executionContext) return; } - AddRequestSpecificInformation(currentActivity, executionContext.RequestContext); AwsPropagator.Inject( new PropagationContext(currentActivity.Context, Baggage.Current), executionContext.RequestContext.Request.Headers, diff --git a/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSSemanticConventions.cs b/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSSemanticConventions.cs index 71fc82ddb3..da22c1c678 100644 --- a/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSSemanticConventions.cs +++ b/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSSemanticConventions.cs @@ -18,6 +18,7 @@ internal static class AWSSemanticConventions public const string AttributeAWSBedrockDataSourceId = "aws.bedrock.data_source.id"; public const string AttributeAWSBedrockGuardrailId = "aws.bedrock.guardrail.id"; public const string AttributeAWSBedrockKnowledgeBaseId = "aws.bedrock.knowledge_base.id"; + public const string AttributeAWSeBedrock = "aws_bedrock"; // should be global convention for Gen AI attributes public const string AttributeGenAiModelId = "gen_ai.request.model"; diff --git a/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSTracingPipelineCustomizer.cs b/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSTracingPipelineCustomizer.cs index 235a656c81..cdb420ad80 100644 --- a/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSTracingPipelineCustomizer.cs +++ b/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSTracingPipelineCustomizer.cs @@ -7,8 +7,8 @@ namespace OpenTelemetry.Instrumentation.AWS.Implementation; /// -/// Wires into the AWS -/// so it can inject trace headers and add request information to the tags. +/// Wires and into the AWS +/// so they can inject trace headers and add request information to the tags. /// internal class AWSTracingPipelineCustomizer : IRuntimePipelineCustomizer { @@ -34,7 +34,12 @@ public void Customize(Type serviceClientType, RuntimePipeline pipeline) return; } - var propagatingPipelineHandler = new AWSPropagatorPipelineHandler(this.options); + var tracingPipelineHandler = new AWSTracingPipelineHandler(this.options); + var propagatingPipelineHandler = new AWSPropagatorPipelineHandler(); + + // AWSTracingPipelineHandler must execute early in the AWS SDK pipeline + // in order to manipulate outgoing requests objects before they are marshalled (ie serialized). + pipeline.AddHandlerBefore(tracingPipelineHandler); // AWSPropagatorPipelineHandler executes after the AWS SDK has marshalled (ie serialized) // the outgoing request object so that it can work with the request's Headers diff --git a/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs b/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs new file mode 100644 index 0000000000..f63730770d --- /dev/null +++ b/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs @@ -0,0 +1,213 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +using System.Diagnostics; +using Amazon.Runtime; +using Amazon.Runtime.Internal; +using Amazon.Runtime.Telemetry; +using OpenTelemetry.Context.Propagation; +using OpenTelemetry.Trace; + +namespace OpenTelemetry.Instrumentation.AWS.Implementation; + +/// +/// Adds additional request and response tags depending on the target AWS Service. +/// +/// This must execute early in the AWS SDK pipeline +/// in order to manipulate outgoing requests objects before they are marshalled (ie serialized). +/// +internal sealed class AWSTracingPipelineHandler : PipelineHandler +{ + private readonly AWSClientInstrumentationOptions options; + + public AWSTracingPipelineHandler(AWSClientInstrumentationOptions options) + { + this.options = options; + } + + public override void InvokeSync(IExecutionContext executionContext) + { + var activity = this.ProcessBeginRequest(executionContext); + base.InvokeSync(executionContext); + ProcessEndRequest(activity, executionContext); + } + + public override async Task InvokeAsync(IExecutionContext executionContext) + { + T? ret = null; + + var activity = this.ProcessBeginRequest(executionContext); + ret = await base.InvokeAsync(executionContext).ConfigureAwait(false); + + ProcessEndRequest(activity, executionContext); + + return ret; + } + +#if NET + [System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessage( + "Trimming", + "IL2075", + Justification = "The reflected properties were already used by the AWS SDK's marshallers so the properties could not have been trimmed.")] +#endif + private static void AddResponseSpecificInformation(Activity activity, IExecutionContext executionContext) + { + var service = executionContext.RequestContext.ServiceMetaData.ServiceId; + var responseContext = executionContext.ResponseContext; + + if (AWSServiceHelper.ServiceResponseParameterMap.TryGetValue(service, out var parameters)) + { + AmazonWebServiceResponse response = responseContext.Response; + + foreach (var parameter in parameters) + { + try + { + // for bedrock agent, extract attribute from object in response. + if (AWSServiceType.IsBedrockAgentService(service)) + { + var operationName = Utils.RemoveSuffix(response.GetType().Name, "Response"); + if (AWSServiceHelper.OperationNameToResourceMap()[operationName] == parameter) + { + AddBedrockAgentResponseAttribute(activity, response, parameter); + } + } + + var property = response.GetType().GetProperty(parameter); + if (property != null) + { + if (AWSServiceHelper.ParameterAttributeMap.TryGetValue(parameter, out var attribute)) + { + activity.SetTag(attribute, property.GetValue(response)); + } + } + } + catch (Exception) + { + // Guard against any reflection-related exceptions when running in AoT. + // See https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/1543#issuecomment-1907667722. + } + } + } + } + +#if NET + [System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessage( + "Trimming", + "IL2075", + Justification = "The reflected properties were already used by the AWS SDK's marshallers so the properties could not have been trimmed.")] +#endif + private static void AddBedrockAgentResponseAttribute(Activity activity, AmazonWebServiceResponse response, string parameter) + { + var responseObject = response.GetType().GetProperty(Utils.RemoveSuffix(parameter, "Id")); + if (responseObject != null) + { + var attributeObject = responseObject.GetValue(response); + if (attributeObject != null) + { + var property = attributeObject.GetType().GetProperty(parameter); + if (property != null) + { + if (AWSServiceHelper.ParameterAttributeMap.TryGetValue(parameter, out var attribute)) + { + activity.SetTag(attribute, property.GetValue(attributeObject)); + } + } + } + } + } + +#if NET + [System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessage( + "Trimming", + "IL2075", + Justification = "The reflected properties were already used by the AWS SDK's marshallers so the properties could not have been trimmed.")] +#endif + private static void AddRequestSpecificInformation(Activity activity, IRequestContext requestContext) + { + var service = requestContext.ServiceMetaData.ServiceId; + + if (AWSServiceHelper.ServiceRequestParameterMap.TryGetValue(service, out var parameters)) + { + AmazonWebServiceRequest request = requestContext.OriginalRequest; + + foreach (var parameter in parameters) + { + try + { + // for bedrock agent, we only extract one attribute based on the operation. + if (AWSServiceType.IsBedrockAgentService(service)) + { + if (AWSServiceHelper.OperationNameToResourceMap()[AWSServiceHelper.GetAWSOperationName(requestContext)] != parameter) + { + continue; + } + } + + var property = request.GetType().GetProperty(parameter); + if (property != null) + { + if (AWSServiceHelper.ParameterAttributeMap.TryGetValue(parameter, out var attribute)) + { + activity.SetTag(attribute, property.GetValue(request)); + } + } + } + catch (Exception) + { + // Guard against any reflection-related exceptions when running in AoT. + // See https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/1543#issuecomment-1907667722. + } + } + } + + if (AWSServiceType.IsDynamoDbService(service)) + { + activity.SetTag(SemanticConventions.AttributeDbSystem, AWSSemanticConventions.AttributeValueDynamoDb); + } + else if (AWSServiceType.IsSqsService(service)) + { + SqsRequestContextHelper.AddAttributes( + requestContext, AWSMessagingUtils.InjectIntoDictionary(new PropagationContext(activity.Context, Baggage.Current))); + } + else if (AWSServiceType.IsSnsService(service)) + { + SnsRequestContextHelper.AddAttributes( + requestContext, AWSMessagingUtils.InjectIntoDictionary(new PropagationContext(activity.Context, Baggage.Current))); + } + else if (AWSServiceType.IsBedrockRuntimeService(service)) + { + activity.SetTag(AWSSemanticConventions.AttributeGenAiSystem, AWSSemanticConventions.AttributeAWSeBedrock); + } + } + + private static void ProcessEndRequest(Activity? activity, IExecutionContext executionContext) + { + if (activity == null || !activity.IsAllDataRequested) + { + return; + } + + AddResponseSpecificInformation(activity, executionContext); + } + + private Activity? ProcessBeginRequest(IExecutionContext executionContext) + { + if (this.options.SuppressDownstreamInstrumentation) + { + SuppressInstrumentationScope.Enter(); + } + + var currentActivity = Activity.Current; + + if (currentActivity == null + || !currentActivity.Source.Name.StartsWith(TelemetryConstants.TelemetryScopePrefix, StringComparison.Ordinal) + || !currentActivity.IsAllDataRequested) + { + return null; + } + + AddRequestSpecificInformation(currentActivity, executionContext.RequestContext); + return currentActivity; + } +} From ba98b61555cd983a2cce54256b0b6a8f3112b6e6 Mon Sep 17 00:00:00 2001 From: Muhammad Othman Date: Wed, 2 Oct 2024 16:23:53 -0400 Subject: [PATCH 2/3] Update CHANGELOG --- src/OpenTelemetry.Instrumentation.AWS/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/OpenTelemetry.Instrumentation.AWS/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AWS/CHANGELOG.md index 49380447de..c77b6fe960 100644 --- a/src/OpenTelemetry.Instrumentation.AWS/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AWS/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +* Move adding request and response info to AWSTracingPipelineHandler + ([#2137](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2137)) + ## 1.1.0-beta.6 Released 2024-Sep-10 From 947f12dd3818fa69c16ace01c953c865730e4af6 Mon Sep 17 00:00:00 2001 From: Muhammad Othman Date: Tue, 15 Oct 2024 11:42:32 -0400 Subject: [PATCH 3/3] fix AttributeAWSBedrock typo --- .../Implementation/AWSSemanticConventions.cs | 2 +- .../Implementation/AWSTracingPipelineHandler.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSSemanticConventions.cs b/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSSemanticConventions.cs index da22c1c678..61d72da34e 100644 --- a/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSSemanticConventions.cs +++ b/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSSemanticConventions.cs @@ -18,7 +18,7 @@ internal static class AWSSemanticConventions public const string AttributeAWSBedrockDataSourceId = "aws.bedrock.data_source.id"; public const string AttributeAWSBedrockGuardrailId = "aws.bedrock.guardrail.id"; public const string AttributeAWSBedrockKnowledgeBaseId = "aws.bedrock.knowledge_base.id"; - public const string AttributeAWSeBedrock = "aws_bedrock"; + public const string AttributeAWSBedrock = "aws_bedrock"; // should be global convention for Gen AI attributes public const string AttributeGenAiModelId = "gen_ai.request.model"; diff --git a/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs b/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs index f63730770d..3ff390ef54 100644 --- a/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs +++ b/src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs @@ -177,7 +177,7 @@ private static void AddRequestSpecificInformation(Activity activity, IRequestCon } else if (AWSServiceType.IsBedrockRuntimeService(service)) { - activity.SetTag(AWSSemanticConventions.AttributeGenAiSystem, AWSSemanticConventions.AttributeAWSeBedrock); + activity.SetTag(AWSSemanticConventions.AttributeGenAiSystem, AWSSemanticConventions.AttributeAWSBedrock); } }