From 051305c8f911148e3c8c1581229d8c47345d1fb0 Mon Sep 17 00:00:00 2001 From: Henrique <999396+hjgraca@users.noreply.github.com> Date: Wed, 21 Aug 2024 12:49:02 +0100 Subject: [PATCH 1/2] Update RecordHanderResult to avoid cirular instantiation on static property, this was breaking LitJson parser on X-Ray SDK with Max allowed object depth reached. Tracing now takes into account the CaptureMode set for each method XRayRecorder now captures the exceptions from EndSubsegment call and does not surface exception to the client Update nuget packages for X-Ray SDK --- .../RecordHandlerResult.cs | 2 +- .../Internal/TracingAspect.cs | 62 +++++++++--------- .../Internal/XRayRecorder.cs | 25 ++++++-- libraries/src/Directory.Packages.props | 6 +- .../Handlers/Handlers.cs | 20 ++++++ .../TracingAttributeTest.cs | 63 +++++++++++++++++++ .../XRayRecorderTests.cs | 32 +++++++++- 7 files changed, 167 insertions(+), 43 deletions(-) diff --git a/libraries/src/AWS.Lambda.Powertools.BatchProcessing/RecordHandlerResult.cs b/libraries/src/AWS.Lambda.Powertools.BatchProcessing/RecordHandlerResult.cs index 4594c242..01419d89 100644 --- a/libraries/src/AWS.Lambda.Powertools.BatchProcessing/RecordHandlerResult.cs +++ b/libraries/src/AWS.Lambda.Powertools.BatchProcessing/RecordHandlerResult.cs @@ -23,7 +23,7 @@ public class RecordHandlerResult /// /// Returns an empty value. /// - public static readonly RecordHandlerResult None = new(); + public static RecordHandlerResult None { get; } = null!; /// /// Convenience method for the creation of a . diff --git a/libraries/src/AWS.Lambda.Powertools.Tracing/Internal/TracingAspect.cs b/libraries/src/AWS.Lambda.Powertools.Tracing/Internal/TracingAspect.cs index 655e2229..46062d0d 100644 --- a/libraries/src/AWS.Lambda.Powertools.Tracing/Internal/TracingAspect.cs +++ b/libraries/src/AWS.Lambda.Powertools.Tracing/Internal/TracingAspect.cs @@ -15,7 +15,6 @@ using System; using System.Linq; -using System.Reflection; using System.Runtime.ExceptionServices; using System.Text; using AspectInjector.Broker; @@ -39,7 +38,7 @@ public class TracingAspect /// X-Ray Recorder /// private readonly IXRayRecorder _xRayRecorder; - + /// /// If true, then is cold start /// @@ -49,17 +48,17 @@ public class TracingAspect /// If true, capture annotations /// private static bool _captureAnnotations = true; - + /// /// If true, annotations have been captured /// private bool _isAnnotationsCaptured; - + /// /// Tracing namespace /// private string _namespace; - + /// /// The capture mode /// @@ -73,18 +72,14 @@ public TracingAspect() _xRayRecorder = XRayRecorder.Instance; _powertoolsConfigurations = PowertoolsConfigurations.Instance; } - + /// /// the code is executed instead of the target method. /// The call to original method is wrapped around the following code /// the original code is called with var result = target(args); /// - /// /// /// - /// - /// - /// /// /// /// @@ -97,38 +92,39 @@ public object Around( { // Before running Function + var trigger = triggers.OfType().First(); try { - var trigger = triggers.OfType().First(); - - if(TracingDisabled()) + if (TracingDisabled()) return target(args); _namespace = trigger.Namespace; - _captureMode = trigger.CaptureMode; - + var segmentName = !string.IsNullOrWhiteSpace(trigger.SegmentName) ? trigger.SegmentName : $"## {name}"; var nameSpace = GetNamespace(); _xRayRecorder.BeginSubsegment(segmentName); _xRayRecorder.SetNamespace(nameSpace); - if (_captureAnnotations) + if (_captureAnnotations) { _xRayRecorder.AddAnnotation("ColdStart", _isColdStart); - + _captureAnnotations = false; _isAnnotationsCaptured = true; if (_powertoolsConfigurations.IsServiceDefined) _xRayRecorder.AddAnnotation("Service", _powertoolsConfigurations.Service); } - + _isColdStart = false; - + // return of the handler var result = target(args); - + + // must get capture after all subsegments run + _captureMode = trigger.CaptureMode; + if (CaptureResponse()) { _xRayRecorder.AddMetadata @@ -138,12 +134,13 @@ public object Around( result ); } - + // after return result; } catch (Exception e) { + _captureMode = trigger.CaptureMode; HandleException(e, name); throw; } @@ -153,8 +150,9 @@ public object Around( /// the code is injected after the method ends. /// [Advice(Kind.After)] - public void OnExit() { - if(TracingDisabled()) + public void OnExit() + { + if (TracingDisabled()) return; if (_isAnnotationsCaptured) @@ -162,7 +160,7 @@ public void OnExit() { _xRayRecorder.EndSubsegment(); } - + /// /// Code that handles when exceptions occur in the client method /// @@ -173,7 +171,7 @@ private void HandleException(Exception exception, string name) if (CaptureError()) { var nameSpace = GetNamespace(); - + var sb = new StringBuilder(); sb.AppendLine($"Exception type: {exception.GetType()}"); sb.AppendLine($"Exception message: {exception.Message}"); @@ -187,7 +185,7 @@ private void HandleException(Exception exception, string name) sb.AppendLine($"Stack trace: {exception.InnerException.StackTrace}"); sb.AppendLine("---END Inner Exception"); } - + _xRayRecorder.AddMetadata ( nameSpace, @@ -209,7 +207,7 @@ private string GetNamespace() { return !string.IsNullOrWhiteSpace(_namespace) ? _namespace : _powertoolsConfigurations.Service; } - + /// /// Method that checks if tracing is disabled /// @@ -230,7 +228,7 @@ private bool TracingDisabled() return false; } - + /// /// Captures the response. /// @@ -250,16 +248,16 @@ private bool CaptureResponse() return false; } } - + /// /// Captures the error. /// /// true if tracing should capture errors, false otherwise. private bool CaptureError() { - if(TracingDisabled()) + if (TracingDisabled()) return false; - + switch (_captureMode) { case TracingCaptureMode.EnvironmentVariable: @@ -273,7 +271,7 @@ private bool CaptureError() return false; } } - + /// /// Resets static variables for test. /// diff --git a/libraries/src/AWS.Lambda.Powertools.Tracing/Internal/XRayRecorder.cs b/libraries/src/AWS.Lambda.Powertools.Tracing/Internal/XRayRecorder.cs index facfd39c..b19726b3 100644 --- a/libraries/src/AWS.Lambda.Powertools.Tracing/Internal/XRayRecorder.cs +++ b/libraries/src/AWS.Lambda.Powertools.Tracing/Internal/XRayRecorder.cs @@ -1,12 +1,12 @@ /* * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * + * * Licensed under the Apache License, Version 2.0 (the "License"). * You may not use this file except in compliance with the License. * A copy of the License is located at - * + * * http://aws.amazon.com/apache2.0 - * + * * or in the "license" file accompanying this file. This file 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 @@ -41,7 +41,8 @@ internal class XRayRecorder : IXRayRecorder /// Gets the instance. /// /// The instance. - public static IXRayRecorder Instance => _instance ??= new XRayRecorder(AWSXRayRecorder.Instance, PowertoolsConfigurations.Instance); + public static IXRayRecorder Instance => + _instance ??= new XRayRecorder(AWSXRayRecorder.Instance, PowertoolsConfigurations.Instance); public XRayRecorder(IAWSXRayRecorder awsxRayRecorder, IPowertoolsConfigurations powertoolsConfigurations) { @@ -56,7 +57,7 @@ public XRayRecorder(IAWSXRayRecorder awsxRayRecorder, IPowertoolsConfigurations /// Checks whether current execution is in AWS Lambda. /// /// Returns true if current execution is in AWS Lambda. - private static bool _isLambda; + private static bool _isLambda; /// /// Gets the emitter. @@ -119,7 +120,19 @@ public void AddMetadata(string nameSpace, string key, object value) public void EndSubsegment() { if (_isLambda) - _awsxRayRecorder.EndSubsegment(); + { + try + { + _awsxRayRecorder.EndSubsegment(); + } + catch (Exception e) + { + _awsxRayRecorder.AddException(e); + _awsxRayRecorder.MarkFault(); + _awsxRayRecorder.MarkError(); + _awsxRayRecorder.EndSubsegment(); + } + } } /// diff --git a/libraries/src/Directory.Packages.props b/libraries/src/Directory.Packages.props index d9c404bd..49aa51ca 100644 --- a/libraries/src/Directory.Packages.props +++ b/libraries/src/Directory.Packages.props @@ -9,13 +9,13 @@ - + - - + + diff --git a/libraries/tests/AWS.Lambda.Powertools.Tracing.Tests/Handlers/Handlers.cs b/libraries/tests/AWS.Lambda.Powertools.Tracing.Tests/Handlers/Handlers.cs index 19ecc681..385ce96c 100644 --- a/libraries/tests/AWS.Lambda.Powertools.Tracing.Tests/Handlers/Handlers.cs +++ b/libraries/tests/AWS.Lambda.Powertools.Tracing.Tests/Handlers/Handlers.cs @@ -83,4 +83,24 @@ public string[] HandleWithCaptureModeDisabled(bool exception = false) throw new Exception("Failed"); return new[] { "A", "B" }; } + + [Tracing(CaptureMode = TracingCaptureMode.ResponseAndError)] + public string DecoratedHandlerCaptureResponse() + { + DecoratedMethodCaptureDisabled(); + return "Hello World"; + } + + [Tracing(CaptureMode = TracingCaptureMode.Disabled)] + public string DecoratedMethodCaptureDisabled() + { + DecoratedMethodCaptureEnabled(); + return "DecoratedMethod Disabled"; + } + + [Tracing(CaptureMode = TracingCaptureMode.ResponseAndError)] + private string DecoratedMethodCaptureEnabled() + { + return "DecoratedMethod Enabled"; + } } \ No newline at end of file diff --git a/libraries/tests/AWS.Lambda.Powertools.Tracing.Tests/TracingAttributeTest.cs b/libraries/tests/AWS.Lambda.Powertools.Tracing.Tests/TracingAttributeTest.cs index b99c41a7..507ded2d 100644 --- a/libraries/tests/AWS.Lambda.Powertools.Tracing.Tests/TracingAttributeTest.cs +++ b/libraries/tests/AWS.Lambda.Powertools.Tracing.Tests/TracingAttributeTest.cs @@ -429,6 +429,69 @@ public void OnSuccess_WhenTracerCaptureModeIsDisabled_DoesNotCaptureResponse() Assert.Single(segment.Subsegments); Assert.False(subSegment.IsMetadataAdded); // does not add metadata } + + [Fact] + public void OnSuccess_WhenTracerCaptureResponseEnvironmentVariableIsFalse_ButDecoratorCapturesResponse() + { + // Arrange + Environment.SetEnvironmentVariable("LAMBDA_TASK_ROOT", "AWS"); + Environment.SetEnvironmentVariable("POWERTOOLS_SERVICE_NAME", "POWERTOOLS"); + Environment.SetEnvironmentVariable("POWERTOOLS_TRACER_CAPTURE_RESPONSE", "false"); + + // Act + var segment = AWSXRayRecorder.Instance.TraceContext.GetEntity(); + _handler.DecoratedHandlerCaptureResponse(); + var subSegment = segment.Subsegments[0]; + + // Assert + Assert.True(segment.IsSubsegmentsAdded); + Assert.Single(segment.Subsegments); + Assert.True(subSegment.IsMetadataAdded); + Assert.True(subSegment.Metadata.ContainsKey("POWERTOOLS")); + + var metadata = subSegment.Metadata["POWERTOOLS"]; + Assert.Equal("DecoratedHandlerCaptureResponse response", metadata.Keys.Cast().First()); + var handlerResponse = metadata.Values.Cast().First(); + Assert.Equal("Hello World", handlerResponse); + + var decoratedMethodSegmentDisabled = subSegment.Subsegments[0]; + Assert.False(decoratedMethodSegmentDisabled.IsMetadataAdded); + Assert.Equal("## DecoratedMethodCaptureDisabled", decoratedMethodSegmentDisabled.Name); + + var decoratedMethodSegmentEnabled = decoratedMethodSegmentDisabled.Subsegments[0]; + Assert.True(decoratedMethodSegmentEnabled.IsMetadataAdded); + + var decoratedMethodSegmentEnabledMetadata = decoratedMethodSegmentEnabled.Metadata["POWERTOOLS"]; + var decoratedMethodSegmentEnabledResponse = decoratedMethodSegmentEnabledMetadata.Values.Cast().First(); + Assert.Equal("DecoratedMethod Enabled", decoratedMethodSegmentEnabledResponse); + Assert.Equal("## DecoratedMethodCaptureEnabled", decoratedMethodSegmentEnabled.Name); + } + + [Fact] + public void OnSuccess_WhenTracerCaptureResponseEnvironmentVariableIsTrue_ButDecoratorCapturesResponseDisabled() + { + // Arrange + Environment.SetEnvironmentVariable("LAMBDA_TASK_ROOT", "AWS"); + Environment.SetEnvironmentVariable("POWERTOOLS_SERVICE_NAME", "POWERTOOLS"); + Environment.SetEnvironmentVariable("POWERTOOLS_TRACER_CAPTURE_RESPONSE", "true"); + + // Act + var segment = AWSXRayRecorder.Instance.TraceContext.GetEntity(); + _handler.DecoratedMethodCaptureDisabled(); + var subSegment = segment.Subsegments[0]; + + // Assert + Assert.True(segment.IsSubsegmentsAdded); + Assert.Single(segment.Subsegments); + Assert.False(subSegment.IsMetadataAdded); + + var decoratedMethodSegmentEnabled = subSegment.Subsegments[0]; + var metadata = decoratedMethodSegmentEnabled.Metadata["POWERTOOLS"]; + Assert.True(decoratedMethodSegmentEnabled.IsMetadataAdded); + var decoratedMethodSegmentEnabledResponse = metadata.Values.Cast().First(); + Assert.Equal("DecoratedMethod Enabled", decoratedMethodSegmentEnabledResponse); + Assert.Equal("## DecoratedMethodCaptureEnabled", decoratedMethodSegmentEnabled.Name); + } #endregion diff --git a/libraries/tests/AWS.Lambda.Powertools.Tracing.Tests/XRayRecorderTests.cs b/libraries/tests/AWS.Lambda.Powertools.Tracing.Tests/XRayRecorderTests.cs index a332bbf1..68e32d00 100644 --- a/libraries/tests/AWS.Lambda.Powertools.Tracing.Tests/XRayRecorderTests.cs +++ b/libraries/tests/AWS.Lambda.Powertools.Tracing.Tests/XRayRecorderTests.cs @@ -126,7 +126,7 @@ public void Tracing_Add_Metadata() // Assert awsXray.Received(1).AddMetadata("nameSpace", "key", "value"); } - + [Fact] public void Tracing_End_Subsegment() { @@ -144,6 +144,36 @@ public void Tracing_End_Subsegment() // Assert awsXray.Received(1).EndSubsegment(); } + + [Fact] + public void Tracing_End_Subsegment_Failed_Should_Cath_And_AddException() + { + // Arrange + var conf = Substitute.For(); + conf.IsLambdaEnvironment.Returns(true); + + var awsXray = Substitute.For(); + var fail = true; + var exception = new Exception("Oops, something went wrong"); + + awsXray.When(x=> x.EndSubsegment()).Do(x => + { + if (fail) + { + fail = false; + throw exception; + } + }); + + // Act + var tracing = new XRayRecorder(awsXray, conf); + + tracing.EndSubsegment(); + + // Assert + awsXray.Received(2).EndSubsegment(); + awsXray.Received(1).AddException(exception); + } [Fact] public void Tracing_Get_Entity_In_Lambda_Environment() From 5a4dd5676b3cb1dd2a38f84a7bbb94316e3cf0e8 Mon Sep 17 00:00:00 2001 From: Henrique <999396+hjgraca@users.noreply.github.com> Date: Wed, 21 Aug 2024 15:33:16 +0100 Subject: [PATCH 2/2] on exception clear Entity and create a new Error subsegment. update tests --- .../Internal/XRayRecorder.cs | 30 +++++++++++-------- .../XRayRecorderTests.cs | 4 ++- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/libraries/src/AWS.Lambda.Powertools.Tracing/Internal/XRayRecorder.cs b/libraries/src/AWS.Lambda.Powertools.Tracing/Internal/XRayRecorder.cs index b19726b3..e192036e 100644 --- a/libraries/src/AWS.Lambda.Powertools.Tracing/Internal/XRayRecorder.cs +++ b/libraries/src/AWS.Lambda.Powertools.Tracing/Internal/XRayRecorder.cs @@ -113,25 +113,29 @@ public void AddMetadata(string nameSpace, string key, object value) if (_isLambda) _awsxRayRecorder.AddMetadata(nameSpace, key, value); } - + /// /// Ends the subsegment. /// public void EndSubsegment() { - if (_isLambda) + if (!_isLambda) return; + try + { + _awsxRayRecorder.EndSubsegment(); + } + catch (Exception e) { - try - { - _awsxRayRecorder.EndSubsegment(); - } - catch (Exception e) - { - _awsxRayRecorder.AddException(e); - _awsxRayRecorder.MarkFault(); - _awsxRayRecorder.MarkError(); - _awsxRayRecorder.EndSubsegment(); - } + // if it fails at this stage the data is lost + // so lets create a new subsegment with the error + + Console.WriteLine("Error in Tracing utility - see Exceptions tab in Cloudwatch Traces"); + + _awsxRayRecorder.TraceContext.ClearEntity(); + _awsxRayRecorder.BeginSubsegment("Error in Tracing utility - see Exceptions tab"); + _awsxRayRecorder.AddException(e); + _awsxRayRecorder.MarkError(); + _awsxRayRecorder.EndSubsegment(); } } diff --git a/libraries/tests/AWS.Lambda.Powertools.Tracing.Tests/XRayRecorderTests.cs b/libraries/tests/AWS.Lambda.Powertools.Tracing.Tests/XRayRecorderTests.cs index 68e32d00..038703d4 100644 --- a/libraries/tests/AWS.Lambda.Powertools.Tracing.Tests/XRayRecorderTests.cs +++ b/libraries/tests/AWS.Lambda.Powertools.Tracing.Tests/XRayRecorderTests.cs @@ -171,8 +171,10 @@ public void Tracing_End_Subsegment_Failed_Should_Cath_And_AddException() tracing.EndSubsegment(); // Assert - awsXray.Received(2).EndSubsegment(); + awsXray.Received(1).BeginSubsegment("Error in Tracing utility - see Exceptions tab"); + awsXray.Received(1).MarkError(); awsXray.Received(1).AddException(exception); + awsXray.Received(2).EndSubsegment(); } [Fact]