From 8b932195c9f450eee8dff69fa9569c2114b65992 Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Tue, 28 Feb 2023 09:56:27 +0100 Subject: [PATCH 01/23] #1034: AWS SQS and SNS support - incoming requests: implemented parent extraction for incoming SQS and SNS events (getters) - outgoing requests: implemented enrichment of outgoing message attributes by trace data (setters) --- .../AWSMessageAttributeHelper.cs | 82 ++++++++++ .../Implementation/AWSMessagingUtils.cs | 74 +++++++++ .../Implementation/AWSServiceHelper.cs | 11 +- .../Implementation/AWSServiceType.cs | 35 +++++ .../AWSTracingPipelineHandler.cs | 10 +- .../IAWSMessageAttributeFormatter.cs | 27 ++++ .../SnsMessageAttributeFormatter.cs | 34 +++++ .../SqsMessageAttributeFormatter.cs | 34 +++++ ...lemetry.Contrib.Instrumentation.AWS.csproj | 6 +- .../Implementation/AWSLambdaUtils.cs | 14 ++ .../Implementation/AWSMessagingUtils.cs | 141 ++++++++++++++++++ ...Telemetry.Instrumentation.AWSLambda.csproj | 4 +- .../AWSMessageAttributeHelperTests.cs | 94 ++++++++++++ .../Implementation/TestsHelper.cs | 82 ++++++++++ ...y.Contrib.Instrumentation.AWS.Tests.csproj | 5 +- .../Tools/Utils.cs | 2 +- 16 files changed, 638 insertions(+), 17 deletions(-) create mode 100644 src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessageAttributeHelper.cs create mode 100644 src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs create mode 100644 src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSServiceType.cs create mode 100644 src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IAWSMessageAttributeFormatter.cs create mode 100644 src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsMessageAttributeFormatter.cs create mode 100644 src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsMessageAttributeFormatter.cs create mode 100644 src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs create mode 100644 test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessageAttributeHelperTests.cs create mode 100644 test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessageAttributeHelper.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessageAttributeHelper.cs new file mode 100644 index 0000000000..5d2b76539a --- /dev/null +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessageAttributeHelper.cs @@ -0,0 +1,82 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System; +using System.Linq; +using Amazon.Runtime; +using Amazon.Runtime.Internal; + +namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; + +internal class AWSMessageAttributeHelper +{ + // SQS/SNS message attributes collection size limit according to + // https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-message-metadata.html and + // https://docs.aws.amazon.com/sns/latest/dg/sns-message-attributes.html + private const int MaxMessageAttributes = 10; + + private readonly IAWSMessageAttributeFormatter attributeFormatter; + + internal AWSMessageAttributeHelper(IAWSMessageAttributeFormatter attributeFormatter) + { + this.attributeFormatter = attributeFormatter ?? throw new ArgumentNullException(nameof(attributeFormatter)); + } + + internal bool TryAddParameter(ParameterCollection parameters, string name, string value) + { + var index = this.GetNextAttributeIndex(parameters, name); + if (!index.HasValue) + { + return false; + } + else if (index >= MaxMessageAttributes) + { + // TODO: Add logging (event source). + return false; + } + + var attributePrefix = this.attributeFormatter.AttributeNamePrefix + $".{index.Value}"; + parameters.Add(attributePrefix + ".Name", name.Trim()); + parameters.Add(attributePrefix + ".Value.DataType", "String"); + parameters.Add(attributePrefix + ".Value.StringValue", value.Trim()); + + return true; + } + + private int? GetNextAttributeIndex(ParameterCollection parameters, string name) + { + var names = parameters.Where(a => this.attributeFormatter.AttributeNameRegex.IsMatch(a.Key)); + if (!names.Any()) + { + return 1; + } + + int? index = 0; + foreach (var nameAttribute in names) + { + if (nameAttribute.Value is StringParameterValue param && param.Value == name) + { + index = null; + break; + } + + var currentIndex = this.attributeFormatter.GetAttributeIndex(nameAttribute.Key); + index = (currentIndex ?? 0) > index ? currentIndex : index; + } + + return ++index; + } +} diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs new file mode 100644 index 0000000000..b46ec0fa8b --- /dev/null +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs @@ -0,0 +1,74 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using Amazon.Runtime; +using SNS = Amazon.SimpleNotificationService.Model; +using SQS = Amazon.SQS.Model; + +namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; + +internal static class AWSMessagingUtils +{ + private static readonly AWSMessageAttributeHelper SqsAttributeHelper = new(new SqsMessageAttributeFormatter()); + private static readonly AWSMessageAttributeHelper SnsAttributeHelper = new(new SnsMessageAttributeFormatter()); + + internal static void SqsMessageAttributeSetter(IRequestContext context, string name, string value) + { + var parameters = context.Request?.ParameterCollection; + if (parameters == null || + !parameters.ContainsKey("MessageBody") || + context.OriginalRequest is not SQS::SendMessageRequest originalRequest) + { + return; + } + + // Add trace data to parameters collection of the request. + if (SqsAttributeHelper.TryAddParameter(parameters, name, value)) + { + // Add trace data to message attributes dictionary of the original request. + // This dictionary must be in sync with parameters collection to pass through the MD5 hash matching check. + if (!originalRequest.MessageAttributes.ContainsKey(name)) + { + originalRequest.MessageAttributes.Add( + name, new SQS::MessageAttributeValue + { DataType = "String", StringValue = value }); + } + } + } + + internal static void SnsMessageAttributeSetter(IRequestContext context, string name, string value) + { + var parameters = context.Request?.ParameterCollection; + if (parameters == null || + !parameters.ContainsKey("Message") || + context.OriginalRequest is not SNS::PublishRequest originalRequest) + { + return; + } + + if (SnsAttributeHelper.TryAddParameter(parameters, name, value)) + { + // Add trace data to message attributes dictionary of the original request. + // This dictionary must be in sync with parameters collection to pass through the MD5 hash matching check. + if (!originalRequest.MessageAttributes.ContainsKey(name)) + { + originalRequest.MessageAttributes.Add( + name, new SNS::MessageAttributeValue + { DataType = "String", StringValue = value }); + } + } + } +} diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSServiceHelper.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSServiceHelper.cs index b36964b4ea..8c11883331 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSServiceHelper.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSServiceHelper.cs @@ -14,7 +14,6 @@ // limitations under the License. // -using System; using System.Collections.Generic; using Amazon.Runtime; @@ -24,8 +23,8 @@ internal class AWSServiceHelper { internal static IReadOnlyDictionary ServiceParameterMap = new Dictionary() { - { DynamoDbService, "TableName" }, - { SQSService, "QueueUrl" }, + { AWSServiceType.DynamoDbService, "TableName" }, + { AWSServiceType.SQSService, "QueueUrl" }, }; internal static IReadOnlyDictionary ParameterAttributeMap = new Dictionary() @@ -34,9 +33,6 @@ internal class AWSServiceHelper { "QueueUrl", AWSSemanticConventions.AttributeAWSSQSQueueUrl }, }; - private const string DynamoDbService = "DynamoDBv2"; - private const string SQSService = "SQS"; - internal static string GetAWSServiceName(IRequestContext requestContext) => Utils.RemoveAmazonPrefixFromServiceName(requestContext.Request.ServiceName); @@ -47,7 +43,4 @@ internal static string GetAWSOperationName(IRequestContext requestContext) var operationName = Utils.RemoveSuffix(completeRequestName, suffix); return operationName; } - - internal static bool IsDynamoDbService(string service) - => DynamoDbService.Equals(service, StringComparison.OrdinalIgnoreCase); } diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSServiceType.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSServiceType.cs new file mode 100644 index 0000000000..24223f6f4b --- /dev/null +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSServiceType.cs @@ -0,0 +1,35 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System; + +namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; + +internal class AWSServiceType +{ + internal const string DynamoDbService = "DynamoDBv2"; + internal const string SQSService = "SQS"; + internal const string SNSService = "SimpleNotificationService"; + + internal static bool IsDynamoDbService(string service) + => DynamoDbService.Equals(service, StringComparison.OrdinalIgnoreCase); + + internal static bool IsSqsService(string service) + => SQSService.Equals(service, StringComparison.OrdinalIgnoreCase); + + internal static bool IsSnsService(string service) + => SNSService.Equals(service, StringComparison.OrdinalIgnoreCase); +} diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs index bbce612cf6..427e043c6b 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs @@ -196,10 +196,18 @@ private void AddRequestSpecificInformation(Activity activity, IRequestContext re } } - if (AWSServiceHelper.IsDynamoDbService(service)) + if (AWSServiceType.IsDynamoDbService(service)) { activity.SetTag(SemanticConventions.AttributeDbSystem, AWSSemanticConventions.AttributeValueDynamoDb); } + else if (AWSServiceType.IsSqsService(service)) + { + Propagators.DefaultTextMapPropagator.Inject(new PropagationContext(activity.Context, Baggage.Current), requestContext, AWSMessagingUtils.SqsMessageAttributeSetter); + } + else if (AWSServiceType.IsSnsService(service)) + { + Propagators.DefaultTextMapPropagator.Inject(new PropagationContext(activity.Context, Baggage.Current), requestContext, AWSMessagingUtils.SnsMessageAttributeSetter); + } } private void AddStatusCodeToActivity(Activity activity, int status_code) diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IAWSMessageAttributeFormatter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IAWSMessageAttributeFormatter.cs new file mode 100644 index 0000000000..536dbe8b50 --- /dev/null +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IAWSMessageAttributeFormatter.cs @@ -0,0 +1,27 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System.Text.RegularExpressions; + +namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; +internal interface IAWSMessageAttributeFormatter +{ + Regex AttributeNameRegex { get; } + + string AttributeNamePrefix { get; } + + int? GetAttributeIndex(string attributeName); +} diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsMessageAttributeFormatter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsMessageAttributeFormatter.cs new file mode 100644 index 0000000000..439dc469c8 --- /dev/null +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsMessageAttributeFormatter.cs @@ -0,0 +1,34 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System.Text.RegularExpressions; + +namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; + +internal class SnsMessageAttributeFormatter : IAWSMessageAttributeFormatter +{ + Regex IAWSMessageAttributeFormatter.AttributeNameRegex => new(@"MessageAttributes\.entry\.\d+\.Name"); + + string IAWSMessageAttributeFormatter.AttributeNamePrefix => "MessageAttributes.entry"; + + int? IAWSMessageAttributeFormatter.GetAttributeIndex(string attributeName) + { + var parts = attributeName.Split('.'); + return (parts.Length >= 3 && int.TryParse(parts[2], out int index)) ? + index : + null; + } +} diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsMessageAttributeFormatter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsMessageAttributeFormatter.cs new file mode 100644 index 0000000000..d3bfc9e4e8 --- /dev/null +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsMessageAttributeFormatter.cs @@ -0,0 +1,34 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System.Text.RegularExpressions; + +namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; + +internal class SqsMessageAttributeFormatter : IAWSMessageAttributeFormatter +{ + Regex IAWSMessageAttributeFormatter.AttributeNameRegex => new(@"MessageAttribute\.\d+\.Name"); + + string IAWSMessageAttributeFormatter.AttributeNamePrefix => "MessageAttribute"; + + int? IAWSMessageAttributeFormatter.GetAttributeIndex(string attributeName) + { + var parts = attributeName.Split('.'); + return (parts.Length >= 2 && int.TryParse(parts[1], out int index)) ? + index : + null; + } +} diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/OpenTelemetry.Contrib.Instrumentation.AWS.csproj b/src/OpenTelemetry.Contrib.Instrumentation.AWS/OpenTelemetry.Contrib.Instrumentation.AWS.csproj index 1a7b34718c..38b0be96b9 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/OpenTelemetry.Contrib.Instrumentation.AWS.csproj +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/OpenTelemetry.Contrib.Instrumentation.AWS.csproj @@ -8,12 +8,14 @@ + + - - + + diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSLambdaUtils.cs b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSLambdaUtils.cs index bda6d7677b..c5b794ffd0 100644 --- a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSLambdaUtils.cs +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSLambdaUtils.cs @@ -20,6 +20,8 @@ using System.Linq; using Amazon.Lambda.APIGatewayEvents; using Amazon.Lambda.Core; +using Amazon.Lambda.SNSEvents; +using Amazon.Lambda.SQSEvents; using OpenTelemetry.Context.Propagation; using OpenTelemetry.Contrib.Extensions.AWSXRay.Trace; @@ -74,6 +76,18 @@ internal static ActivityContext ExtractParentContext(TInput input) case APIGatewayHttpApiV2ProxyRequest apiGatewayHttpApiV2ProxyRequest: propagationContext = Propagators.DefaultTextMapPropagator.Extract(default, apiGatewayHttpApiV2ProxyRequest, GetHeaderValues); break; + case SQSEvent sqsEvent: + propagationContext = AWSMessagingUtils.ExtractParentContext(sqsEvent); + break; + case SQSEvent.SQSMessage sqsMessage: + propagationContext = AWSMessagingUtils.ExtractParentContext(sqsMessage); + break; + case SNSEvent snsEvent: + propagationContext = AWSMessagingUtils.ExtractParentContext(snsEvent); + break; + case SNSEvent.SNSRecord snsRecord: + propagationContext = AWSMessagingUtils.ExtractParentContext(snsRecord); + break; } return propagationContext.ActivityContext; diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs new file mode 100644 index 0000000000..f9663b9f88 --- /dev/null +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs @@ -0,0 +1,141 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System.Collections.Generic; +using System.Linq; +using Amazon.Lambda.SNSEvents; +using Amazon.Lambda.SQSEvents; +using Newtonsoft.Json; +using OpenTelemetry.Context.Propagation; + +namespace OpenTelemetry.Instrumentation.AWSLambda.Implementation; + +internal class AWSMessagingUtils +{ + // SNS attribute types: https://docs.aws.amazon.com/sns/latest/dg/sns-message-attributes.html + private const string SnsAttributeTypeString = "String"; + private const string SnsAttributeTypeStringArray = "String.Array"; + private const string SnsMessageAttributes = "MessageAttributes"; + + internal static PropagationContext ExtractParentContext(SQSEvent sqsEvent) + { + if (sqsEvent == null) + { + return default; + } + + // We assume there can be only one parent that's why we consider only a single (the last) record as the carrier. + var message = sqsEvent.Records.LastOrDefault(); + return ExtractParentContext(message); + } + + internal static PropagationContext ExtractParentContext(SQSEvent.SQSMessage sqsMessage) + { + if (sqsMessage == null) + { + return default; + } + + // SQS subscribed to SNS topic with raw delivery disabled case, i.e. SNS record serialized into SQS body. + // https://docs.aws.amazon.com/sns/latest/dg/sns-large-payload-raw-message-delivery.html + SNSEvent.SNSMessage snsMessage = GetSnsMessage(sqsMessage); + return snsMessage != null ? + ExtractParentContext(snsMessage) : + Propagators.DefaultTextMapPropagator.Extract(default, sqsMessage.MessageAttributes, SqsMessageAttributeGetter); + } + + internal static PropagationContext ExtractParentContext(SNSEvent snsEvent) + { + if (snsEvent == null) + { + return default; + } + + // We assume there can be only one parent that's why we consider only a single (the last) record as the carrier. + var record = snsEvent.Records.LastOrDefault(); + return ExtractParentContext(record); + } + + internal static PropagationContext ExtractParentContext(SNSEvent.SNSRecord record) + { + return (record?.Sns != null) ? + Propagators.DefaultTextMapPropagator.Extract(default, record.Sns.MessageAttributes, SnsMessageAttributeGetter) : + default; + } + + internal static PropagationContext ExtractParentContext(SNSEvent.SNSMessage message) + { + return (message != null) ? + Propagators.DefaultTextMapPropagator.Extract(default, message.MessageAttributes, SnsMessageAttributeGetter) : + default; + } + + private static IEnumerable SqsMessageAttributeGetter(IDictionary attributes, string attributeName) + { + SQSEvent.MessageAttribute attribute = attributes.GetValueByKeyIgnoringCase(attributeName); + if (attribute == null) + { + return null; + } + + return attribute.StringValue != null ? + new[] { attribute.StringValue } : + attribute.StringListValues; + } + + private static IEnumerable SnsMessageAttributeGetter(IDictionary attributes, string attributeName) + { + SNSEvent.MessageAttribute attribute = attributes.GetValueByKeyIgnoringCase(attributeName); + if (attribute == null) + { + return null; + } + + switch (attribute.Type) + { + case SnsAttributeTypeString when attribute.Value != null: + return new[] { attribute.Value }; + case SnsAttributeTypeStringArray when attribute.Value != null: + // Multiple values are stored as CSV (https://docs.aws.amazon.com/sns/latest/dg/sns-message-attributes.html). + return attribute.Value.Split(','); + default: + return null; + } + } + + private static SNSEvent.SNSMessage GetSnsMessage(SQSEvent.SQSMessage sqsMessage) + { + SNSEvent.SNSMessage snsMessage = null; + + var body = sqsMessage.Body; + if (body != null && + body.TrimStart().StartsWith("{") && + body.Contains(SnsMessageAttributes)) + { + try + { + snsMessage = JsonConvert.DeserializeObject(body); + } + catch (JsonException) + { + // TODO: log exception. + return null; + } + } + + return snsMessage; + } +} diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/OpenTelemetry.Instrumentation.AWSLambda.csproj b/src/OpenTelemetry.Instrumentation.AWSLambda/OpenTelemetry.Instrumentation.AWSLambda.csproj index 14a98b2675..e2ae6b6736 100644 --- a/src/OpenTelemetry.Instrumentation.AWSLambda/OpenTelemetry.Instrumentation.AWSLambda.csproj +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/OpenTelemetry.Instrumentation.AWSLambda.csproj @@ -10,13 +10,15 @@ + + - + diff --git a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessageAttributeHelperTests.cs b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessageAttributeHelperTests.cs new file mode 100644 index 0000000000..adc4c70167 --- /dev/null +++ b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessageAttributeHelperTests.cs @@ -0,0 +1,94 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System.Collections.Generic; +using Amazon.Runtime.Internal; +using OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; +using Xunit; + +namespace OpenTelemetry.Contrib.Instrumentation.AWS.Tests.Implementation; + +public class AWSMessageAttributeHelperTests +{ + [Theory] + [InlineData(AWSServiceType.SQSService)] + [InlineData(AWSServiceType.SNSService)] + public void TryAddParameter_CollectionSizeReachesLimit_ParameterNotAdded(string serviceType) + { + var helper = TestsHelper.CreateAttributeHelper(serviceType); + var parameters = new ParameterCollection(); + parameters.AddStringParameters(TestsHelper.GetNamePrefix(serviceType), 10); + + var added = helper.TryAddParameter(parameters, "testName", "testValue"); + + Assert.False(added, "Expected parameter not to be added."); + Assert.Equal(30, parameters.Count); + } + + [Theory] + [InlineData(AWSServiceType.SQSService)] + [InlineData(AWSServiceType.SNSService)] + public void TryAddParameter_EmptyCollection_ParameterAdded(string serviceType) + { + var helper = TestsHelper.CreateAttributeHelper(serviceType); + var parameters = new ParameterCollection(); + var expectedParameters = new List>() + { + new KeyValuePair("testName", "testValue"), + }; + + var added = helper.TryAddParameter(parameters, "testName", "testValue"); + + Assert.True(added, "Expected parameter to be added."); + TestsHelper.AssertStringParameters(expectedParameters, parameters, TestsHelper.GetNamePrefix(serviceType)); + } + + [Theory] + [InlineData(AWSServiceType.SQSService)] + [InlineData(AWSServiceType.SNSService)] + public void TryAddParameter_CollectionWithSingleParameter_SecondParameterAdded(string serviceType) + { + var helper = TestsHelper.CreateAttributeHelper(serviceType); + var parameters = new ParameterCollection(); + parameters.AddStringParameter("testNameA", "testValueA", TestsHelper.GetNamePrefix(serviceType), 1); + + var expectedParameters = new List>() + { + new KeyValuePair("testNameA", "testValueA"), + new KeyValuePair("testNameB", "testValueB"), + }; + + var added = helper.TryAddParameter(parameters, "testNameB", "testValueB"); + + Assert.True(added, "Expected parameter to be added."); + TestsHelper.AssertStringParameters(expectedParameters, parameters, TestsHelper.GetNamePrefix(serviceType)); + } + + [Theory] + [InlineData(AWSServiceType.SQSService)] + [InlineData(AWSServiceType.SNSService)] + public void TryAddParameter_ParameterPresentInCollection_ParameterNotAdded(string serviceType) + { + var helper = TestsHelper.CreateAttributeHelper(serviceType); + var parameters = new ParameterCollection(); + parameters.AddStringParameter("testNameA", "testValueA", TestsHelper.GetNamePrefix(serviceType), 1); + + var added = helper.TryAddParameter(parameters, "testNameA", "testValueA"); + + Assert.False(added, "Expected parameter not to be added."); + Assert.Equal(3, parameters.Count); + } +} diff --git a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs new file mode 100644 index 0000000000..b41f2acbf5 --- /dev/null +++ b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs @@ -0,0 +1,82 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System; +using System.Collections.Generic; +using Amazon.Runtime; +using Amazon.Runtime.Internal; +using OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; +using Xunit; + +namespace OpenTelemetry.Contrib.Instrumentation.AWS.Tests.Implementation; +internal static class TestsHelper +{ + internal static AWSMessageAttributeHelper CreateAttributeHelper(string serviceType) + { + return serviceType switch + { + AWSServiceType.SQSService => new(new SqsMessageAttributeFormatter()), + AWSServiceType.SNSService => new(new SnsMessageAttributeFormatter()), + _ => throw new NotSupportedException($"Tests for service type {serviceType} not supported."), + }; + } + + internal static string GetNamePrefix(string serviceType) + { + return serviceType switch + { + AWSServiceType.SQSService => "MessageAttribute", + AWSServiceType.SNSService => "MessageAttributes.entry", + _ => throw new NotSupportedException($"Tests for service type {serviceType} not supported."), + }; + } + + internal static void AddStringParameter(this ParameterCollection parameters, string name, string value, string namePrefix, int index) + { + var prefix = $"{namePrefix}.{index}"; + parameters.Add($"{prefix}.Name", name); + parameters.Add($"{prefix}.Value.DataType", "String"); + parameters.Add($"{prefix}.Value.StringValue", value); + } + + internal static void AddStringParameters(this ParameterCollection parameters, string namePrefix, int count) + { + for (int i = 1; i <= count; i++) + { + AddStringParameter(parameters, $"name{i}", $"value{i}", namePrefix, i); + } + } + + internal static void AssertStringParameters(List> expectedParameters, ParameterCollection actualParameters, string namePrefix) + { + Assert.Equal(expectedParameters.Count * 3, actualParameters.Count); + + for (int i = 0; i < expectedParameters.Count; i++) + { + var prefix = $"{namePrefix}.{i + 1}"; + static string Value(ParameterValue p) => (p as StringParameterValue).Value; + + Assert.True(actualParameters.ContainsKey($"{prefix}.Name")); + Assert.Equal(expectedParameters[i].Key, Value(actualParameters[$"{prefix}.Name"])); + + Assert.True(actualParameters.ContainsKey($"{prefix}.Value.DataType")); + Assert.Equal("String", Value(actualParameters[$"{prefix}.Value.DataType"])); + + Assert.True(actualParameters.ContainsKey($"{prefix}.Value.StringValue")); + Assert.Equal(expectedParameters[i].Value, Value(actualParameters[$"{prefix}.Value.StringValue"])); + } + } +} diff --git a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/OpenTelemetry.Contrib.Instrumentation.AWS.Tests.csproj b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/OpenTelemetry.Contrib.Instrumentation.AWS.Tests.csproj index e8fa57bb14..a30b9e351f 100644 --- a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/OpenTelemetry.Contrib.Instrumentation.AWS.Tests.csproj +++ b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/OpenTelemetry.Contrib.Instrumentation.AWS.Tests.csproj @@ -8,11 +8,10 @@ + - - - + all diff --git a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Tools/Utils.cs b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Tools/Utils.cs index 635f5efabf..65fd24ff65 100644 --- a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Tools/Utils.cs +++ b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Tools/Utils.cs @@ -71,6 +71,6 @@ public static IEnumerable FindResourceName(Predicate match) public static object GetTagValue(Activity activity, string tagName) { - return Implementation.Utils.GetTagValue(activity, tagName); + return AWS.Implementation.Utils.GetTagValue(activity, tagName); } } From 7f75e129973c95bcce3c44ed018294de7ba10731 Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Tue, 28 Feb 2023 10:39:16 +0100 Subject: [PATCH 02/23] #1034: fixed filenames in headers --- .../Implementation/SnsMessageAttributeFormatter.cs | 2 +- .../Implementation/SqsMessageAttributeFormatter.cs | 2 +- .../Implementation/AWSMessageAttributeHelperTests.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsMessageAttributeFormatter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsMessageAttributeFormatter.cs index 439dc469c8..ba5f23a9ec 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsMessageAttributeFormatter.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsMessageAttributeFormatter.cs @@ -1,4 +1,4 @@ -// +// // Copyright The OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsMessageAttributeFormatter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsMessageAttributeFormatter.cs index d3bfc9e4e8..329a2c7159 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsMessageAttributeFormatter.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsMessageAttributeFormatter.cs @@ -1,4 +1,4 @@ -// +// // Copyright The OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessageAttributeHelperTests.cs b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessageAttributeHelperTests.cs index adc4c70167..c24ae3de7d 100644 --- a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessageAttributeHelperTests.cs +++ b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessageAttributeHelperTests.cs @@ -1,4 +1,4 @@ -// +// // Copyright The OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); From c0ff714ab96de4c59e9079cbca8a83f314b45ab5 Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Tue, 28 Feb 2023 10:51:26 +0100 Subject: [PATCH 03/23] #1034: removed error CHARSET (fixed file encoding) + comment for used abbreviation --- .../Implementation/AWSServiceType.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSServiceType.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSServiceType.cs index 24223f6f4b..750fab7b23 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSServiceType.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSServiceType.cs @@ -1,4 +1,4 @@ -// +// // Copyright The OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -22,7 +22,7 @@ internal class AWSServiceType { internal const string DynamoDbService = "DynamoDBv2"; internal const string SQSService = "SQS"; - internal const string SNSService = "SimpleNotificationService"; + internal const string SNSService = "SimpleNotificationService"; // SNS internal static bool IsDynamoDbService(string service) => DynamoDbService.Equals(service, StringComparison.OrdinalIgnoreCase); From 4cc849a9a2f48bd6a481116bf52923872a330d58 Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Wed, 1 Mar 2023 18:15:32 +0100 Subject: [PATCH 04/23] #1034: refactored in order to support "nothing or all" trace data injection + adapted tests accordingly --- .../AWSMessageAttributeHelper.cs | 82 -------- .../Implementation/AWSMessagingUtils.cs | 64 +++---- .../AWSTracingPipelineHandler.cs | 4 +- ...Formatter.cs => IRequestContextAdapter.cs} | 18 +- .../SnsMessageAttributeFormatter.cs | 34 ---- .../SnsRequestContextAdapter.cs | 59 ++++++ .../SqsMessageAttributeFormatter.cs | 34 ---- .../SqsRequestContextAdapter.cs | 59 ++++++ .../AWSMessageAttributeHelperTests.cs | 94 ---------- .../Implementation/AWSMessagingUtilsTests.cs | 176 ++++++++++++++++++ .../Implementation/TestsHelper.cs | 125 ++++++++++--- 11 files changed, 437 insertions(+), 312 deletions(-) delete mode 100644 src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessageAttributeHelper.cs rename src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/{IAWSMessageAttributeFormatter.cs => IRequestContextAdapter.cs} (62%) delete mode 100644 src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsMessageAttributeFormatter.cs create mode 100644 src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextAdapter.cs delete mode 100644 src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsMessageAttributeFormatter.cs create mode 100644 src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs delete mode 100644 test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessageAttributeHelperTests.cs create mode 100644 test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessagingUtilsTests.cs diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessageAttributeHelper.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessageAttributeHelper.cs deleted file mode 100644 index 5d2b76539a..0000000000 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessageAttributeHelper.cs +++ /dev/null @@ -1,82 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -using System; -using System.Linq; -using Amazon.Runtime; -using Amazon.Runtime.Internal; - -namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; - -internal class AWSMessageAttributeHelper -{ - // SQS/SNS message attributes collection size limit according to - // https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-message-metadata.html and - // https://docs.aws.amazon.com/sns/latest/dg/sns-message-attributes.html - private const int MaxMessageAttributes = 10; - - private readonly IAWSMessageAttributeFormatter attributeFormatter; - - internal AWSMessageAttributeHelper(IAWSMessageAttributeFormatter attributeFormatter) - { - this.attributeFormatter = attributeFormatter ?? throw new ArgumentNullException(nameof(attributeFormatter)); - } - - internal bool TryAddParameter(ParameterCollection parameters, string name, string value) - { - var index = this.GetNextAttributeIndex(parameters, name); - if (!index.HasValue) - { - return false; - } - else if (index >= MaxMessageAttributes) - { - // TODO: Add logging (event source). - return false; - } - - var attributePrefix = this.attributeFormatter.AttributeNamePrefix + $".{index.Value}"; - parameters.Add(attributePrefix + ".Name", name.Trim()); - parameters.Add(attributePrefix + ".Value.DataType", "String"); - parameters.Add(attributePrefix + ".Value.StringValue", value.Trim()); - - return true; - } - - private int? GetNextAttributeIndex(ParameterCollection parameters, string name) - { - var names = parameters.Where(a => this.attributeFormatter.AttributeNameRegex.IsMatch(a.Key)); - if (!names.Any()) - { - return 1; - } - - int? index = 0; - foreach (var nameAttribute in names) - { - if (nameAttribute.Value is StringParameterValue param && param.Value == name) - { - index = null; - break; - } - - var currentIndex = this.attributeFormatter.GetAttributeIndex(nameAttribute.Key); - index = (currentIndex ?? 0) > index ? currentIndex : index; - } - - return ++index; - } -} diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs index b46ec0fa8b..fce9899942 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs @@ -14,61 +14,53 @@ // limitations under the License. // -using Amazon.Runtime; -using SNS = Amazon.SimpleNotificationService.Model; -using SQS = Amazon.SQS.Model; +using System.Collections.Generic; +using OpenTelemetry.Context.Propagation; namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; internal static class AWSMessagingUtils { - private static readonly AWSMessageAttributeHelper SqsAttributeHelper = new(new SqsMessageAttributeFormatter()); - private static readonly AWSMessageAttributeHelper SnsAttributeHelper = new(new SnsMessageAttributeFormatter()); + // SQS/SNS message attributes collection size limit according to + // https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-message-metadata.html and + // https://docs.aws.amazon.com/sns/latest/dg/sns-message-attributes.html + private const int MaxMessageAttributes = 10; - internal static void SqsMessageAttributeSetter(IRequestContext context, string name, string value) + internal static void Inject(IRequestContextAdapter requestAdapter, PropagationContext propagationContext) { - var parameters = context.Request?.ParameterCollection; - if (parameters == null || - !parameters.ContainsKey("MessageBody") || - context.OriginalRequest is not SQS::SendMessageRequest originalRequest) + if (!requestAdapter.HasMessageBody || + !requestAdapter.HasOriginalRequest) { return; } - // Add trace data to parameters collection of the request. - if (SqsAttributeHelper.TryAddParameter(parameters, name, value)) - { - // Add trace data to message attributes dictionary of the original request. - // This dictionary must be in sync with parameters collection to pass through the MD5 hash matching check. - if (!originalRequest.MessageAttributes.ContainsKey(name)) - { - originalRequest.MessageAttributes.Add( - name, new SQS::MessageAttributeValue - { DataType = "String", StringValue = value }); - } - } - } + var carrier = new Dictionary(); + Propagators.DefaultTextMapPropagator.Inject(propagationContext, carrier, Setter); - internal static void SnsMessageAttributeSetter(IRequestContext context, string name, string value) - { - var parameters = context.Request?.ParameterCollection; - if (parameters == null || - !parameters.ContainsKey("Message") || - context.OriginalRequest is not SNS::PublishRequest originalRequest) + int attributesCount = requestAdapter.AttributesCount; + if (carrier.Count + attributesCount > MaxMessageAttributes) { + // TODO: Add logging (event source). return; } - if (SnsAttributeHelper.TryAddParameter(parameters, name, value)) + int nextAttributeIndex = attributesCount + 1; + foreach (var param in carrier) { - // Add trace data to message attributes dictionary of the original request. - // This dictionary must be in sync with parameters collection to pass through the MD5 hash matching check. - if (!originalRequest.MessageAttributes.ContainsKey(name)) + if (requestAdapter.ContainsAttribute(param.Key)) { - originalRequest.MessageAttributes.Add( - name, new SNS::MessageAttributeValue - { DataType = "String", StringValue = value }); + continue; } + + requestAdapter.AddAttribute(param.Key, param.Value, nextAttributeIndex); + nextAttributeIndex++; + + // Add trace data to message attributes dictionary of the original request. + // This dictionary must be in sync with parameters collection to pass through the MD5 hash matching check. + requestAdapter.AddAttributeToOriginalRequest(param.Key, param.Value); } } + + private static void Setter(IDictionary carrier, string name, string value) => + carrier[name] = value; } diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs index 427e043c6b..9c8effd042 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs @@ -202,11 +202,11 @@ private void AddRequestSpecificInformation(Activity activity, IRequestContext re } else if (AWSServiceType.IsSqsService(service)) { - Propagators.DefaultTextMapPropagator.Inject(new PropagationContext(activity.Context, Baggage.Current), requestContext, AWSMessagingUtils.SqsMessageAttributeSetter); + AWSMessagingUtils.Inject(new SqsRequestContextAdapter(requestContext), new PropagationContext(activity.Context, Baggage.Current)); } else if (AWSServiceType.IsSnsService(service)) { - Propagators.DefaultTextMapPropagator.Inject(new PropagationContext(activity.Context, Baggage.Current), requestContext, AWSMessagingUtils.SnsMessageAttributeSetter); + AWSMessagingUtils.Inject(new SnsRequestContextAdapter(requestContext), new PropagationContext(activity.Context, Baggage.Current)); } } diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IAWSMessageAttributeFormatter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IRequestContextAdapter.cs similarity index 62% rename from src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IAWSMessageAttributeFormatter.cs rename to src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IRequestContextAdapter.cs index 536dbe8b50..2da881aeb1 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IAWSMessageAttributeFormatter.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IRequestContextAdapter.cs @@ -1,4 +1,4 @@ -// +// // Copyright The OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -14,14 +14,18 @@ // limitations under the License. // -using System.Text.RegularExpressions; - namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; -internal interface IAWSMessageAttributeFormatter +internal interface IRequestContextAdapter { - Regex AttributeNameRegex { get; } + bool HasMessageBody { get; } + + bool HasOriginalRequest { get; } + + int AttributesCount { get; } + + bool ContainsAttribute(string name); - string AttributeNamePrefix { get; } + void AddAttribute(string name, string value, int nextAttributeIndex); - int? GetAttributeIndex(string attributeName); + void AddAttributeToOriginalRequest(string name, string value); } diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsMessageAttributeFormatter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsMessageAttributeFormatter.cs deleted file mode 100644 index ba5f23a9ec..0000000000 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsMessageAttributeFormatter.cs +++ /dev/null @@ -1,34 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -using System.Text.RegularExpressions; - -namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; - -internal class SnsMessageAttributeFormatter : IAWSMessageAttributeFormatter -{ - Regex IAWSMessageAttributeFormatter.AttributeNameRegex => new(@"MessageAttributes\.entry\.\d+\.Name"); - - string IAWSMessageAttributeFormatter.AttributeNamePrefix => "MessageAttributes.entry"; - - int? IAWSMessageAttributeFormatter.GetAttributeIndex(string attributeName) - { - var parts = attributeName.Split('.'); - return (parts.Length >= 3 && int.TryParse(parts[2], out int index)) ? - index : - null; - } -} diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextAdapter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextAdapter.cs new file mode 100644 index 0000000000..dd40d44545 --- /dev/null +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextAdapter.cs @@ -0,0 +1,59 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using Amazon.Runtime; +using Amazon.Runtime.Internal; +using Amazon.SimpleNotificationService.Model; + +namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; +internal class SnsRequestContextAdapter : IRequestContextAdapter +{ + private readonly ParameterCollection parameters; + private readonly PublishRequest originalRequest; + + public SnsRequestContextAdapter(IRequestContext context) + { + this.parameters = context.Request?.ParameterCollection; + this.originalRequest = context.OriginalRequest as PublishRequest; + } + + public bool HasMessageBody => + this.parameters?.ContainsKey("Message") ?? false; + + public bool HasOriginalRequest => this.originalRequest != null; + + public int AttributesCount => + this.originalRequest?.MessageAttributes.Count ?? 0; + + public void AddAttribute(string name, string value, int nextAttributeIndex) + { + if (this.parameters == null) + { + return; + } + + var attributePrefix = "MessageAttributes.entry." + nextAttributeIndex; + this.parameters.Add(attributePrefix + ".Name", name); + this.parameters.Add(attributePrefix + ".Value.DataType", "String"); + this.parameters.Add(attributePrefix + ".Value.StringValue", value); + } + + public void AddAttributeToOriginalRequest(string name, string value) => + this.originalRequest?.MessageAttributes.Add(name, new MessageAttributeValue { DataType = "String", StringValue = value }); + + public bool ContainsAttribute(string name) + => this.originalRequest?.MessageAttributes.ContainsKey(name) ?? false; +} diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsMessageAttributeFormatter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsMessageAttributeFormatter.cs deleted file mode 100644 index 329a2c7159..0000000000 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsMessageAttributeFormatter.cs +++ /dev/null @@ -1,34 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -using System.Text.RegularExpressions; - -namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; - -internal class SqsMessageAttributeFormatter : IAWSMessageAttributeFormatter -{ - Regex IAWSMessageAttributeFormatter.AttributeNameRegex => new(@"MessageAttribute\.\d+\.Name"); - - string IAWSMessageAttributeFormatter.AttributeNamePrefix => "MessageAttribute"; - - int? IAWSMessageAttributeFormatter.GetAttributeIndex(string attributeName) - { - var parts = attributeName.Split('.'); - return (parts.Length >= 2 && int.TryParse(parts[1], out int index)) ? - index : - null; - } -} diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs new file mode 100644 index 0000000000..42f90cab18 --- /dev/null +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs @@ -0,0 +1,59 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using Amazon.Runtime; +using Amazon.Runtime.Internal; +using Amazon.SQS.Model; + +namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; +internal class SqsRequestContextAdapter : IRequestContextAdapter +{ + private readonly ParameterCollection parameters; + private readonly SendMessageRequest originalRequest; + + public SqsRequestContextAdapter(IRequestContext context) + { + this.parameters = context.Request?.ParameterCollection; + this.originalRequest = context.OriginalRequest as SendMessageRequest; + } + + public bool HasMessageBody => + this.parameters?.ContainsKey("MessageBody") ?? false; + + public bool HasOriginalRequest => this.originalRequest != null; + + public int AttributesCount => + this.originalRequest?.MessageAttributes.Count ?? 0; + + public void AddAttribute(string name, string value, int nextAttributeIndex) + { + if (this.parameters == null) + { + return; + } + + var attributePrefix = "MessageAttribute." + nextAttributeIndex; + this.parameters.Add(attributePrefix + ".Name", name); + this.parameters.Add(attributePrefix + ".Value.DataType", "String"); + this.parameters.Add(attributePrefix + ".Value.StringValue", value); + } + + public void AddAttributeToOriginalRequest(string name, string value) => + this.originalRequest?.MessageAttributes.Add(name, new MessageAttributeValue { DataType = "String", StringValue = value }); + + public bool ContainsAttribute(string name) => + this.originalRequest?.MessageAttributes.ContainsKey(name) ?? false; +} diff --git a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessageAttributeHelperTests.cs b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessageAttributeHelperTests.cs deleted file mode 100644 index c24ae3de7d..0000000000 --- a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessageAttributeHelperTests.cs +++ /dev/null @@ -1,94 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -using System.Collections.Generic; -using Amazon.Runtime.Internal; -using OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; -using Xunit; - -namespace OpenTelemetry.Contrib.Instrumentation.AWS.Tests.Implementation; - -public class AWSMessageAttributeHelperTests -{ - [Theory] - [InlineData(AWSServiceType.SQSService)] - [InlineData(AWSServiceType.SNSService)] - public void TryAddParameter_CollectionSizeReachesLimit_ParameterNotAdded(string serviceType) - { - var helper = TestsHelper.CreateAttributeHelper(serviceType); - var parameters = new ParameterCollection(); - parameters.AddStringParameters(TestsHelper.GetNamePrefix(serviceType), 10); - - var added = helper.TryAddParameter(parameters, "testName", "testValue"); - - Assert.False(added, "Expected parameter not to be added."); - Assert.Equal(30, parameters.Count); - } - - [Theory] - [InlineData(AWSServiceType.SQSService)] - [InlineData(AWSServiceType.SNSService)] - public void TryAddParameter_EmptyCollection_ParameterAdded(string serviceType) - { - var helper = TestsHelper.CreateAttributeHelper(serviceType); - var parameters = new ParameterCollection(); - var expectedParameters = new List>() - { - new KeyValuePair("testName", "testValue"), - }; - - var added = helper.TryAddParameter(parameters, "testName", "testValue"); - - Assert.True(added, "Expected parameter to be added."); - TestsHelper.AssertStringParameters(expectedParameters, parameters, TestsHelper.GetNamePrefix(serviceType)); - } - - [Theory] - [InlineData(AWSServiceType.SQSService)] - [InlineData(AWSServiceType.SNSService)] - public void TryAddParameter_CollectionWithSingleParameter_SecondParameterAdded(string serviceType) - { - var helper = TestsHelper.CreateAttributeHelper(serviceType); - var parameters = new ParameterCollection(); - parameters.AddStringParameter("testNameA", "testValueA", TestsHelper.GetNamePrefix(serviceType), 1); - - var expectedParameters = new List>() - { - new KeyValuePair("testNameA", "testValueA"), - new KeyValuePair("testNameB", "testValueB"), - }; - - var added = helper.TryAddParameter(parameters, "testNameB", "testValueB"); - - Assert.True(added, "Expected parameter to be added."); - TestsHelper.AssertStringParameters(expectedParameters, parameters, TestsHelper.GetNamePrefix(serviceType)); - } - - [Theory] - [InlineData(AWSServiceType.SQSService)] - [InlineData(AWSServiceType.SNSService)] - public void TryAddParameter_ParameterPresentInCollection_ParameterNotAdded(string serviceType) - { - var helper = TestsHelper.CreateAttributeHelper(serviceType); - var parameters = new ParameterCollection(); - parameters.AddStringParameter("testNameA", "testValueA", TestsHelper.GetNamePrefix(serviceType), 1); - - var added = helper.TryAddParameter(parameters, "testNameA", "testValueA"); - - Assert.False(added, "Expected parameter not to be added."); - Assert.Equal(3, parameters.Count); - } -} diff --git a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessagingUtilsTests.cs b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessagingUtilsTests.cs new file mode 100644 index 0000000000..d3eedc0355 --- /dev/null +++ b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessagingUtilsTests.cs @@ -0,0 +1,176 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using Amazon.Runtime; +using Amazon.Runtime.Internal; +using Moq; +using OpenTelemetry.Context.Propagation; +using OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; +using OpenTelemetry.Trace; +using Xunit; + +namespace OpenTelemetry.Contrib.Instrumentation.AWS.Tests.Implementation; + +public class AWSMessagingUtilsTests +{ + private const string TraceId = "5759e988bd862e3fe1be46a994272793"; + private const string ParentId = "53995c3f42cd8ad8"; + private const string TraceState = "trace-state"; + + public AWSMessagingUtilsTests() + { + Sdk.CreateTracerProviderBuilder() + .Build(); + } + + [Theory] + [InlineData(AWSServiceType.SQSService)] + [InlineData(AWSServiceType.SNSService)] + public void Inject_ParametersCollectionSizeReachesLimit_TraceDataNotInjected(string serviceType) + { + AmazonWebServiceRequest originalRequest = TestsHelper.CreateOriginalRequest(serviceType, 10); + var parameters = TestsHelper.DefaultParameterCollection(serviceType); + parameters.AddStringParameters(serviceType, originalRequest); + + var request = new Mock(); + request.Setup(x => x.ParameterCollection) + .Returns(parameters); + + var context = new Mock(); + context.Setup(x => x.OriginalRequest) + .Returns(originalRequest); + context.Setup(x => x.Request) + .Returns(request.Object); + + var adapter = TestsHelper.CreateRequestContextAdapter(serviceType, context.Object); + + AWSMessagingUtils.Inject(adapter, CreatePropagationContext()); + + Assert.Equal(31, parameters.Count); + } + + [Theory] + [InlineData(AWSServiceType.SQSService)] + [InlineData(AWSServiceType.SNSService)] + public void Inject_DefaultParametersCollection_TraceDataInjected(string serviceType) + { + var expectedParameters = new List>() + { + new KeyValuePair("traceparent", $"00-{TraceId}-{ParentId}-00"), + new KeyValuePair("tracestate", "trace-state"), + }; + + AmazonWebServiceRequest originalRequest = TestsHelper.CreateOriginalRequest(serviceType, 0); + var parameters = TestsHelper.DefaultParameterCollection(serviceType); + + var request = new Mock(); + request.Setup(x => x.ParameterCollection) + .Returns(parameters); + + var context = new Mock(); + context.Setup(x => x.OriginalRequest) + .Returns(originalRequest); + context.Setup(x => x.Request) + .Returns(request.Object); + + var adapter = TestsHelper.CreateRequestContextAdapter(serviceType, context.Object); + + AWSMessagingUtils.Inject(adapter, CreatePropagationContext()); + + TestsHelper.AssertStringParameters(serviceType, expectedParameters, parameters); + } + + [Theory] + [InlineData(AWSServiceType.SQSService)] + [InlineData(AWSServiceType.SNSService)] + public void Inject_ParametersCollectionWithCustomParameter_TraceDataInjected(string serviceType) + { + var expectedParameters = new List>() + { + new KeyValuePair("name1", "value1"), + new KeyValuePair("traceparent", $"00-{TraceId}-{ParentId}-00"), + new KeyValuePair("tracestate", "trace-state"), + }; + + AmazonWebServiceRequest originalRequest = TestsHelper.CreateOriginalRequest(serviceType, 1); + var parameters = TestsHelper.DefaultParameterCollection(serviceType); + parameters.AddStringParameters(serviceType, originalRequest); + + var request = new Mock(); + request.Setup(x => x.ParameterCollection) + .Returns(parameters); + + var context = new Mock(); + context.Setup(x => x.OriginalRequest) + .Returns(originalRequest); + context.Setup(x => x.Request) + .Returns(request.Object); + + var adapter = TestsHelper.CreateRequestContextAdapter(serviceType, context.Object); + + AWSMessagingUtils.Inject(adapter, CreatePropagationContext()); + + TestsHelper.AssertStringParameters(serviceType, expectedParameters, parameters); + } + + [Theory] + [InlineData(AWSServiceType.SQSService)] + [InlineData(AWSServiceType.SNSService)] + public void Inject_ParametersCollectionWithTraceData_TraceDataNotInjected(string serviceType) + { + var expectedParameters = new List>() + { + new KeyValuePair("traceparent", $"00-{TraceId}-{ParentId}-00"), + new KeyValuePair("tracestate", "trace-state"), + }; + + AmazonWebServiceRequest originalRequest = TestsHelper.CreateOriginalRequest(serviceType, 0); + originalRequest.AddAttribute("traceparent", $"00-{TraceId}-{ParentId}-00"); + originalRequest.AddAttribute("tracestate", $"trace-state"); + + var parameters = TestsHelper.DefaultParameterCollection(serviceType); + parameters.AddStringParameters(serviceType, originalRequest); + + var request = new Mock(); + request.Setup(x => x.ParameterCollection) + .Returns(parameters); + + var context = new Mock(); + context.Setup(x => x.OriginalRequest) + .Returns(originalRequest); + context.Setup(x => x.Request) + .Returns(request.Object); + + var adapter = TestsHelper.CreateRequestContextAdapter(serviceType, context.Object); + + AWSMessagingUtils.Inject(adapter, CreatePropagationContext()); + + TestsHelper.AssertStringParameters(serviceType, expectedParameters, parameters); + } + + private static PropagationContext CreatePropagationContext() + { + var traceId = ActivityTraceId.CreateFromString(TraceId.AsSpan()); + var parentId = ActivitySpanId.CreateFromString(ParentId.AsSpan()); + var traceFlags = ActivityTraceFlags.None; + var activityContext = new ActivityContext(traceId, parentId, traceFlags, TraceState, isRemote: true); + + return new PropagationContext(activityContext, Baggage.Current); + } +} diff --git a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs index b41f2acbf5..8a366a0e7d 100644 --- a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs +++ b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs @@ -16,67 +16,146 @@ using System; using System.Collections.Generic; +using System.Diagnostics; +using System.Drawing; using Amazon.Runtime; using Amazon.Runtime.Internal; +using OpenTelemetry.Context.Propagation; using OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; using Xunit; +using SNS = Amazon.SimpleNotificationService.Model; +using SQS = Amazon.SQS.Model; namespace OpenTelemetry.Contrib.Instrumentation.AWS.Tests.Implementation; internal static class TestsHelper { - internal static AWSMessageAttributeHelper CreateAttributeHelper(string serviceType) + internal static IRequestContextAdapter CreateRequestContextAdapter(string serviceType, IRequestContext context) { return serviceType switch { - AWSServiceType.SQSService => new(new SqsMessageAttributeFormatter()), - AWSServiceType.SNSService => new(new SnsMessageAttributeFormatter()), + AWSServiceType.SQSService => new SqsRequestContextAdapter(context), + AWSServiceType.SNSService => new SnsRequestContextAdapter(context), _ => throw new NotSupportedException($"Tests for service type {serviceType} not supported."), }; } - internal static string GetNamePrefix(string serviceType) + internal static AmazonWebServiceRequest CreateOriginalRequest(string serviceType, int attributesCount) { - return serviceType switch + AmazonWebServiceRequest resultRequest; + var sendRequest = new SQS::SendMessageRequest(); + var publishRequest = new SNS::PublishRequest(); + Action addAttribute; + + switch (serviceType) { - AWSServiceType.SQSService => "MessageAttribute", - AWSServiceType.SNSService => "MessageAttributes.entry", - _ => throw new NotSupportedException($"Tests for service type {serviceType} not supported."), - }; + case AWSServiceType.SQSService: + resultRequest = sendRequest; + addAttribute = i => sendRequest.MessageAttributes.Add($"name{i}", new SQS::MessageAttributeValue { DataType = "String", StringValue = $"value{i}" }); + break; + case AWSServiceType.SNSService: + resultRequest = publishRequest; + addAttribute = i => publishRequest.MessageAttributes.Add($"name{i}", new SNS::MessageAttributeValue { DataType = "String", StringValue = $"value{i}" }); + break; + default: + throw new NotSupportedException($"Tests for service type {serviceType} not supported."); + } + + for (int i = 1; i <= attributesCount; i++) + { + addAttribute(i); + } + + return resultRequest; } - internal static void AddStringParameter(this ParameterCollection parameters, string name, string value, string namePrefix, int index) + internal static void AddAttribute(this AmazonWebServiceRequest serviceRequest, string name, string value) { - var prefix = $"{namePrefix}.{index}"; + var sendRequest = serviceRequest as SQS::SendMessageRequest; + var publishRequest = serviceRequest as SNS::PublishRequest; + if (sendRequest != null) + { + sendRequest.MessageAttributes.Add(name, new SQS::MessageAttributeValue { DataType = "String", StringValue = value }); + } + else if (publishRequest != null) + { + publishRequest.MessageAttributes.Add(name, new SNS::MessageAttributeValue { DataType = "String", StringValue = value }); + } + } + + internal static void AddStringParameter(this ParameterCollection parameters, string serviceType, string name, string value, int index) + { + var prefix = $"{GetNamePrefix(serviceType)}.{index}"; parameters.Add($"{prefix}.Name", name); parameters.Add($"{prefix}.Value.DataType", "String"); parameters.Add($"{prefix}.Value.StringValue", value); } - internal static void AddStringParameters(this ParameterCollection parameters, string namePrefix, int count) + internal static void AddStringParameters(this ParameterCollection parameters, string serviceType, AmazonWebServiceRequest serviceRequest) { - for (int i = 1; i <= count; i++) + var sendRequest = serviceRequest as SQS::SendMessageRequest; + var publishRequest = serviceRequest as SNS::PublishRequest; + int index = 1; + if (sendRequest != null) { - AddStringParameter(parameters, $"name{i}", $"value{i}", namePrefix, i); + foreach (var a in sendRequest.MessageAttributes) + { + AddStringParameter(parameters, serviceType, a.Key, a.Value.StringValue, index++); + } + } + else if (publishRequest != null) + { + foreach (var a in publishRequest.MessageAttributes) + { + AddStringParameter(parameters, serviceType, a.Key, a.Value.StringValue, index++); + } } } - internal static void AssertStringParameters(List> expectedParameters, ParameterCollection actualParameters, string namePrefix) + internal static void AssertStringParameters(string serviceType, List> expectedParameters, ParameterCollection parameters) { - Assert.Equal(expectedParameters.Count * 3, actualParameters.Count); + Assert.Equal((expectedParameters.Count * 3) + 1, parameters.Count); for (int i = 0; i < expectedParameters.Count; i++) { - var prefix = $"{namePrefix}.{i + 1}"; + var prefix = $"{GetNamePrefix(serviceType)}.{i + 1}"; static string Value(ParameterValue p) => (p as StringParameterValue).Value; - Assert.True(actualParameters.ContainsKey($"{prefix}.Name")); - Assert.Equal(expectedParameters[i].Key, Value(actualParameters[$"{prefix}.Name"])); + Assert.True(parameters.ContainsKey($"{prefix}.Name")); + Assert.Equal(expectedParameters[i].Key, Value(parameters[$"{prefix}.Name"])); - Assert.True(actualParameters.ContainsKey($"{prefix}.Value.DataType")); - Assert.Equal("String", Value(actualParameters[$"{prefix}.Value.DataType"])); + Assert.True(parameters.ContainsKey($"{prefix}.Value.DataType")); + Assert.Equal("String", Value(parameters[$"{prefix}.Value.DataType"])); - Assert.True(actualParameters.ContainsKey($"{prefix}.Value.StringValue")); - Assert.Equal(expectedParameters[i].Value, Value(actualParameters[$"{prefix}.Value.StringValue"])); + Assert.True(parameters.ContainsKey($"{prefix}.Value.StringValue")); + Assert.Equal(expectedParameters[i].Value, Value(parameters[$"{prefix}.Value.StringValue"])); } } + + internal static ParameterCollection DefaultParameterCollection(string serviceType) + { + return new ParameterCollection + { + { GetMessageBodyAttributeName(serviceType), string.Empty }, + }; + } + + private static string GetNamePrefix(string serviceType) + { + return serviceType switch + { + AWSServiceType.SQSService => "MessageAttribute", + AWSServiceType.SNSService => "MessageAttributes.entry", + _ => throw new NotSupportedException($"Tests for service type {serviceType} not supported."), + }; + } + + private static string GetMessageBodyAttributeName(string serviceType) + { + return serviceType switch + { + AWSServiceType.SQSService => "MessageBody", + AWSServiceType.SNSService => "Message", + _ => throw new NotSupportedException($"Tests for service type {serviceType} not supported."), + }; + } } From 8bdad4d601f92231943037e0b4128cae5202edc1 Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Thu, 2 Mar 2023 10:53:08 +0100 Subject: [PATCH 05/23] #1034: swap the order of parent context check for the SNS in SQS case --- .../Implementation/AWSMessagingUtils.cs | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs index f9663b9f88..9ba1f1561f 100644 --- a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs @@ -14,6 +14,7 @@ // limitations under the License. // +using System; using System.Collections.Generic; using System.Linq; using Amazon.Lambda.SNSEvents; @@ -49,12 +50,19 @@ internal static PropagationContext ExtractParentContext(SQSEvent.SQSMessage sqsM return default; } - // SQS subscribed to SNS topic with raw delivery disabled case, i.e. SNS record serialized into SQS body. - // https://docs.aws.amazon.com/sns/latest/dg/sns-large-payload-raw-message-delivery.html - SNSEvent.SNSMessage snsMessage = GetSnsMessage(sqsMessage); - return snsMessage != null ? - ExtractParentContext(snsMessage) : - Propagators.DefaultTextMapPropagator.Extract(default, sqsMessage.MessageAttributes, SqsMessageAttributeGetter); + var parentContext = Propagators.DefaultTextMapPropagator.Extract(default, sqsMessage.MessageAttributes, SqsMessageAttributeGetter); + if (parentContext == default) + { + // SQS subscribed to SNS topic with raw delivery disabled case, i.e. SNS record serialized into SQS body. + // https://docs.aws.amazon.com/sns/latest/dg/sns-large-payload-raw-message-delivery.html + SNSEvent.SNSMessage snsMessage = GetSnsMessage(sqsMessage); + if (snsMessage != null) + { + parentContext = ExtractParentContext(snsMessage); + } + } + + return parentContext; } internal static PropagationContext ExtractParentContext(SNSEvent snsEvent) @@ -129,7 +137,7 @@ private static SNSEvent.SNSMessage GetSnsMessage(SQSEvent.SQSMessage sqsMessage) { snsMessage = JsonConvert.DeserializeObject(body); } - catch (JsonException) + catch (Exception) { // TODO: log exception. return null; From 1147917fe889025d4f2f9602db8246d0670729cc Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Thu, 2 Mar 2023 16:23:44 +0100 Subject: [PATCH 06/23] #1034: review suggestions and other improvements --- .../Implementation/AWSMessagingUtils.cs | 28 ++++++++-------- .../Implementation/IRequestContextAdapter.cs | 8 ++--- .../SnsRequestContextAdapter.cs | 18 +++++------ .../SqsRequestContextAdapter.cs | 19 +++++------ .../Implementation/AWSMessagingUtils.cs | 32 ++++++------------- .../Implementation/AWSMessagingUtilsTests.cs | 20 ++++++------ .../Implementation/TestsHelper.cs | 23 +------------ 7 files changed, 55 insertions(+), 93 deletions(-) diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs index fce9899942..c52693bd20 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs @@ -15,6 +15,7 @@ // using System.Collections.Generic; +using System.Linq; using OpenTelemetry.Context.Propagation; namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; @@ -26,41 +27,38 @@ internal static class AWSMessagingUtils // https://docs.aws.amazon.com/sns/latest/dg/sns-message-attributes.html private const int MaxMessageAttributes = 10; - internal static void Inject(IRequestContextAdapter requestAdapter, PropagationContext propagationContext) + internal static void Inject(IRequestContextAdapter request, PropagationContext propagationContext) { - if (!requestAdapter.HasMessageBody || - !requestAdapter.HasOriginalRequest) + if (!request.CanInject) { return; } var carrier = new Dictionary(); - Propagators.DefaultTextMapPropagator.Inject(propagationContext, carrier, Setter); + Propagators.DefaultTextMapPropagator.Inject(propagationContext, carrier, (c, k, v) => c[k] = v); + if (carrier.Keys.Any(k => request.ContainsAttribute(k))) + { + // If at least one attribute is already present in the request then we skip the injection. + return; + } - int attributesCount = requestAdapter.AttributesCount; + int attributesCount = request.AttributesCount; if (carrier.Count + attributesCount > MaxMessageAttributes) { - // TODO: Add logging (event source). + // TODO: add logging (event source). return; } int nextAttributeIndex = attributesCount + 1; foreach (var param in carrier) { - if (requestAdapter.ContainsAttribute(param.Key)) + if (request.ContainsAttribute(param.Key)) { continue; } - requestAdapter.AddAttribute(param.Key, param.Value, nextAttributeIndex); + request.AddAttribute(param.Key, param.Value, nextAttributeIndex); nextAttributeIndex++; - - // Add trace data to message attributes dictionary of the original request. - // This dictionary must be in sync with parameters collection to pass through the MD5 hash matching check. - requestAdapter.AddAttributeToOriginalRequest(param.Key, param.Value); } } - - private static void Setter(IDictionary carrier, string name, string value) => - carrier[name] = value; } diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IRequestContextAdapter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IRequestContextAdapter.cs index 2da881aeb1..eb4cdfb994 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IRequestContextAdapter.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IRequestContextAdapter.cs @@ -17,15 +17,11 @@ namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; internal interface IRequestContextAdapter { - bool HasMessageBody { get; } - - bool HasOriginalRequest { get; } + bool CanInject { get; } int AttributesCount { get; } bool ContainsAttribute(string name); - void AddAttribute(string name, string value, int nextAttributeIndex); - - void AddAttributeToOriginalRequest(string name, string value); + void AddAttribute(string name, string value, int attributeIndex); } diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextAdapter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextAdapter.cs index dd40d44545..ea04aa4065 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextAdapter.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextAdapter.cs @@ -30,10 +30,7 @@ public SnsRequestContextAdapter(IRequestContext context) this.originalRequest = context.OriginalRequest as PublishRequest; } - public bool HasMessageBody => - this.parameters?.ContainsKey("Message") ?? false; - - public bool HasOriginalRequest => this.originalRequest != null; + public bool CanInject => this.originalRequest != null; public int AttributesCount => this.originalRequest?.MessageAttributes.Count ?? 0; @@ -45,14 +42,15 @@ public void AddAttribute(string name, string value, int nextAttributeIndex) return; } - var attributePrefix = "MessageAttributes.entry." + nextAttributeIndex; - this.parameters.Add(attributePrefix + ".Name", name); - this.parameters.Add(attributePrefix + ".Value.DataType", "String"); - this.parameters.Add(attributePrefix + ".Value.StringValue", value); - } + var prefix = "MessageAttributes.entry." + nextAttributeIndex; + this.parameters.Add(prefix + ".Name", name); + this.parameters.Add(prefix + ".Value.DataType", "String"); + this.parameters.Add(prefix + ".Value.StringValue", value); - public void AddAttributeToOriginalRequest(string name, string value) => + // Add injected attributes to the original request as well. + // This dictionary must be in sync with parameters collection to pass through the MD5 hash matching check. this.originalRequest?.MessageAttributes.Add(name, new MessageAttributeValue { DataType = "String", StringValue = value }); + } public bool ContainsAttribute(string name) => this.originalRequest?.MessageAttributes.ContainsKey(name) ?? false; diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs index 42f90cab18..f93a7fa6dc 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs @@ -30,25 +30,26 @@ public SqsRequestContextAdapter(IRequestContext context) this.originalRequest = context.OriginalRequest as SendMessageRequest; } - public bool HasMessageBody => - this.parameters?.ContainsKey("MessageBody") ?? false; - - public bool HasOriginalRequest => this.originalRequest != null; + public bool CanInject => this.originalRequest != null; public int AttributesCount => this.originalRequest?.MessageAttributes.Count ?? 0; - public void AddAttribute(string name, string value, int nextAttributeIndex) + public void AddAttribute(string name, string value, int attributeIndex) { if (this.parameters == null) { return; } - var attributePrefix = "MessageAttribute." + nextAttributeIndex; - this.parameters.Add(attributePrefix + ".Name", name); - this.parameters.Add(attributePrefix + ".Value.DataType", "String"); - this.parameters.Add(attributePrefix + ".Value.StringValue", value); + var prefix = "MessageAttribute." + attributeIndex; + this.parameters.Add(prefix + ".Name", name); + this.parameters.Add(prefix + ".Value.DataType", "String"); + this.parameters.Add(prefix + ".Value.StringValue", value); + + // Add injected attributes to the original request as well. + // This dictionary must be in sync with parameters collection to pass through the MD5 hash matching check. + this.originalRequest?.MessageAttributes.Add(name, new MessageAttributeValue { DataType = "String", StringValue = value }); } public void AddAttributeToOriginalRequest(string name, string value) => diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs index 9ba1f1561f..0be2c14b32 100644 --- a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs @@ -33,19 +33,14 @@ internal class AWSMessagingUtils internal static PropagationContext ExtractParentContext(SQSEvent sqsEvent) { - if (sqsEvent == null) - { - return default; - } - // We assume there can be only one parent that's why we consider only a single (the last) record as the carrier. - var message = sqsEvent.Records.LastOrDefault(); + var message = sqsEvent?.Records?.LastOrDefault(); return ExtractParentContext(message); } internal static PropagationContext ExtractParentContext(SQSEvent.SQSMessage sqsMessage) { - if (sqsMessage == null) + if (sqsMessage?.MessageAttributes == null) { return default; } @@ -67,52 +62,45 @@ internal static PropagationContext ExtractParentContext(SQSEvent.SQSMessage sqsM internal static PropagationContext ExtractParentContext(SNSEvent snsEvent) { - if (snsEvent == null) - { - return default; - } - // We assume there can be only one parent that's why we consider only a single (the last) record as the carrier. - var record = snsEvent.Records.LastOrDefault(); + var record = snsEvent?.Records?.LastOrDefault(); return ExtractParentContext(record); } internal static PropagationContext ExtractParentContext(SNSEvent.SNSRecord record) { - return (record?.Sns != null) ? + return (record?.Sns?.MessageAttributes != null) ? Propagators.DefaultTextMapPropagator.Extract(default, record.Sns.MessageAttributes, SnsMessageAttributeGetter) : default; } internal static PropagationContext ExtractParentContext(SNSEvent.SNSMessage message) { - return (message != null) ? + return (message?.MessageAttributes != null) ? Propagators.DefaultTextMapPropagator.Extract(default, message.MessageAttributes, SnsMessageAttributeGetter) : default; } private static IEnumerable SqsMessageAttributeGetter(IDictionary attributes, string attributeName) { - SQSEvent.MessageAttribute attribute = attributes.GetValueByKeyIgnoringCase(attributeName); - if (attribute == null) + if (!attributes.TryGetValue(attributeName, out var attribute)) { return null; } - return attribute.StringValue != null ? + return attribute?.StringValue != null ? new[] { attribute.StringValue } : - attribute.StringListValues; + attribute?.StringListValues; } private static IEnumerable SnsMessageAttributeGetter(IDictionary attributes, string attributeName) { - SNSEvent.MessageAttribute attribute = attributes.GetValueByKeyIgnoringCase(attributeName); - if (attribute == null) + if (!attributes.TryGetValue(attributeName, out var attribute)) { return null; } - switch (attribute.Type) + switch (attribute?.Type) { case SnsAttributeTypeString when attribute.Value != null: return new[] { attribute.Value }; diff --git a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessagingUtilsTests.cs b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessagingUtilsTests.cs index d3eedc0355..48c583346a 100644 --- a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessagingUtilsTests.cs +++ b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessagingUtilsTests.cs @@ -45,7 +45,7 @@ public AWSMessagingUtilsTests() public void Inject_ParametersCollectionSizeReachesLimit_TraceDataNotInjected(string serviceType) { AmazonWebServiceRequest originalRequest = TestsHelper.CreateOriginalRequest(serviceType, 10); - var parameters = TestsHelper.DefaultParameterCollection(serviceType); + var parameters = new ParameterCollection(); parameters.AddStringParameters(serviceType, originalRequest); var request = new Mock(); @@ -62,13 +62,13 @@ public void Inject_ParametersCollectionSizeReachesLimit_TraceDataNotInjected(str AWSMessagingUtils.Inject(adapter, CreatePropagationContext()); - Assert.Equal(31, parameters.Count); + Assert.Equal(30, parameters.Count); } [Theory] [InlineData(AWSServiceType.SQSService)] [InlineData(AWSServiceType.SNSService)] - public void Inject_DefaultParametersCollection_TraceDataInjected(string serviceType) + public void Inject_ParametersCollection_TraceDataInjected(string serviceType) { var expectedParameters = new List>() { @@ -77,7 +77,7 @@ public void Inject_DefaultParametersCollection_TraceDataInjected(string serviceT }; AmazonWebServiceRequest originalRequest = TestsHelper.CreateOriginalRequest(serviceType, 0); - var parameters = TestsHelper.DefaultParameterCollection(serviceType); + var parameters = new ParameterCollection(); var request = new Mock(); request.Setup(x => x.ParameterCollection) @@ -109,7 +109,7 @@ public void Inject_ParametersCollectionWithCustomParameter_TraceDataInjected(str }; AmazonWebServiceRequest originalRequest = TestsHelper.CreateOriginalRequest(serviceType, 1); - var parameters = TestsHelper.DefaultParameterCollection(serviceType); + var parameters = new ParameterCollection(); parameters.AddStringParameters(serviceType, originalRequest); var request = new Mock(); @@ -132,19 +132,21 @@ public void Inject_ParametersCollectionWithCustomParameter_TraceDataInjected(str [Theory] [InlineData(AWSServiceType.SQSService)] [InlineData(AWSServiceType.SNSService)] - public void Inject_ParametersCollectionWithTraceData_TraceDataNotInjected(string serviceType) + public void Inject_ParametersCollectionWithTraceParent_TraceStateNotInjected(string serviceType) { + // This test just checks the common implementation logic: + // if at least one attribute is already present the whole injection is skipped. + // We just use default trace propagator as an example which injects only traceparent and tracestate. + var expectedParameters = new List>() { new KeyValuePair("traceparent", $"00-{TraceId}-{ParentId}-00"), - new KeyValuePair("tracestate", "trace-state"), }; AmazonWebServiceRequest originalRequest = TestsHelper.CreateOriginalRequest(serviceType, 0); originalRequest.AddAttribute("traceparent", $"00-{TraceId}-{ParentId}-00"); - originalRequest.AddAttribute("tracestate", $"trace-state"); - var parameters = TestsHelper.DefaultParameterCollection(serviceType); + var parameters = new ParameterCollection(); parameters.AddStringParameters(serviceType, originalRequest); var request = new Mock(); diff --git a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs index 8a366a0e7d..1c50b8cfa9 100644 --- a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs +++ b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs @@ -16,11 +16,8 @@ using System; using System.Collections.Generic; -using System.Diagnostics; -using System.Drawing; using Amazon.Runtime; using Amazon.Runtime.Internal; -using OpenTelemetry.Context.Propagation; using OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; using Xunit; using SNS = Amazon.SimpleNotificationService.Model; @@ -113,7 +110,7 @@ internal static void AddStringParameters(this ParameterCollection parameters, st internal static void AssertStringParameters(string serviceType, List> expectedParameters, ParameterCollection parameters) { - Assert.Equal((expectedParameters.Count * 3) + 1, parameters.Count); + Assert.Equal(expectedParameters.Count * 3, parameters.Count); for (int i = 0; i < expectedParameters.Count; i++) { @@ -131,14 +128,6 @@ internal static void AssertStringParameters(string serviceType, List throw new NotSupportedException($"Tests for service type {serviceType} not supported."), }; } - - private static string GetMessageBodyAttributeName(string serviceType) - { - return serviceType switch - { - AWSServiceType.SQSService => "MessageBody", - AWSServiceType.SNSService => "Message", - _ => throw new NotSupportedException($"Tests for service type {serviceType} not supported."), - }; - } } From ed1ef5a6306f6e24e3648eae6ee2e475ad5fe574 Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Fri, 3 Mar 2023 10:48:24 +0100 Subject: [PATCH 07/23] #1034: simplified adapter's interface and moved most of the processing code from AWSMessagingUtils to adapter's implementations. --- .../Implementation/AWSMessagingUtils.cs | 33 +---------- .../Implementation/IRequestContextAdapter.cs | 8 +-- .../SnsRequestContextAdapter.cs | 54 +++++++++++++---- .../SqsRequestContextAdapter.cs | 58 ++++++++++++++----- .../Implementation/TestsHelper.cs | 2 +- 5 files changed, 93 insertions(+), 62 deletions(-) diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs index c52693bd20..a9c52f23a1 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs @@ -15,18 +15,12 @@ // using System.Collections.Generic; -using System.Linq; using OpenTelemetry.Context.Propagation; namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; internal static class AWSMessagingUtils { - // SQS/SNS message attributes collection size limit according to - // https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-message-metadata.html and - // https://docs.aws.amazon.com/sns/latest/dg/sns-message-attributes.html - private const int MaxMessageAttributes = 10; - internal static void Inject(IRequestContextAdapter request, PropagationContext propagationContext) { if (!request.CanInject) @@ -35,30 +29,7 @@ internal static void Inject(IRequestContextAdapter request, PropagationContext p } var carrier = new Dictionary(); - Propagators.DefaultTextMapPropagator.Inject(propagationContext, carrier, (c, k, v) => c[k] = v); - if (carrier.Keys.Any(k => request.ContainsAttribute(k))) - { - // If at least one attribute is already present in the request then we skip the injection. - return; - } - - int attributesCount = request.AttributesCount; - if (carrier.Count + attributesCount > MaxMessageAttributes) - { - // TODO: add logging (event source). - return; - } - - int nextAttributeIndex = attributesCount + 1; - foreach (var param in carrier) - { - if (request.ContainsAttribute(param.Key)) - { - continue; - } - - request.AddAttribute(param.Key, param.Value, nextAttributeIndex); - nextAttributeIndex++; - } + Propagators.DefaultTextMapPropagator.Inject(propagationContext, carrier, (c, k, v) => c[k] = v); + request.AddAttributes(carrier); } } diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IRequestContextAdapter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IRequestContextAdapter.cs index eb4cdfb994..d86cdf0577 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IRequestContextAdapter.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IRequestContextAdapter.cs @@ -14,14 +14,12 @@ // limitations under the License. // +using System.Collections.Generic; + namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; internal interface IRequestContextAdapter { bool CanInject { get; } - int AttributesCount { get; } - - bool ContainsAttribute(string name); - - void AddAttribute(string name, string value, int attributeIndex); + void AddAttributes(IReadOnlyDictionary attributes); } diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextAdapter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextAdapter.cs index ea04aa4065..8b0a679451 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextAdapter.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextAdapter.cs @@ -14,6 +14,8 @@ // limitations under the License. // +using System.Collections.Generic; +using System.Linq; using Amazon.Runtime; using Amazon.Runtime.Internal; using Amazon.SimpleNotificationService.Model; @@ -21,8 +23,13 @@ namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; internal class SnsRequestContextAdapter : IRequestContextAdapter { - private readonly ParameterCollection parameters; - private readonly PublishRequest originalRequest; + // SQS/SNS message attributes collection size limit according to + // https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-message-metadata.html and + // https://docs.aws.amazon.com/sns/latest/dg/sns-message-attributes.html + private const int MaxMessageAttributes = 10; + + private readonly ParameterCollection? parameters; + private readonly PublishRequest? originalRequest; public SnsRequestContextAdapter(IRequestContext context) { @@ -30,28 +37,53 @@ public SnsRequestContextAdapter(IRequestContext context) this.originalRequest = context.OriginalRequest as PublishRequest; } - public bool CanInject => this.originalRequest != null; + public bool CanInject => this.originalRequest?.MessageAttributes != null && this.parameters != null; - public int AttributesCount => - this.originalRequest?.MessageAttributes.Count ?? 0; + public void AddAttributes(IReadOnlyDictionary attributes) + { + if (!this.CanInject) + { + return; + } + + if (attributes.Keys.Any(k => this.ContainsAttribute(k))) + { + // If at least one attribute is already present in the request then we skip the injection. + return; + } + + int attributesCount = this.originalRequest?.MessageAttributes.Count ?? 0; + if (attributes.Count + attributesCount > MaxMessageAttributes) + { + // TODO: add logging (event source). + return; + } + + int nextAttributeIndex = attributesCount + 1; + foreach (var param in attributes) + { + this.AddAttribute(param.Key, param.Value, nextAttributeIndex); + nextAttributeIndex++; + } + } - public void AddAttribute(string name, string value, int nextAttributeIndex) + private void AddAttribute(string name, string value, int nextAttributeIndex) { - if (this.parameters == null) + if (!this.CanInject) { return; } var prefix = "MessageAttributes.entry." + nextAttributeIndex; - this.parameters.Add(prefix + ".Name", name); - this.parameters.Add(prefix + ".Value.DataType", "String"); - this.parameters.Add(prefix + ".Value.StringValue", value); + this.parameters?.Add(prefix + ".Name", name); + this.parameters?.Add(prefix + ".Value.DataType", "String"); + this.parameters?.Add(prefix + ".Value.StringValue", value); // Add injected attributes to the original request as well. // This dictionary must be in sync with parameters collection to pass through the MD5 hash matching check. this.originalRequest?.MessageAttributes.Add(name, new MessageAttributeValue { DataType = "String", StringValue = value }); } - public bool ContainsAttribute(string name) + private bool ContainsAttribute(string name) => this.originalRequest?.MessageAttributes.ContainsKey(name) ?? false; } diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs index f93a7fa6dc..af5e37e4aa 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs @@ -14,6 +14,8 @@ // limitations under the License. // +using System.Collections.Generic; +using System.Linq; using Amazon.Runtime; using Amazon.Runtime.Internal; using Amazon.SQS.Model; @@ -21,8 +23,13 @@ namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; internal class SqsRequestContextAdapter : IRequestContextAdapter { - private readonly ParameterCollection parameters; - private readonly SendMessageRequest originalRequest; + // SQS/SNS message attributes collection size limit according to + // https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-message-metadata.html and + // https://docs.aws.amazon.com/sns/latest/dg/sns-message-attributes.html + private const int MaxMessageAttributes = 10; + + private readonly ParameterCollection? parameters; + private readonly SendMessageRequest? originalRequest; public SqsRequestContextAdapter(IRequestContext context) { @@ -30,31 +37,54 @@ public SqsRequestContextAdapter(IRequestContext context) this.originalRequest = context.OriginalRequest as SendMessageRequest; } - public bool CanInject => this.originalRequest != null; + public bool CanInject => this.originalRequest?.MessageAttributes != null && this.parameters != null; + + public void AddAttributes(IReadOnlyDictionary attributes) + { + if (!this.CanInject) + { + return; + } + + if (attributes.Keys.Any(k => this.ContainsAttribute(k))) + { + // If at least one attribute is already present in the request then we skip the injection. + return; + } + + int attributesCount = this.originalRequest?.MessageAttributes.Count ?? 0; + if (attributes.Count + attributesCount > MaxMessageAttributes) + { + // TODO: add logging (event source). + return; + } + + int nextAttributeIndex = attributesCount + 1; + foreach (var param in attributes) + { + this.AddAttribute(param.Key, param.Value, nextAttributeIndex); + nextAttributeIndex++; + } + } - public int AttributesCount => - this.originalRequest?.MessageAttributes.Count ?? 0; - public void AddAttribute(string name, string value, int attributeIndex) + private void AddAttribute(string name, string value, int attributeIndex) { - if (this.parameters == null) + if (!this.CanInject) { return; } var prefix = "MessageAttribute." + attributeIndex; - this.parameters.Add(prefix + ".Name", name); - this.parameters.Add(prefix + ".Value.DataType", "String"); - this.parameters.Add(prefix + ".Value.StringValue", value); + this.parameters?.Add(prefix + ".Name", name); + this.parameters?.Add(prefix + ".Value.DataType", "String"); + this.parameters?.Add(prefix + ".Value.StringValue", value); // Add injected attributes to the original request as well. // This dictionary must be in sync with parameters collection to pass through the MD5 hash matching check. this.originalRequest?.MessageAttributes.Add(name, new MessageAttributeValue { DataType = "String", StringValue = value }); } - public void AddAttributeToOriginalRequest(string name, string value) => - this.originalRequest?.MessageAttributes.Add(name, new MessageAttributeValue { DataType = "String", StringValue = value }); - - public bool ContainsAttribute(string name) => + private bool ContainsAttribute(string name) => this.originalRequest?.MessageAttributes.ContainsKey(name) ?? false; } diff --git a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs index 1c50b8cfa9..9ec89587c2 100644 --- a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs +++ b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs @@ -115,7 +115,7 @@ internal static void AssertStringParameters(string serviceType, List (p as StringParameterValue).Value; + static string? Value(ParameterValue p) => (p as StringParameterValue)?.Value; Assert.True(parameters.ContainsKey($"{prefix}.Name")); Assert.Equal(expectedParameters[i].Key, Value(parameters[$"{prefix}.Name"])); From 2061808c02dbf7ee4d1c01c2b8c98bf30ab50c54 Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Fri, 3 Mar 2023 10:54:43 +0100 Subject: [PATCH 08/23] #1034: fixed trailing whitespaces --- .../Implementation/AWSMessagingUtils.cs | 2 +- .../Implementation/SqsRequestContextAdapter.cs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs index a9c52f23a1..8b1e57f852 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs @@ -29,7 +29,7 @@ internal static void Inject(IRequestContextAdapter request, PropagationContext p } var carrier = new Dictionary(); - Propagators.DefaultTextMapPropagator.Inject(propagationContext, carrier, (c, k, v) => c[k] = v); + Propagators.DefaultTextMapPropagator.Inject(propagationContext, carrier, (c, k, v) => c[k] = v); request.AddAttributes(carrier); } } diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs index af5e37e4aa..9a7be2ddb2 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs @@ -67,7 +67,6 @@ public void AddAttributes(IReadOnlyDictionary attributes) } } - private void AddAttribute(string name, string value, int attributeIndex) { if (!this.CanInject) From c4265609d5efb08b3aa25f2cef146072229755be Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Fri, 3 Mar 2023 15:25:57 +0100 Subject: [PATCH 09/23] #1034: further improvements - got rid of IRequestContextAdapter interface and replaced it by Action - returned direct ref. to AWSSDK.Core 3.5.1.24 and downgraded AWSSDK.SimpleNotificationService and AWSSDK.SQS to be compatible with the AWSSDK.Core --- .../Implementation/AWSMessagingUtils.cs | 7 +-- .../AWSTracingPipelineHandler.cs | 4 +- .../Implementation/IRequestContextAdapter.cs | 25 ---------- .../SnsRequestContextAdapter.cs | 46 ++++++++++--------- .../SqsRequestContextAdapter.cs | 46 ++++++++++--------- ...lemetry.Contrib.Instrumentation.AWS.csproj | 5 +- .../Implementation/AWSMessagingUtilsTests.cs | 16 +++---- .../Implementation/TestsHelper.cs | 6 +-- ...y.Contrib.Instrumentation.AWS.Tests.csproj | 3 +- 9 files changed, 69 insertions(+), 89 deletions(-) delete mode 100644 src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IRequestContextAdapter.cs diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs index 8b1e57f852..6d08bf2643 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs @@ -14,6 +14,7 @@ // limitations under the License. // +using System; using System.Collections.Generic; using OpenTelemetry.Context.Propagation; @@ -21,15 +22,15 @@ namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; internal static class AWSMessagingUtils { - internal static void Inject(IRequestContextAdapter request, PropagationContext propagationContext) + internal static void Inject(Action>? addAttributes, PropagationContext propagationContext) { - if (!request.CanInject) + if (addAttributes == null) { return; } var carrier = new Dictionary(); Propagators.DefaultTextMapPropagator.Inject(propagationContext, carrier, (c, k, v) => c[k] = v); - request.AddAttributes(carrier); + addAttributes(carrier); } } diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs index d5248efe59..040e952949 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs @@ -202,11 +202,11 @@ private void AddRequestSpecificInformation(Activity activity, IRequestContext re } else if (AWSServiceType.IsSqsService(service)) { - AWSMessagingUtils.Inject(new SqsRequestContextAdapter(requestContext), new PropagationContext(activity.Context, Baggage.Current)); + AWSMessagingUtils.Inject(SqsRequestContextAdapter.CreateAddAttributesAction(requestContext), new PropagationContext(activity.Context, Baggage.Current)); } else if (AWSServiceType.IsSnsService(service)) { - AWSMessagingUtils.Inject(new SnsRequestContextAdapter(requestContext), new PropagationContext(activity.Context, Baggage.Current)); + AWSMessagingUtils.Inject(SnsRequestContextAdapter.CreateAddAttributesAction(requestContext), new PropagationContext(activity.Context, Baggage.Current)); } } diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IRequestContextAdapter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IRequestContextAdapter.cs deleted file mode 100644 index d86cdf0577..0000000000 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/IRequestContextAdapter.cs +++ /dev/null @@ -1,25 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -using System.Collections.Generic; - -namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; -internal interface IRequestContextAdapter -{ - bool CanInject { get; } - - void AddAttributes(IReadOnlyDictionary attributes); -} diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextAdapter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextAdapter.cs index 8b0a679451..cddb830340 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextAdapter.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextAdapter.cs @@ -14,6 +14,7 @@ // limitations under the License. // +using System; using System.Collections.Generic; using System.Linq; using Amazon.Runtime; @@ -21,38 +22,31 @@ using Amazon.SimpleNotificationService.Model; namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; -internal class SnsRequestContextAdapter : IRequestContextAdapter +internal class SnsRequestContextAdapter { // SQS/SNS message attributes collection size limit according to // https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-message-metadata.html and // https://docs.aws.amazon.com/sns/latest/dg/sns-message-attributes.html private const int MaxMessageAttributes = 10; - private readonly ParameterCollection? parameters; - private readonly PublishRequest? originalRequest; + private readonly ParameterCollection parameters; + private readonly PublishRequest originalRequest; - public SnsRequestContextAdapter(IRequestContext context) + private SnsRequestContextAdapter(ParameterCollection parameters, PublishRequest originalRequest) { - this.parameters = context.Request?.ParameterCollection; - this.originalRequest = context.OriginalRequest as PublishRequest; + this.parameters = parameters; + this.originalRequest = originalRequest; } - public bool CanInject => this.originalRequest?.MessageAttributes != null && this.parameters != null; - public void AddAttributes(IReadOnlyDictionary attributes) { - if (!this.CanInject) - { - return; - } - if (attributes.Keys.Any(k => this.ContainsAttribute(k))) { // If at least one attribute is already present in the request then we skip the injection. return; } - int attributesCount = this.originalRequest?.MessageAttributes.Count ?? 0; + int attributesCount = this.originalRequest.MessageAttributes.Count; if (attributes.Count + attributesCount > MaxMessageAttributes) { // TODO: add logging (event source). @@ -67,23 +61,31 @@ public void AddAttributes(IReadOnlyDictionary attributes) } } - private void AddAttribute(string name, string value, int nextAttributeIndex) + internal static Action>? CreateAddAttributesAction(IRequestContext context) { - if (!this.CanInject) + var parameters = context.Request?.ParameterCollection; + var originalRequest = context.OriginalRequest as PublishRequest; + if (originalRequest?.MessageAttributes == null || parameters == null) { - return; + return null; } + var request = new SnsRequestContextAdapter(parameters, originalRequest); + return request.AddAttributes; + } + + private void AddAttribute(string name, string value, int nextAttributeIndex) + { var prefix = "MessageAttributes.entry." + nextAttributeIndex; - this.parameters?.Add(prefix + ".Name", name); - this.parameters?.Add(prefix + ".Value.DataType", "String"); - this.parameters?.Add(prefix + ".Value.StringValue", value); + this.parameters.Add(prefix + ".Name", name); + this.parameters.Add(prefix + ".Value.DataType", "String"); + this.parameters.Add(prefix + ".Value.StringValue", value); // Add injected attributes to the original request as well. // This dictionary must be in sync with parameters collection to pass through the MD5 hash matching check. - this.originalRequest?.MessageAttributes.Add(name, new MessageAttributeValue { DataType = "String", StringValue = value }); + this.originalRequest.MessageAttributes.Add(name, new MessageAttributeValue { DataType = "String", StringValue = value }); } private bool ContainsAttribute(string name) - => this.originalRequest?.MessageAttributes.ContainsKey(name) ?? false; + => this.originalRequest.MessageAttributes.ContainsKey(name); } diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs index 9a7be2ddb2..574160fa48 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs @@ -14,6 +14,7 @@ // limitations under the License. // +using System; using System.Collections.Generic; using System.Linq; using Amazon.Runtime; @@ -21,38 +22,31 @@ using Amazon.SQS.Model; namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; -internal class SqsRequestContextAdapter : IRequestContextAdapter +internal class SqsRequestContextAdapter { // SQS/SNS message attributes collection size limit according to // https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-message-metadata.html and // https://docs.aws.amazon.com/sns/latest/dg/sns-message-attributes.html private const int MaxMessageAttributes = 10; - private readonly ParameterCollection? parameters; - private readonly SendMessageRequest? originalRequest; + private readonly ParameterCollection parameters; + private readonly SendMessageRequest originalRequest; - public SqsRequestContextAdapter(IRequestContext context) + private SqsRequestContextAdapter(ParameterCollection parameters, SendMessageRequest originalRequest) { - this.parameters = context.Request?.ParameterCollection; - this.originalRequest = context.OriginalRequest as SendMessageRequest; + this.parameters = parameters; + this.originalRequest = originalRequest; } - public bool CanInject => this.originalRequest?.MessageAttributes != null && this.parameters != null; - public void AddAttributes(IReadOnlyDictionary attributes) { - if (!this.CanInject) - { - return; - } - if (attributes.Keys.Any(k => this.ContainsAttribute(k))) { // If at least one attribute is already present in the request then we skip the injection. return; } - int attributesCount = this.originalRequest?.MessageAttributes.Count ?? 0; + int attributesCount = this.originalRequest.MessageAttributes.Count; if (attributes.Count + attributesCount > MaxMessageAttributes) { // TODO: add logging (event source). @@ -67,23 +61,31 @@ public void AddAttributes(IReadOnlyDictionary attributes) } } - private void AddAttribute(string name, string value, int attributeIndex) + internal static Action>? CreateAddAttributesAction(IRequestContext context) { - if (!this.CanInject) + var parameters = context.Request?.ParameterCollection; + var originalRequest = context.OriginalRequest as SendMessageRequest; + if (originalRequest?.MessageAttributes == null || parameters == null) { - return; + return null; } + var request = new SqsRequestContextAdapter(parameters, originalRequest); + return request.AddAttributes; + } + + private void AddAttribute(string name, string value, int attributeIndex) + { var prefix = "MessageAttribute." + attributeIndex; - this.parameters?.Add(prefix + ".Name", name); - this.parameters?.Add(prefix + ".Value.DataType", "String"); - this.parameters?.Add(prefix + ".Value.StringValue", value); + this.parameters.Add(prefix + ".Name", name); + this.parameters.Add(prefix + ".Value.DataType", "String"); + this.parameters.Add(prefix + ".Value.StringValue", value); // Add injected attributes to the original request as well. // This dictionary must be in sync with parameters collection to pass through the MD5 hash matching check. - this.originalRequest?.MessageAttributes.Add(name, new MessageAttributeValue { DataType = "String", StringValue = value }); + this.originalRequest.MessageAttributes.Add(name, new MessageAttributeValue { DataType = "String", StringValue = value }); } private bool ContainsAttribute(string name) => - this.originalRequest?.MessageAttributes.ContainsKey(name) ?? false; + this.originalRequest.MessageAttributes.ContainsKey(name); } diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/OpenTelemetry.Contrib.Instrumentation.AWS.csproj b/src/OpenTelemetry.Contrib.Instrumentation.AWS/OpenTelemetry.Contrib.Instrumentation.AWS.csproj index e15b6c2267..5e8d6a44e2 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/OpenTelemetry.Contrib.Instrumentation.AWS.csproj +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/OpenTelemetry.Contrib.Instrumentation.AWS.csproj @@ -9,8 +9,9 @@ - - + + + diff --git a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessagingUtilsTests.cs b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessagingUtilsTests.cs index 48c583346a..c8adbb97a6 100644 --- a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessagingUtilsTests.cs +++ b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessagingUtilsTests.cs @@ -58,9 +58,9 @@ public void Inject_ParametersCollectionSizeReachesLimit_TraceDataNotInjected(str context.Setup(x => x.Request) .Returns(request.Object); - var adapter = TestsHelper.CreateRequestContextAdapter(serviceType, context.Object); + var addAttributes = TestsHelper.CreateAddAttributesAction(serviceType, context.Object); - AWSMessagingUtils.Inject(adapter, CreatePropagationContext()); + AWSMessagingUtils.Inject(addAttributes, CreatePropagationContext()); Assert.Equal(30, parameters.Count); } @@ -89,9 +89,9 @@ public void Inject_ParametersCollection_TraceDataInjected(string serviceType) context.Setup(x => x.Request) .Returns(request.Object); - var adapter = TestsHelper.CreateRequestContextAdapter(serviceType, context.Object); + var addAttributes = TestsHelper.CreateAddAttributesAction(serviceType, context.Object); - AWSMessagingUtils.Inject(adapter, CreatePropagationContext()); + AWSMessagingUtils.Inject(addAttributes, CreatePropagationContext()); TestsHelper.AssertStringParameters(serviceType, expectedParameters, parameters); } @@ -122,9 +122,9 @@ public void Inject_ParametersCollectionWithCustomParameter_TraceDataInjected(str context.Setup(x => x.Request) .Returns(request.Object); - var adapter = TestsHelper.CreateRequestContextAdapter(serviceType, context.Object); + var addAttributes = TestsHelper.CreateAddAttributesAction(serviceType, context.Object); - AWSMessagingUtils.Inject(adapter, CreatePropagationContext()); + AWSMessagingUtils.Inject(addAttributes, CreatePropagationContext()); TestsHelper.AssertStringParameters(serviceType, expectedParameters, parameters); } @@ -159,9 +159,9 @@ public void Inject_ParametersCollectionWithTraceParent_TraceStateNotInjected(str context.Setup(x => x.Request) .Returns(request.Object); - var adapter = TestsHelper.CreateRequestContextAdapter(serviceType, context.Object); + var addAttributes = TestsHelper.CreateAddAttributesAction(serviceType, context.Object); - AWSMessagingUtils.Inject(adapter, CreatePropagationContext()); + AWSMessagingUtils.Inject(addAttributes, CreatePropagationContext()); TestsHelper.AssertStringParameters(serviceType, expectedParameters, parameters); } diff --git a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs index 9ec89587c2..c55ec74ad9 100644 --- a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs +++ b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs @@ -26,12 +26,12 @@ namespace OpenTelemetry.Contrib.Instrumentation.AWS.Tests.Implementation; internal static class TestsHelper { - internal static IRequestContextAdapter CreateRequestContextAdapter(string serviceType, IRequestContext context) + internal static Action>? CreateAddAttributesAction(string serviceType, IRequestContext context) { return serviceType switch { - AWSServiceType.SQSService => new SqsRequestContextAdapter(context), - AWSServiceType.SNSService => new SnsRequestContextAdapter(context), + AWSServiceType.SQSService => SqsRequestContextAdapter.CreateAddAttributesAction(context), + AWSServiceType.SNSService => SnsRequestContextAdapter.CreateAddAttributesAction(context), _ => throw new NotSupportedException($"Tests for service type {serviceType} not supported."), }; } diff --git a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/OpenTelemetry.Contrib.Instrumentation.AWS.Tests.csproj b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/OpenTelemetry.Contrib.Instrumentation.AWS.Tests.csproj index c26ccb0e7f..ea815d5d1a 100644 --- a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/OpenTelemetry.Contrib.Instrumentation.AWS.Tests.csproj +++ b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/OpenTelemetry.Contrib.Instrumentation.AWS.Tests.csproj @@ -9,10 +9,9 @@ - - + all From 165a0d15f07b1e59f21354c7d591bfe6794c5546 Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Fri, 3 Mar 2023 15:54:02 +0100 Subject: [PATCH 10/23] #1034: further code simplification --- .../Implementation/AWSMessagingUtils.cs | 10 ++-------- .../AWSTracingPipelineHandler.cs | 6 ++++-- .../Implementation/AWSMessagingUtilsTests.cs | 20 ++++++++----------- 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs index 6d08bf2643..d723743c3a 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSMessagingUtils.cs @@ -14,7 +14,6 @@ // limitations under the License. // -using System; using System.Collections.Generic; using OpenTelemetry.Context.Propagation; @@ -22,15 +21,10 @@ namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; internal static class AWSMessagingUtils { - internal static void Inject(Action>? addAttributes, PropagationContext propagationContext) + internal static IReadOnlyDictionary InjectIntoDictionary(PropagationContext propagationContext) { - if (addAttributes == null) - { - return; - } - var carrier = new Dictionary(); Propagators.DefaultTextMapPropagator.Inject(propagationContext, carrier, (c, k, v) => c[k] = v); - addAttributes(carrier); + return carrier; } } diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs index 040e952949..1cac4490f7 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs @@ -202,11 +202,13 @@ private void AddRequestSpecificInformation(Activity activity, IRequestContext re } else if (AWSServiceType.IsSqsService(service)) { - AWSMessagingUtils.Inject(SqsRequestContextAdapter.CreateAddAttributesAction(requestContext), new PropagationContext(activity.Context, Baggage.Current)); + var addAttributes = SqsRequestContextAdapter.CreateAddAttributesAction(requestContext); + addAttributes?.Invoke(AWSMessagingUtils.InjectIntoDictionary(new PropagationContext(activity.Context, Baggage.Current))); } else if (AWSServiceType.IsSnsService(service)) { - AWSMessagingUtils.Inject(SnsRequestContextAdapter.CreateAddAttributesAction(requestContext), new PropagationContext(activity.Context, Baggage.Current)); + var addAttributes = SnsRequestContextAdapter.CreateAddAttributesAction(requestContext); + addAttributes?.Invoke(AWSMessagingUtils.InjectIntoDictionary(new PropagationContext(activity.Context, Baggage.Current))); } } diff --git a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessagingUtilsTests.cs b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessagingUtilsTests.cs index c8adbb97a6..894a0c5fe8 100644 --- a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessagingUtilsTests.cs +++ b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessagingUtilsTests.cs @@ -42,7 +42,7 @@ public AWSMessagingUtilsTests() [Theory] [InlineData(AWSServiceType.SQSService)] [InlineData(AWSServiceType.SNSService)] - public void Inject_ParametersCollectionSizeReachesLimit_TraceDataNotInjected(string serviceType) + public void InjectIntoDictionary_ParametersCollectionSizeReachesLimit_TraceDataNotInjected(string serviceType) { AmazonWebServiceRequest originalRequest = TestsHelper.CreateOriginalRequest(serviceType, 10); var parameters = new ParameterCollection(); @@ -59,8 +59,7 @@ public void Inject_ParametersCollectionSizeReachesLimit_TraceDataNotInjected(str .Returns(request.Object); var addAttributes = TestsHelper.CreateAddAttributesAction(serviceType, context.Object); - - AWSMessagingUtils.Inject(addAttributes, CreatePropagationContext()); + addAttributes?.Invoke(AWSMessagingUtils.InjectIntoDictionary(CreatePropagationContext())); Assert.Equal(30, parameters.Count); } @@ -68,7 +67,7 @@ public void Inject_ParametersCollectionSizeReachesLimit_TraceDataNotInjected(str [Theory] [InlineData(AWSServiceType.SQSService)] [InlineData(AWSServiceType.SNSService)] - public void Inject_ParametersCollection_TraceDataInjected(string serviceType) + public void InjectIntoDictionary_ParametersCollection_TraceDataInjected(string serviceType) { var expectedParameters = new List>() { @@ -90,8 +89,7 @@ public void Inject_ParametersCollection_TraceDataInjected(string serviceType) .Returns(request.Object); var addAttributes = TestsHelper.CreateAddAttributesAction(serviceType, context.Object); - - AWSMessagingUtils.Inject(addAttributes, CreatePropagationContext()); + addAttributes?.Invoke(AWSMessagingUtils.InjectIntoDictionary(CreatePropagationContext())); TestsHelper.AssertStringParameters(serviceType, expectedParameters, parameters); } @@ -99,7 +97,7 @@ public void Inject_ParametersCollection_TraceDataInjected(string serviceType) [Theory] [InlineData(AWSServiceType.SQSService)] [InlineData(AWSServiceType.SNSService)] - public void Inject_ParametersCollectionWithCustomParameter_TraceDataInjected(string serviceType) + public void InjectIntoDictionary_ParametersCollectionWithCustomParameter_TraceDataInjected(string serviceType) { var expectedParameters = new List>() { @@ -123,8 +121,7 @@ public void Inject_ParametersCollectionWithCustomParameter_TraceDataInjected(str .Returns(request.Object); var addAttributes = TestsHelper.CreateAddAttributesAction(serviceType, context.Object); - - AWSMessagingUtils.Inject(addAttributes, CreatePropagationContext()); + addAttributes?.Invoke(AWSMessagingUtils.InjectIntoDictionary(CreatePropagationContext())); TestsHelper.AssertStringParameters(serviceType, expectedParameters, parameters); } @@ -132,7 +129,7 @@ public void Inject_ParametersCollectionWithCustomParameter_TraceDataInjected(str [Theory] [InlineData(AWSServiceType.SQSService)] [InlineData(AWSServiceType.SNSService)] - public void Inject_ParametersCollectionWithTraceParent_TraceStateNotInjected(string serviceType) + public void InjectIntoDictionary_ParametersCollectionWithTraceParent_TraceStateNotInjected(string serviceType) { // This test just checks the common implementation logic: // if at least one attribute is already present the whole injection is skipped. @@ -160,8 +157,7 @@ public void Inject_ParametersCollectionWithTraceParent_TraceStateNotInjected(str .Returns(request.Object); var addAttributes = TestsHelper.CreateAddAttributesAction(serviceType, context.Object); - - AWSMessagingUtils.Inject(addAttributes, CreatePropagationContext()); + addAttributes?.Invoke(AWSMessagingUtils.InjectIntoDictionary(CreatePropagationContext())); TestsHelper.AssertStringParameters(serviceType, expectedParameters, parameters); } From 9f8d96a969bb1541b0803f8b5cc81b8582d2feb0 Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Fri, 3 Mar 2023 16:37:17 +0100 Subject: [PATCH 11/23] #1034: further code simplifications --- .../AWSTracingPipelineHandler.cs | 8 +-- ...tAdapter.cs => SnsRequestContextHelper.cs} | 55 ++++++------------- ...tAdapter.cs => SqsRequestContextHelper.cs} | 53 ++++++------------ ...sTests.cs => RequestContextHelperTests.cs} | 22 ++++---- .../Implementation/TestsHelper.cs | 6 +- 5 files changed, 53 insertions(+), 91 deletions(-) rename src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/{SnsRequestContextAdapter.cs => SnsRequestContextHelper.cs} (56%) rename src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/{SqsRequestContextAdapter.cs => SqsRequestContextHelper.cs} (57%) rename test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/{AWSMessagingUtilsTests.cs => RequestContextHelperTests.cs} (85%) diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs index 1cac4490f7..8bbb6f3435 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/AWSTracingPipelineHandler.cs @@ -202,13 +202,13 @@ private void AddRequestSpecificInformation(Activity activity, IRequestContext re } else if (AWSServiceType.IsSqsService(service)) { - var addAttributes = SqsRequestContextAdapter.CreateAddAttributesAction(requestContext); - addAttributes?.Invoke(AWSMessagingUtils.InjectIntoDictionary(new PropagationContext(activity.Context, Baggage.Current))); + SqsRequestContextHelper.AddAttributes( + requestContext, AWSMessagingUtils.InjectIntoDictionary(new PropagationContext(activity.Context, Baggage.Current))); } else if (AWSServiceType.IsSnsService(service)) { - var addAttributes = SnsRequestContextAdapter.CreateAddAttributesAction(requestContext); - addAttributes?.Invoke(AWSMessagingUtils.InjectIntoDictionary(new PropagationContext(activity.Context, Baggage.Current))); + SnsRequestContextHelper.AddAttributes( + requestContext, AWSMessagingUtils.InjectIntoDictionary(new PropagationContext(activity.Context, Baggage.Current))); } } diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextAdapter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextHelper.cs similarity index 56% rename from src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextAdapter.cs rename to src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextHelper.cs index cddb830340..5b8b09da87 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextAdapter.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SnsRequestContextHelper.cs @@ -1,4 +1,4 @@ -// +// // Copyright The OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -14,7 +14,6 @@ // limitations under the License. // -using System; using System.Collections.Generic; using System.Linq; using Amazon.Runtime; @@ -22,31 +21,29 @@ using Amazon.SimpleNotificationService.Model; namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; -internal class SnsRequestContextAdapter +internal class SnsRequestContextHelper { // SQS/SNS message attributes collection size limit according to // https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-message-metadata.html and // https://docs.aws.amazon.com/sns/latest/dg/sns-message-attributes.html private const int MaxMessageAttributes = 10; - private readonly ParameterCollection parameters; - private readonly PublishRequest originalRequest; - - private SnsRequestContextAdapter(ParameterCollection parameters, PublishRequest originalRequest) + internal static void AddAttributes(IRequestContext context, IReadOnlyDictionary attributes) { - this.parameters = parameters; - this.originalRequest = originalRequest; - } + var parameters = context.Request?.ParameterCollection; + var originalRequest = context.OriginalRequest as PublishRequest; + if (originalRequest?.MessageAttributes == null || parameters == null) + { + return; + } - public void AddAttributes(IReadOnlyDictionary attributes) - { - if (attributes.Keys.Any(k => this.ContainsAttribute(k))) + if (attributes.Keys.Any(k => originalRequest.MessageAttributes.ContainsKey(k))) { // If at least one attribute is already present in the request then we skip the injection. return; } - int attributesCount = this.originalRequest.MessageAttributes.Count; + int attributesCount = originalRequest.MessageAttributes.Count; if (attributes.Count + attributesCount > MaxMessageAttributes) { // TODO: add logging (event source). @@ -56,36 +53,20 @@ public void AddAttributes(IReadOnlyDictionary attributes) int nextAttributeIndex = attributesCount + 1; foreach (var param in attributes) { - this.AddAttribute(param.Key, param.Value, nextAttributeIndex); + AddAttribute(parameters, originalRequest, param.Key, param.Value, nextAttributeIndex); nextAttributeIndex++; } } - internal static Action>? CreateAddAttributesAction(IRequestContext context) + private static void AddAttribute(ParameterCollection parameters, PublishRequest originalRequest, string name, string value, int attributeIndex) { - var parameters = context.Request?.ParameterCollection; - var originalRequest = context.OriginalRequest as PublishRequest; - if (originalRequest?.MessageAttributes == null || parameters == null) - { - return null; - } - - var request = new SnsRequestContextAdapter(parameters, originalRequest); - return request.AddAttributes; - } - - private void AddAttribute(string name, string value, int nextAttributeIndex) - { - var prefix = "MessageAttributes.entry." + nextAttributeIndex; - this.parameters.Add(prefix + ".Name", name); - this.parameters.Add(prefix + ".Value.DataType", "String"); - this.parameters.Add(prefix + ".Value.StringValue", value); + var prefix = "MessageAttributes.entry." + attributeIndex; + parameters.Add(prefix + ".Name", name); + parameters.Add(prefix + ".Value.DataType", "String"); + parameters.Add(prefix + ".Value.StringValue", value); // Add injected attributes to the original request as well. // This dictionary must be in sync with parameters collection to pass through the MD5 hash matching check. - this.originalRequest.MessageAttributes.Add(name, new MessageAttributeValue { DataType = "String", StringValue = value }); + originalRequest.MessageAttributes.Add(name, new MessageAttributeValue { DataType = "String", StringValue = value }); } - - private bool ContainsAttribute(string name) - => this.originalRequest.MessageAttributes.ContainsKey(name); } diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextHelper.cs similarity index 57% rename from src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs rename to src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextHelper.cs index 574160fa48..aad44cd023 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextAdapter.cs +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/Implementation/SqsRequestContextHelper.cs @@ -1,4 +1,4 @@ -// +// // Copyright The OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -14,7 +14,6 @@ // limitations under the License. // -using System; using System.Collections.Generic; using System.Linq; using Amazon.Runtime; @@ -22,31 +21,29 @@ using Amazon.SQS.Model; namespace OpenTelemetry.Contrib.Instrumentation.AWS.Implementation; -internal class SqsRequestContextAdapter +internal class SqsRequestContextHelper { // SQS/SNS message attributes collection size limit according to // https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-message-metadata.html and // https://docs.aws.amazon.com/sns/latest/dg/sns-message-attributes.html private const int MaxMessageAttributes = 10; - private readonly ParameterCollection parameters; - private readonly SendMessageRequest originalRequest; - - private SqsRequestContextAdapter(ParameterCollection parameters, SendMessageRequest originalRequest) + internal static void AddAttributes(IRequestContext context, IReadOnlyDictionary attributes) { - this.parameters = parameters; - this.originalRequest = originalRequest; - } + var parameters = context.Request?.ParameterCollection; + var originalRequest = context.OriginalRequest as SendMessageRequest; + if (originalRequest?.MessageAttributes == null || parameters == null) + { + return; + } - public void AddAttributes(IReadOnlyDictionary attributes) - { - if (attributes.Keys.Any(k => this.ContainsAttribute(k))) + if (attributes.Keys.Any(k => originalRequest.MessageAttributes.ContainsKey(k))) { // If at least one attribute is already present in the request then we skip the injection. return; } - int attributesCount = this.originalRequest.MessageAttributes.Count; + int attributesCount = originalRequest.MessageAttributes.Count; if (attributes.Count + attributesCount > MaxMessageAttributes) { // TODO: add logging (event source). @@ -56,36 +53,20 @@ public void AddAttributes(IReadOnlyDictionary attributes) int nextAttributeIndex = attributesCount + 1; foreach (var param in attributes) { - this.AddAttribute(param.Key, param.Value, nextAttributeIndex); + AddAttribute(parameters, originalRequest, param.Key, param.Value, nextAttributeIndex); nextAttributeIndex++; } } - internal static Action>? CreateAddAttributesAction(IRequestContext context) - { - var parameters = context.Request?.ParameterCollection; - var originalRequest = context.OriginalRequest as SendMessageRequest; - if (originalRequest?.MessageAttributes == null || parameters == null) - { - return null; - } - - var request = new SqsRequestContextAdapter(parameters, originalRequest); - return request.AddAttributes; - } - - private void AddAttribute(string name, string value, int attributeIndex) + private static void AddAttribute(ParameterCollection parameters, SendMessageRequest originalRequest, string name, string value, int attributeIndex) { var prefix = "MessageAttribute." + attributeIndex; - this.parameters.Add(prefix + ".Name", name); - this.parameters.Add(prefix + ".Value.DataType", "String"); - this.parameters.Add(prefix + ".Value.StringValue", value); + parameters.Add(prefix + ".Name", name); + parameters.Add(prefix + ".Value.DataType", "String"); + parameters.Add(prefix + ".Value.StringValue", value); // Add injected attributes to the original request as well. // This dictionary must be in sync with parameters collection to pass through the MD5 hash matching check. - this.originalRequest.MessageAttributes.Add(name, new MessageAttributeValue { DataType = "String", StringValue = value }); + originalRequest.MessageAttributes.Add(name, new MessageAttributeValue { DataType = "String", StringValue = value }); } - - private bool ContainsAttribute(string name) => - this.originalRequest.MessageAttributes.ContainsKey(name); } diff --git a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessagingUtilsTests.cs b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/RequestContextHelperTests.cs similarity index 85% rename from test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessagingUtilsTests.cs rename to test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/RequestContextHelperTests.cs index 894a0c5fe8..41426d2a61 100644 --- a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/AWSMessagingUtilsTests.cs +++ b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/RequestContextHelperTests.cs @@ -1,4 +1,4 @@ -// +// // Copyright The OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -27,13 +27,13 @@ namespace OpenTelemetry.Contrib.Instrumentation.AWS.Tests.Implementation; -public class AWSMessagingUtilsTests +public class RequestContextHelperTests { private const string TraceId = "5759e988bd862e3fe1be46a994272793"; private const string ParentId = "53995c3f42cd8ad8"; private const string TraceState = "trace-state"; - public AWSMessagingUtilsTests() + public RequestContextHelperTests() { Sdk.CreateTracerProviderBuilder() .Build(); @@ -42,7 +42,7 @@ public AWSMessagingUtilsTests() [Theory] [InlineData(AWSServiceType.SQSService)] [InlineData(AWSServiceType.SNSService)] - public void InjectIntoDictionary_ParametersCollectionSizeReachesLimit_TraceDataNotInjected(string serviceType) + public void AddAttributes_ParametersCollectionSizeReachesLimit_TraceDataNotInjected(string serviceType) { AmazonWebServiceRequest originalRequest = TestsHelper.CreateOriginalRequest(serviceType, 10); var parameters = new ParameterCollection(); @@ -59,7 +59,7 @@ public void InjectIntoDictionary_ParametersCollectionSizeReachesLimit_TraceDataN .Returns(request.Object); var addAttributes = TestsHelper.CreateAddAttributesAction(serviceType, context.Object); - addAttributes?.Invoke(AWSMessagingUtils.InjectIntoDictionary(CreatePropagationContext())); + addAttributes?.Invoke(context.Object, AWSMessagingUtils.InjectIntoDictionary(CreatePropagationContext())); Assert.Equal(30, parameters.Count); } @@ -67,7 +67,7 @@ public void InjectIntoDictionary_ParametersCollectionSizeReachesLimit_TraceDataN [Theory] [InlineData(AWSServiceType.SQSService)] [InlineData(AWSServiceType.SNSService)] - public void InjectIntoDictionary_ParametersCollection_TraceDataInjected(string serviceType) + public void AddAttributes_ParametersCollection_TraceDataInjected(string serviceType) { var expectedParameters = new List>() { @@ -89,7 +89,7 @@ public void InjectIntoDictionary_ParametersCollection_TraceDataInjected(string s .Returns(request.Object); var addAttributes = TestsHelper.CreateAddAttributesAction(serviceType, context.Object); - addAttributes?.Invoke(AWSMessagingUtils.InjectIntoDictionary(CreatePropagationContext())); + addAttributes?.Invoke(context.Object, AWSMessagingUtils.InjectIntoDictionary(CreatePropagationContext())); TestsHelper.AssertStringParameters(serviceType, expectedParameters, parameters); } @@ -97,7 +97,7 @@ public void InjectIntoDictionary_ParametersCollection_TraceDataInjected(string s [Theory] [InlineData(AWSServiceType.SQSService)] [InlineData(AWSServiceType.SNSService)] - public void InjectIntoDictionary_ParametersCollectionWithCustomParameter_TraceDataInjected(string serviceType) + public void AddAttributes_ParametersCollectionWithCustomParameter_TraceDataInjected(string serviceType) { var expectedParameters = new List>() { @@ -121,7 +121,7 @@ public void InjectIntoDictionary_ParametersCollectionWithCustomParameter_TraceDa .Returns(request.Object); var addAttributes = TestsHelper.CreateAddAttributesAction(serviceType, context.Object); - addAttributes?.Invoke(AWSMessagingUtils.InjectIntoDictionary(CreatePropagationContext())); + addAttributes?.Invoke(context.Object, AWSMessagingUtils.InjectIntoDictionary(CreatePropagationContext())); TestsHelper.AssertStringParameters(serviceType, expectedParameters, parameters); } @@ -129,7 +129,7 @@ public void InjectIntoDictionary_ParametersCollectionWithCustomParameter_TraceDa [Theory] [InlineData(AWSServiceType.SQSService)] [InlineData(AWSServiceType.SNSService)] - public void InjectIntoDictionary_ParametersCollectionWithTraceParent_TraceStateNotInjected(string serviceType) + public void AddAttributes_ParametersCollectionWithTraceParent_TraceStateNotInjected(string serviceType) { // This test just checks the common implementation logic: // if at least one attribute is already present the whole injection is skipped. @@ -157,7 +157,7 @@ public void InjectIntoDictionary_ParametersCollectionWithTraceParent_TraceStateN .Returns(request.Object); var addAttributes = TestsHelper.CreateAddAttributesAction(serviceType, context.Object); - addAttributes?.Invoke(AWSMessagingUtils.InjectIntoDictionary(CreatePropagationContext())); + addAttributes?.Invoke(context.Object, AWSMessagingUtils.InjectIntoDictionary(CreatePropagationContext())); TestsHelper.AssertStringParameters(serviceType, expectedParameters, parameters); } diff --git a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs index c55ec74ad9..d2d8107015 100644 --- a/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs +++ b/test/OpenTelemetry.Contrib.Instrumentation.AWS.Tests/Implementation/TestsHelper.cs @@ -26,12 +26,12 @@ namespace OpenTelemetry.Contrib.Instrumentation.AWS.Tests.Implementation; internal static class TestsHelper { - internal static Action>? CreateAddAttributesAction(string serviceType, IRequestContext context) + internal static Action>? CreateAddAttributesAction(string serviceType, IRequestContext context) { return serviceType switch { - AWSServiceType.SQSService => SqsRequestContextAdapter.CreateAddAttributesAction(context), - AWSServiceType.SNSService => SnsRequestContextAdapter.CreateAddAttributesAction(context), + AWSServiceType.SQSService => SqsRequestContextHelper.AddAttributes, + AWSServiceType.SNSService => SnsRequestContextHelper.AddAttributes, _ => throw new NotSupportedException($"Tests for service type {serviceType} not supported."), }; } From 94e489cd791b1f0bf15ac1c54b3c6898827d8869 Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Fri, 24 Mar 2023 14:21:26 +0100 Subject: [PATCH 12/23] #1034: replaced string.StartsWith(...) call by it's overload with StringComparison --- .../Implementation/AWSMessagingUtils.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs index 0be2c14b32..762c7c6025 100644 --- a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs @@ -118,7 +118,7 @@ private static SNSEvent.SNSMessage GetSnsMessage(SQSEvent.SQSMessage sqsMessage) var body = sqsMessage.Body; if (body != null && - body.TrimStart().StartsWith("{") && + body.TrimStart().StartsWith("{", StringComparison.Ordinal) && body.Contains(SnsMessageAttributes)) { try From e88edbe98534c65924834d96edc2b0b1a706fb8a Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Mon, 27 Mar 2023 16:31:48 +0200 Subject: [PATCH 13/23] #1034: added sqs messages contexts as activity links --- .../AWSLambdaWrapper.cs | 6 +++-- .../Implementation/AWSLambdaUtils.cs | 19 ++++++++------- .../Implementation/AWSMessagingUtils.cs | 24 ++++++++++++++++--- 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/AWSLambdaWrapper.cs b/src/OpenTelemetry.Instrumentation.AWSLambda/AWSLambdaWrapper.cs index ba62e517cc..937ad3f9aa 100644 --- a/src/OpenTelemetry.Instrumentation.AWSLambda/AWSLambdaWrapper.cs +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/AWSLambdaWrapper.cs @@ -15,6 +15,7 @@ // using System; +using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; @@ -170,9 +171,10 @@ public static Task TraceAsync( internal static Activity OnFunctionStart(TInput input, ILambdaContext context, ActivityContext parentContext = default) { + IEnumerable links = null; if (parentContext == default) { - parentContext = AWSLambdaUtils.ExtractParentContext(input); + (parentContext, links) = AWSLambdaUtils.ExtractParentContext(input); if (parentContext == default && !DisableAwsXRayContextExtraction) { parentContext = AWSLambdaUtils.GetXRayParentContext(); @@ -184,7 +186,7 @@ internal static Activity OnFunctionStart(TInput input, ILambdaContext co // We assume that functionTags and httpTags have no intersection. var activityName = AWSLambdaUtils.GetFunctionName(context) ?? "AWS Lambda Invoke"; - var activity = AWSLambdaActivitySource.StartActivity(activityName, ActivityKind.Server, parentContext, functionTags.Concat(httpTags)); + var activity = AWSLambdaActivitySource.StartActivity(activityName, ActivityKind.Server, parentContext, functionTags.Concat(httpTags), links); return activity; } diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSLambdaUtils.cs b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSLambdaUtils.cs index c5b794ffd0..234b86fb62 100644 --- a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSLambdaUtils.cs +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSLambdaUtils.cs @@ -65,32 +65,33 @@ internal static ActivityContext GetXRayParentContext() return activityContext; } - internal static ActivityContext ExtractParentContext(TInput input) + internal static (ActivityContext ParentContext, IEnumerable Links) ExtractParentContext(TInput input) { - PropagationContext propagationContext = default; + PropagationContext parentContext = default; + IEnumerable links = null; switch (input) { case APIGatewayProxyRequest apiGatewayProxyRequest: - propagationContext = Propagators.DefaultTextMapPropagator.Extract(default, apiGatewayProxyRequest, GetHeaderValues); + parentContext = Propagators.DefaultTextMapPropagator.Extract(default, apiGatewayProxyRequest, GetHeaderValues); break; case APIGatewayHttpApiV2ProxyRequest apiGatewayHttpApiV2ProxyRequest: - propagationContext = Propagators.DefaultTextMapPropagator.Extract(default, apiGatewayHttpApiV2ProxyRequest, GetHeaderValues); + parentContext = Propagators.DefaultTextMapPropagator.Extract(default, apiGatewayHttpApiV2ProxyRequest, GetHeaderValues); break; case SQSEvent sqsEvent: - propagationContext = AWSMessagingUtils.ExtractParentContext(sqsEvent); + (parentContext, links) = AWSMessagingUtils.ExtractParentContext(sqsEvent); break; case SQSEvent.SQSMessage sqsMessage: - propagationContext = AWSMessagingUtils.ExtractParentContext(sqsMessage); + parentContext = AWSMessagingUtils.ExtractParentContext(sqsMessage); break; case SNSEvent snsEvent: - propagationContext = AWSMessagingUtils.ExtractParentContext(snsEvent); + parentContext = AWSMessagingUtils.ExtractParentContext(snsEvent); break; case SNSEvent.SNSRecord snsRecord: - propagationContext = AWSMessagingUtils.ExtractParentContext(snsRecord); + parentContext = AWSMessagingUtils.ExtractParentContext(snsRecord); break; } - return propagationContext.ActivityContext; + return (parentContext.ActivityContext, links); } internal static string GetCloudProvider() diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs index 762c7c6025..53e5221675 100644 --- a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs @@ -16,6 +16,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using Amazon.Lambda.SNSEvents; using Amazon.Lambda.SQSEvents; @@ -31,11 +32,28 @@ internal class AWSMessagingUtils private const string SnsAttributeTypeStringArray = "String.Array"; private const string SnsMessageAttributes = "MessageAttributes"; - internal static PropagationContext ExtractParentContext(SQSEvent sqsEvent) + internal static (PropagationContext ParentContext, IEnumerable Links) ExtractParentContext(SQSEvent sqsEvent) { + if (sqsEvent?.Records == null) + { + return (default, null); + } + // We assume there can be only one parent that's why we consider only a single (the last) record as the carrier. - var message = sqsEvent?.Records?.LastOrDefault(); - return ExtractParentContext(message); + var parentRecord = sqsEvent.Records.LastOrDefault(); + var parentContext = ExtractParentContext(parentRecord); + + var links = new List(); + foreach (var record in sqsEvent.Records) + { + var context = ExtractParentContext(record); + if (context != default) + { + links.Add(new ActivityLink(context.ActivityContext)); + } + } + + return (parentContext, links); } internal static PropagationContext ExtractParentContext(SQSEvent.SQSMessage sqsMessage) From 5bed7c6fd29de01e805b4ee6b41553e4021a0a06 Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Wed, 29 Mar 2023 17:05:37 +0200 Subject: [PATCH 14/23] #1034: small code optimization --- .../Implementation/AWSMessagingUtils.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs index 53e5221675..732856e7a8 100644 --- a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs @@ -46,7 +46,7 @@ internal static (PropagationContext ParentContext, IEnumerable Lin var links = new List(); foreach (var record in sqsEvent.Records) { - var context = ExtractParentContext(record); + var context = ReferenceEquals(record, parentRecord) ? parentContext : ExtractParentContext(record); if (context != default) { links.Add(new ActivityLink(context.ActivityContext)); From d2613c41bb773daaaedd0954887fdee088565b6a Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Wed, 12 Apr 2023 17:47:36 +0200 Subject: [PATCH 15/23] #1034: added option for configuring setting parent from SQS messages batch --- .../netstandard2.0/PublicAPI.Unshipped.txt | 2 ++ .../AWSLambdaInstrumentationOptions.cs | 6 ++++++ .../Implementation/AWSMessagingUtils.cs | 15 +++++++++++---- .../TracerProviderBuilderExtensions.cs | 1 + 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Instrumentation.AWSLambda/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt index 04f3cd5b24..53f2ba0765 100644 --- a/src/OpenTelemetry.Instrumentation.AWSLambda/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt @@ -2,6 +2,8 @@ OpenTelemetry.Instrumentation.AWSLambda.AWSLambdaInstrumentationOptions OpenTelemetry.Instrumentation.AWSLambda.AWSLambdaInstrumentationOptions.AWSLambdaInstrumentationOptions() -> void OpenTelemetry.Instrumentation.AWSLambda.AWSLambdaInstrumentationOptions.DisableAwsXRayContextExtraction.get -> bool OpenTelemetry.Instrumentation.AWSLambda.AWSLambdaInstrumentationOptions.DisableAwsXRayContextExtraction.set -> void +OpenTelemetry.Instrumentation.AWSLambda.AWSLambdaInstrumentationOptions.SetParentFromMessageBatch.get -> bool +OpenTelemetry.Instrumentation.AWSLambda.AWSLambdaInstrumentationOptions.SetParentFromMessageBatch.set -> void OpenTelemetry.Instrumentation.AWSLambda.AWSLambdaWrapper OpenTelemetry.Instrumentation.AWSLambda.TracerProviderBuilderExtensions static OpenTelemetry.Instrumentation.AWSLambda.AWSLambdaWrapper.Trace(OpenTelemetry.Trace.TracerProvider tracerProvider, System.Func lambdaHandler, TInput input, Amazon.Lambda.Core.ILambdaContext context, System.Diagnostics.ActivityContext parentContext = default(System.Diagnostics.ActivityContext)) -> TResult diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/AWSLambdaInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.AWSLambda/AWSLambdaInstrumentationOptions.cs index 1fc086521c..72f8e83476 100644 --- a/src/OpenTelemetry.Instrumentation.AWSLambda/AWSLambdaInstrumentationOptions.cs +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/AWSLambdaInstrumentationOptions.cs @@ -25,4 +25,10 @@ public class AWSLambdaInstrumentationOptions /// Gets or sets a value indicating whether AWS X-Ray context extraction should be disabled. /// public bool DisableAwsXRayContextExtraction { get; set; } + + /// + /// Gets or sets a value indicating whether the parent Activity should be set when SQS message batch is received. + /// If option is set to true then the parent is set using the last received message otherwise the parent is not set at all. + /// + public bool SetParentFromMessageBatch { get; set; } } diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs index 732856e7a8..457acd4fe3 100644 --- a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs @@ -32,6 +32,12 @@ internal class AWSMessagingUtils private const string SnsAttributeTypeStringArray = "String.Array"; private const string SnsMessageAttributes = "MessageAttributes"; + /// + /// Gets or sets a value indicating whether the parent Activity should be set when SQS message batch is received. + /// If option is set to true then the parent is set using the last received message otherwise the parent is not set at all. + /// + internal static bool SetParentFromMessageBatch { get; set; } + internal static (PropagationContext ParentContext, IEnumerable Links) ExtractParentContext(SQSEvent sqsEvent) { if (sqsEvent?.Records == null) @@ -39,9 +45,9 @@ internal static (PropagationContext ParentContext, IEnumerable Lin return (default, null); } - // We assume there can be only one parent that's why we consider only a single (the last) record as the carrier. - var parentRecord = sqsEvent.Records.LastOrDefault(); - var parentContext = ExtractParentContext(parentRecord); + // We choose the last message (record) as the carrier to set the parent. + var parentRecord = SetParentFromMessageBatch ? sqsEvent.Records.LastOrDefault() : null; + var parentContext = (parentRecord != null) ? ExtractParentContext(parentRecord) : default; var links = new List(); foreach (var record in sqsEvent.Records) @@ -80,7 +86,8 @@ internal static PropagationContext ExtractParentContext(SQSEvent.SQSMessage sqsM internal static PropagationContext ExtractParentContext(SNSEvent snsEvent) { - // We assume there can be only one parent that's why we consider only a single (the last) record as the carrier. + // We assume there can be only a single SNS record (message) and records list is kept in the model consistency. + // See https://aws.amazon.com/sns/faqs/ for details. var record = snsEvent?.Records?.LastOrDefault(); return ExtractParentContext(record); } diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AWSLambda/TracerProviderBuilderExtensions.cs index 5be84b18df..dbc17b26d8 100644 --- a/src/OpenTelemetry.Instrumentation.AWSLambda/TracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/TracerProviderBuilderExtensions.cs @@ -51,6 +51,7 @@ public static TracerProviderBuilder AddAWSLambdaConfigurations( configure?.Invoke(options); AWSLambdaWrapper.DisableAwsXRayContextExtraction = options.DisableAwsXRayContextExtraction; + AWSMessagingUtils.SetParentFromMessageBatch = options.SetParentFromMessageBatch; builder.AddSource(AWSLambdaWrapper.ActivitySourceName); builder.SetResourceBuilder(ResourceBuilder From 546ef3fd68e8e93368af53fd6dcee079b8ba3a45 Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin <88040756+rypdal@users.noreply.github.com> Date: Thu, 13 Apr 2023 10:06:14 +0200 Subject: [PATCH 16/23] Update src/OpenTelemetry.Instrumentation.AWSLambda/AWSLambdaInstrumentationOptions.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1034: suggested config option description Co-authored-by: Christian Neumüller --- .../AWSLambdaInstrumentationOptions.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/AWSLambdaInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.AWSLambda/AWSLambdaInstrumentationOptions.cs index 72f8e83476..8930b25a49 100644 --- a/src/OpenTelemetry.Instrumentation.AWSLambda/AWSLambdaInstrumentationOptions.cs +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/AWSLambdaInstrumentationOptions.cs @@ -27,8 +27,11 @@ public class AWSLambdaInstrumentationOptions public bool DisableAwsXRayContextExtraction { get; set; } /// - /// Gets or sets a value indicating whether the parent Activity should be set when SQS message batch is received. - /// If option is set to true then the parent is set using the last received message otherwise the parent is not set at all. + /// Gets or sets a value indicating whether the parent Activity should be set when a potentially batched event is received where multiple parents are potentially available (e.g. SQS). + /// If set to true, the parent is set using the last received record (e.g. last message). Otherwise the parent is not set. In both cases, links will be created for such events. /// - public bool SetParentFromMessageBatch { get; set; } + /// + /// Currently, the only event type to which this applies is SQS. + /// + public bool SetParentFromBatch { get; set; } } From 0460e9b14f9e3c2651cdf82f531b09c12e5d787b Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin <88040756+rypdal@users.noreply.github.com> Date: Thu, 13 Apr 2023 10:12:06 +0200 Subject: [PATCH 17/23] Update src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1034: review suggestion Co-authored-by: Christian Neumüller --- .../Implementation/AWSMessagingUtils.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs index 457acd4fe3..05f28caacf 100644 --- a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs @@ -87,7 +87,7 @@ internal static PropagationContext ExtractParentContext(SQSEvent.SQSMessage sqsM internal static PropagationContext ExtractParentContext(SNSEvent snsEvent) { // We assume there can be only a single SNS record (message) and records list is kept in the model consistency. - // See https://aws.amazon.com/sns/faqs/ for details. + // See https://aws.amazon.com/sns/faqs/#Reliability for details. var record = snsEvent?.Records?.LastOrDefault(); return ExtractParentContext(record); } From 297effc66f8e81d91069ff8fb11425ed1bd903f9 Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Thu, 13 Apr 2023 16:18:41 +0200 Subject: [PATCH 18/23] #1034: aligned naming + unit tests for SetParentFromMessageBatch option --- .../netstandard2.0/PublicAPI.Unshipped.txt | 4 +- .../TracerProviderBuilderExtensions.cs | 2 +- .../Implementation/AWSMessagingUtilsTests.cs | 79 +++++++++++++++++++ 3 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 test/OpenTelemetry.Instrumentation.AWSLambda.Tests/Implementation/AWSMessagingUtilsTests.cs diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Instrumentation.AWSLambda/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt index 53f2ba0765..d5ed4319ec 100644 --- a/src/OpenTelemetry.Instrumentation.AWSLambda/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt @@ -2,8 +2,8 @@ OpenTelemetry.Instrumentation.AWSLambda.AWSLambdaInstrumentationOptions OpenTelemetry.Instrumentation.AWSLambda.AWSLambdaInstrumentationOptions.AWSLambdaInstrumentationOptions() -> void OpenTelemetry.Instrumentation.AWSLambda.AWSLambdaInstrumentationOptions.DisableAwsXRayContextExtraction.get -> bool OpenTelemetry.Instrumentation.AWSLambda.AWSLambdaInstrumentationOptions.DisableAwsXRayContextExtraction.set -> void -OpenTelemetry.Instrumentation.AWSLambda.AWSLambdaInstrumentationOptions.SetParentFromMessageBatch.get -> bool -OpenTelemetry.Instrumentation.AWSLambda.AWSLambdaInstrumentationOptions.SetParentFromMessageBatch.set -> void +OpenTelemetry.Instrumentation.AWSLambda.AWSLambdaInstrumentationOptions.SetParentFromBatch.get -> bool +OpenTelemetry.Instrumentation.AWSLambda.AWSLambdaInstrumentationOptions.SetParentFromBatch.set -> void OpenTelemetry.Instrumentation.AWSLambda.AWSLambdaWrapper OpenTelemetry.Instrumentation.AWSLambda.TracerProviderBuilderExtensions static OpenTelemetry.Instrumentation.AWSLambda.AWSLambdaWrapper.Trace(OpenTelemetry.Trace.TracerProvider tracerProvider, System.Func lambdaHandler, TInput input, Amazon.Lambda.Core.ILambdaContext context, System.Diagnostics.ActivityContext parentContext = default(System.Diagnostics.ActivityContext)) -> TResult diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AWSLambda/TracerProviderBuilderExtensions.cs index dbc17b26d8..e543c9dc94 100644 --- a/src/OpenTelemetry.Instrumentation.AWSLambda/TracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/TracerProviderBuilderExtensions.cs @@ -51,7 +51,7 @@ public static TracerProviderBuilder AddAWSLambdaConfigurations( configure?.Invoke(options); AWSLambdaWrapper.DisableAwsXRayContextExtraction = options.DisableAwsXRayContextExtraction; - AWSMessagingUtils.SetParentFromMessageBatch = options.SetParentFromMessageBatch; + AWSMessagingUtils.SetParentFromMessageBatch = options.SetParentFromBatch; builder.AddSource(AWSLambdaWrapper.ActivitySourceName); builder.SetResourceBuilder(ResourceBuilder diff --git a/test/OpenTelemetry.Instrumentation.AWSLambda.Tests/Implementation/AWSMessagingUtilsTests.cs b/test/OpenTelemetry.Instrumentation.AWSLambda.Tests/Implementation/AWSMessagingUtilsTests.cs new file mode 100644 index 0000000000..fda5c9136c --- /dev/null +++ b/test/OpenTelemetry.Instrumentation.AWSLambda.Tests/Implementation/AWSMessagingUtilsTests.cs @@ -0,0 +1,79 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using Amazon.Lambda.SQSEvents; +using OpenTelemetry.Context.Propagation; +using OpenTelemetry.Instrumentation.AWSLambda.Implementation; +using OpenTelemetry.Trace; +using Xunit; +using static Amazon.Lambda.SQSEvents.SQSEvent; + +namespace OpenTelemetry.Instrumentation.AWSLambda.Tests.Implementation; + +public class AWSMessagingUtilsTests +{ + private const string TraceId = "0af7651916cd43dd8448eb211c80319c"; + private const string SpanId1 = "b9c7c989f97918e1"; + private const string SpanId2 = "b9c7c989f97918e2"; + + public AWSMessagingUtilsTests() + { + var tracerProvider = Sdk.CreateTracerProviderBuilder() + .Build(); + } + + [Fact] + public void ExtractParentContext_SetParentFromMessageBatchIsDisabled_ParentIsNotSet() + { + AWSMessagingUtils.SetParentFromMessageBatch = false; + var @event = CreateSqsEventWithMessages(new[] { SpanId1, SpanId2 }); + + (PropagationContext parentContext, IEnumerable links) = AWSMessagingUtils.ExtractParentContext(@event); + + Assert.Equal(default, parentContext); + Assert.Equal(2, links.Count()); + } + + [Fact] + public void ExtractParentContext_SetParentFromMessageBatchIsEnabled_ParentIsSetFromLastMessage() + { + AWSMessagingUtils.SetParentFromMessageBatch = true; + var @event = CreateSqsEventWithMessages(new[] { SpanId1, SpanId2 }); + + (PropagationContext parentContext, IEnumerable links) = AWSMessagingUtils.ExtractParentContext(@event); + + Assert.NotEqual(default, parentContext); + Assert.Equal(SpanId2, parentContext.ActivityContext.SpanId.ToHexString()); + Assert.Equal(2, links.Count()); + } + + private static SQSEvent CreateSqsEventWithMessages(string[] spans) + { + var @event = new SQSEvent { Records = new List() }; + for (var i = 0; i < spans.Length; i++) + { + var message = new SQSMessage { MessageAttributes = new Dictionary() }; + message.MessageAttributes.Add("traceparent", new MessageAttribute { StringValue = $"00-{TraceId}-{spans[i]}-01" }); + message.MessageAttributes.Add("tracestate", new MessageAttribute { StringValue = $"k1=v1,k2=v2" }); + @event.Records.Add(message); + } + + return @event; + } +} From 9a9fa26b543afe45e8dbea80b481fde9545d596f Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Thu, 13 Apr 2023 17:45:31 +0200 Subject: [PATCH 19/23] #1034: renamed var to avoid @-synthax --- .../Implementation/AWSMessagingUtilsTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/OpenTelemetry.Instrumentation.AWSLambda.Tests/Implementation/AWSMessagingUtilsTests.cs b/test/OpenTelemetry.Instrumentation.AWSLambda.Tests/Implementation/AWSMessagingUtilsTests.cs index fda5c9136c..aba531493b 100644 --- a/test/OpenTelemetry.Instrumentation.AWSLambda.Tests/Implementation/AWSMessagingUtilsTests.cs +++ b/test/OpenTelemetry.Instrumentation.AWSLambda.Tests/Implementation/AWSMessagingUtilsTests.cs @@ -42,9 +42,9 @@ public AWSMessagingUtilsTests() public void ExtractParentContext_SetParentFromMessageBatchIsDisabled_ParentIsNotSet() { AWSMessagingUtils.SetParentFromMessageBatch = false; - var @event = CreateSqsEventWithMessages(new[] { SpanId1, SpanId2 }); + var sqsEvent = CreateSqsEventWithMessages(new[] { SpanId1, SpanId2 }); - (PropagationContext parentContext, IEnumerable links) = AWSMessagingUtils.ExtractParentContext(@event); + (PropagationContext parentContext, IEnumerable links) = AWSMessagingUtils.ExtractParentContext(sqsEvent); Assert.Equal(default, parentContext); Assert.Equal(2, links.Count()); @@ -54,9 +54,9 @@ public void ExtractParentContext_SetParentFromMessageBatchIsDisabled_ParentIsNot public void ExtractParentContext_SetParentFromMessageBatchIsEnabled_ParentIsSetFromLastMessage() { AWSMessagingUtils.SetParentFromMessageBatch = true; - var @event = CreateSqsEventWithMessages(new[] { SpanId1, SpanId2 }); + var sqsEvent = CreateSqsEventWithMessages(new[] { SpanId1, SpanId2 }); - (PropagationContext parentContext, IEnumerable links) = AWSMessagingUtils.ExtractParentContext(@event); + (PropagationContext parentContext, IEnumerable links) = AWSMessagingUtils.ExtractParentContext(sqsEvent); Assert.NotEqual(default, parentContext); Assert.Equal(SpanId2, parentContext.ActivityContext.SpanId.ToHexString()); From 89cb763c1f1a0acee580750642c7c3768ba3a2f8 Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Thu, 27 Apr 2023 10:35:08 +0200 Subject: [PATCH 20/23] #1034: CHANGELOG update --- src/OpenTelemetry.Contrib.Instrumentation.AWS/CHANGELOG.md | 2 ++ src/OpenTelemetry.Instrumentation.AWSLambda/CHANGELOG.md | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/CHANGELOG.md b/src/OpenTelemetry.Contrib.Instrumentation.AWS/CHANGELOG.md index ae9dc28eab..3f53aed310 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/CHANGELOG.md +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/CHANGELOG.md @@ -6,6 +6,8 @@ ([#1095](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1095)) * Removes `AddAWSInstrumentation` method with default configure default parameter. ([#1117](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1117)) +* Added tracing of outgoing SQS and SNS calls. + ([#1051](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1051)) ## 1.0.2 diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AWSLambda/CHANGELOG.md index 3cbdb46d89..d4879480fd 100644 --- a/src/OpenTelemetry.Instrumentation.AWSLambda/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/CHANGELOG.md @@ -8,6 +8,8 @@ ([#943](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/943)) * BREAKING (behavior): `AddAWSLambdaConfigurations` no longer calls `AddService` ([#1080](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1080)) +* Added tracing of Lambda handlers receiving SQS and SNS messages. + ([#1051](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1051)) ## 1.1.0-beta.2 From f6a6f7b22ac7d98890f72b9178d50b87d98ef918 Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Thu, 27 Apr 2023 10:47:04 +0200 Subject: [PATCH 21/23] #1034: changelog description update and white space fix --- src/OpenTelemetry.Instrumentation.AWSLambda/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AWSLambda/CHANGELOG.md index d4879480fd..fb136f3eb6 100644 --- a/src/OpenTelemetry.Instrumentation.AWSLambda/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AWSLambda/CHANGELOG.md @@ -8,7 +8,7 @@ ([#943](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/943)) * BREAKING (behavior): `AddAWSLambdaConfigurations` no longer calls `AddService` ([#1080](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1080)) -* Added tracing of Lambda handlers receiving SQS and SNS messages. +* Added tracing of AWS Lambda handlers receiving SQS and SNS messages. ([#1051](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1051)) ## 1.1.0-beta.2 From 8cec2c79ec3954e15e1757ff2cb9889a350f5c07 Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin <88040756+rypdal@users.noreply.github.com> Date: Thu, 27 Apr 2023 11:15:56 +0200 Subject: [PATCH 22/23] Update src/OpenTelemetry.Contrib.Instrumentation.AWS/CHANGELOG.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Christian Neumüller --- src/OpenTelemetry.Contrib.Instrumentation.AWS/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/CHANGELOG.md b/src/OpenTelemetry.Contrib.Instrumentation.AWS/CHANGELOG.md index 3f53aed310..ee78bb6066 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/CHANGELOG.md +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/CHANGELOG.md @@ -6,7 +6,7 @@ ([#1095](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1095)) * Removes `AddAWSInstrumentation` method with default configure default parameter. ([#1117](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1117)) -* Added tracing of outgoing SQS and SNS calls. +* Global propagator is now used to inject into sent SQS and SNS message attributes (in addition to X-Ray propagation). ([#1051](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1051)) ## 1.0.2 From d0a30014a26df620dccf91962d980ec637ae04bd Mon Sep 17 00:00:00 2001 From: Oleksiy Dubinin Date: Thu, 27 Apr 2023 11:21:02 +0200 Subject: [PATCH 23/23] #1034: long line split --- src/OpenTelemetry.Contrib.Instrumentation.AWS/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Contrib.Instrumentation.AWS/CHANGELOG.md b/src/OpenTelemetry.Contrib.Instrumentation.AWS/CHANGELOG.md index ee78bb6066..31cde222a7 100644 --- a/src/OpenTelemetry.Contrib.Instrumentation.AWS/CHANGELOG.md +++ b/src/OpenTelemetry.Contrib.Instrumentation.AWS/CHANGELOG.md @@ -6,7 +6,8 @@ ([#1095](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1095)) * Removes `AddAWSInstrumentation` method with default configure default parameter. ([#1117](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1117)) -* Global propagator is now used to inject into sent SQS and SNS message attributes (in addition to X-Ray propagation). +* Global propagator is now used to inject into sent SQS and SNS message + attributes (in addition to X-Ray propagation). ([#1051](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1051)) ## 1.0.2