Address peer review feedback

- Use simpler JSON deserialization. - Remove dependency on Amazon.Lambda.Serialization.SystemTextJson. - Fix-up nullability.
open-telemetry · Jan 22, 2024 · 8c69f42 · 8c69f42
1 parent b1d8ac9
commit 8c69f42
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 31 deletions.
diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/AWSLambdaWrapper.cs b/src/OpenTelemetry.Instrumentation.AWSLambda/AWSLambdaWrapper.cs
@@ -23,10 +23,10 @@ public static class AWSLambdaWrapper
     private static readonly AssemblyName AssemblyName = typeof(AWSLambdaWrapper).Assembly.GetName();
 
     [SuppressMessage("StyleCop.CSharp.OrderingRules", "SA1202:ElementsMustBeOrderedByAccess", Justification = "Initialization order.")]
-    internal static readonly string ActivitySourceName = AssemblyName.Name!;
+    internal static readonly string? ActivitySourceName = AssemblyName.Name;
 
-    private static readonly Version Version = AssemblyName.Version!;
-    private static readonly ActivitySource AWSLambdaActivitySource = new(ActivitySourceName, Version.ToString());
+    private static readonly Version? Version = AssemblyName.Version;
+    private static readonly ActivitySource AWSLambdaActivitySource = new(ActivitySourceName ?? "OpenTelemetry.Instrumentation.AWSLambda", Version?.ToString());
 
     /// <summary>
     /// Gets or sets a value indicating whether AWS X-Ray propagation should be ignored. Default value is false.

diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSLambdaResourceDetector.cs b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSLambdaResourceDetector.cs
@@ -14,12 +14,12 @@ internal sealed class AWSLambdaResourceDetector : IResourceDetector
     /// <returns>Detected resource.</returns>
     public Resource Detect()
     {
-        var resourceAttributes = new List<KeyValuePair<string, object>>
+        var resourceAttributes = new List<KeyValuePair<string, object>>(4)
         {
             new(AWSLambdaSemanticConventions.AttributeCloudProvider, AWSLambdaUtils.GetCloudProvider()),
-            new(AWSLambdaSemanticConventions.AttributeCloudRegion, AWSLambdaUtils.GetAWSRegion()),
-            new(AWSLambdaSemanticConventions.AttributeFaasName, AWSLambdaUtils.GetFunctionName()),
-            new(AWSLambdaSemanticConventions.AttributeFaasVersion, AWSLambdaUtils.GetFunctionVersion()),
+            new(AWSLambdaSemanticConventions.AttributeCloudRegion, AWSLambdaUtils.GetAWSRegion() ?? string.Empty),
+            new(AWSLambdaSemanticConventions.AttributeFaasName, AWSLambdaUtils.GetFunctionName() ?? string.Empty),
+            new(AWSLambdaSemanticConventions.AttributeFaasVersion, AWSLambdaUtils.GetFunctionVersion() ?? string.Empty),
         };
 
         return new Resource(resourceAttributes);

diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSLambdaUtils.cs b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSLambdaUtils.cs
@@ -86,19 +86,19 @@ internal static string GetCloudProvider()
         return CloudProvider;
     }
 
-    internal static string GetAWSRegion()
+    internal static string? GetAWSRegion()
     {
-        return Environment.GetEnvironmentVariable(AWSRegion) ?? string.Empty;
+        return Environment.GetEnvironmentVariable(AWSRegion);
     }
 
-    internal static string GetFunctionName(ILambdaContext? context = null)
+    internal static string? GetFunctionName(ILambdaContext? context = null)
     {
-        return context?.FunctionName ?? Environment.GetEnvironmentVariable(FunctionName) ?? string.Empty;
+        return context?.FunctionName ?? Environment.GetEnvironmentVariable(FunctionName);
     }
 
-    internal static string GetFunctionVersion()
+    internal static string? GetFunctionVersion()
     {
-        return Environment.GetEnvironmentVariable(FunctionVersion) ?? string.Empty;
+        return Environment.GetEnvironmentVariable(FunctionVersion);
     }
 
     internal static IEnumerable<KeyValuePair<string, object>> GetFunctionTags<TInput>(TInput input, ILambdaContext context)

diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs b/src/OpenTelemetry.Instrumentation.AWSLambda/Implementation/AWSMessagingUtils.cs
@@ -4,16 +4,8 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
-#if NET6_0_OR_GREATER
-using System.IO;
-#endif
 using System.Linq;
-#if !NET6_0_OR_GREATER
 using System.Text.Json;
-#endif
-#if NET6_0_OR_GREATER
-using Amazon.Lambda.Serialization.SystemTextJson;
-#endif
 using Amazon.Lambda.SNSEvents;
 using Amazon.Lambda.SQSEvents;
 using OpenTelemetry.Context.Propagation;
@@ -27,10 +19,6 @@ internal class AWSMessagingUtils
     private const string SnsAttributeTypeStringArray = "String.Array";
     private const string SnsMessageAttributes = "MessageAttributes";
 
-#if NET6_0_OR_GREATER
-    private static readonly SourceGeneratorLambdaJsonSerializer<SourceGenerationContext> LambdaSerializer = new();
-#endif
-
     /// <summary>
     /// 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.
@@ -150,8 +138,7 @@ internal static PropagationContext ExtractParentContext(SNSEvent.SNSMessage? mes
             try
             {
 #if NET6_0_OR_GREATER
-                using var stream = new MemoryStream(System.Text.Encoding.UTF8.GetBytes(body));
-                snsMessage = LambdaSerializer.Deserialize<SNSEvent.SNSMessage>(stream);
+                snsMessage = JsonSerializer.Deserialize(body, SourceGenerationContext.Default.SNSMessage);
 #else
                 snsMessage = JsonSerializer.Deserialize<SNSEvent.SNSMessage>(body);
 #endif

diff --git a/src/OpenTelemetry.Instrumentation.AWSLambda/OpenTelemetry.Instrumentation.AWSLambda.csproj b/src/OpenTelemetry.Instrumentation.AWSLambda/OpenTelemetry.Instrumentation.AWSLambda.csproj
@@ -19,10 +19,6 @@
     <PackageReference Include="System.Text.Json" Version="6.0.0" />
   </ItemGroup>
 
-  <ItemGroup Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net6.0'))">
-    <PackageReference Include="Amazon.Lambda.Serialization.SystemTextJson" Version="2.4.0" />
-  </ItemGroup>
-
   <ItemGroup>
     <Compile Include="$(RepoRoot)\src\Shared\Guard.cs" Link="Includes\Guard.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\SemanticConventions.cs" Link="Includes\SemanticConventions.cs"/>