From 4434a8c5a869c80ca68a0ce6720da1125905b789 Mon Sep 17 00:00:00 2001 From: chynesNR Date: Thu, 26 Sep 2024 14:03:36 -0700 Subject: [PATCH 01/11] First pass at instrumentation --- .../Attributes/AttributeDefinitionService.cs | 15 +++ .../Core/Transactions/NoOpTransaction.cs | 5 + .../Agent/Core/Transactions/Transaction.cs | 12 +++ .../Api/ITransaction.cs | 2 + .../Helpers/AwsSdk.cs | 60 ++++++++++++ .../Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs | 23 +++++ .../AwsSdk/LambdaInvokeRequestHandler.cs | 97 +++++++++++++++++++ .../Helpers/AwsSdkHelperTests.cs | 36 +++++++ 8 files changed, 250 insertions(+) create mode 100644 src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs create mode 100644 src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs create mode 100644 tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Helpers/AwsSdkHelperTests.cs diff --git a/src/Agent/NewRelic/Agent/Core/Attributes/AttributeDefinitionService.cs b/src/Agent/NewRelic/Agent/Core/Attributes/AttributeDefinitionService.cs index c51ea26b61..9f1a7151b8 100644 --- a/src/Agent/NewRelic/Agent/Core/Attributes/AttributeDefinitionService.cs +++ b/src/Agent/NewRelic/Agent/Core/Attributes/AttributeDefinitionService.cs @@ -126,6 +126,7 @@ public interface IAttributeDefinitions AttributeDefinition GetLambdaAttribute(string name); AttributeDefinition GetFaasAttribute(string name); + AttributeDefinition GetCloudSdkAttribute(string name); AttributeDefinition GetRequestParameterAttribute(string paramName); @@ -190,6 +191,7 @@ public AttributeDefinitions(IAttributeFilter attribFilter) private readonly ConcurrentDictionary> _requestHeadersAttributes = new ConcurrentDictionary>(); private readonly ConcurrentDictionary> _lambdaAttributes = new ConcurrentDictionary>(); private readonly ConcurrentDictionary> _faasAttributes = new(); + private readonly ConcurrentDictionary> _cloudSdkAttributes = new(); private readonly ConcurrentDictionary> _typeAttributes = new ConcurrentDictionary>(); @@ -281,6 +283,19 @@ public AttributeDefinition GetFaasAttribute(string name) } + private AttributeDefinition CreateCloudSdkAttribute(string attribName) + { + return AttributeDefinitionBuilder + .Create(attribName, AttributeClassification.AgentAttributes) + .AppliesTo(AttributeDestinations.TransactionTrace) + .AppliesTo(AttributeDestinations.SpanEvent) + .Build(_attribFilter); + } + + public AttributeDefinition GetCloudSdkAttribute(string name) + { + return _cloudSdkAttributes.GetOrAdd(name, CreateCloudSdkAttribute); + } public AttributeDefinition GetCustomAttributeForTransaction(string name) { return _trxCustomAttributes.GetOrAdd(name, CreateCustomAttributeForTransaction); diff --git a/src/Agent/NewRelic/Agent/Core/Transactions/NoOpTransaction.cs b/src/Agent/NewRelic/Agent/Core/Transactions/NoOpTransaction.cs index 404cbac7f4..91c3723e52 100644 --- a/src/Agent/NewRelic/Agent/Core/Transactions/NoOpTransaction.cs +++ b/src/Agent/NewRelic/Agent/Core/Transactions/NoOpTransaction.cs @@ -331,5 +331,10 @@ public void AddFaasAttribute(string name, object value) { return; } + + public void AddCloudSdkAttribute(string name, object value) + { + return; + } } } diff --git a/src/Agent/NewRelic/Agent/Core/Transactions/Transaction.cs b/src/Agent/NewRelic/Agent/Core/Transactions/Transaction.cs index 9cbe293901..3706e795d0 100644 --- a/src/Agent/NewRelic/Agent/Core/Transactions/Transaction.cs +++ b/src/Agent/NewRelic/Agent/Core/Transactions/Transaction.cs @@ -1393,5 +1393,17 @@ public void AddFaasAttribute(string name, object value) var faasAttrib = _attribDefs.GetFaasAttribute(name); TransactionMetadata.UserAndRequestAttributes.TrySetValue(faasAttrib, value); } + + public void AddCloudSdkAttribute(string name, object value) + { + if (string.IsNullOrWhiteSpace(name)) + { + Log.Debug($"AddCloudSdkAttribute - Unable to set Cloud value on transaction because the key is null/empty"); + return; + } + + var cloudAttrib = _attribDefs.GetCloudSdkAttribute(name); + TransactionMetadata.UserAndRequestAttributes.TrySetValue(cloudAttrib, value); + } } } diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Api/ITransaction.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Api/ITransaction.cs index ea0f22e041..333ea16e6b 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Api/ITransaction.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Api/ITransaction.cs @@ -311,5 +311,7 @@ ISegment StartMessageBrokerSegment(MethodCall methodCall, MessageBrokerDestinati void AddLambdaAttribute(string name, object value); void AddFaasAttribute(string name, object value); + + void AddCloudSdkAttribute(string name, object value); } } diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs new file mode 100644 index 0000000000..f473533221 --- /dev/null +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs @@ -0,0 +1,60 @@ +// Copyright 2020 New Relic, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +using NewRelic.Agent.Api; + +namespace NewRelic.Agent.Extensions.Helpers +{ + public static class AwsSdkHelpers + { + public static string ConstructArn(IAgent agent, string invocationName, string region, string accountId) + { + if (invocationName.StartsWith("arn:")) + { + if (invocationName.StartsWith("arn:aws:lambda:")) + { + return invocationName; + } + agent?.Logger.Debug($"Unable to parse function name '{invocationName}'"); + return null; + } + var segments = invocationName.Split(':'); + string functionName; + + if ((segments.Length == 1) || (segments.Length == 2)) + { + // 'myfunction' or 'myfunction:alias' + // Need account ID to reconstruct ARN + if (string.IsNullOrEmpty(accountId)) + { + agent?.Logger.Debug($"Need account ID in order to resolve function '{invocationName}'"); + return null; + } + functionName = invocationName; + } + else if (segments.Length == 3) + { + // 123456789012:function:my-function' + accountId = segments[0]; + functionName = segments[2]; + } + else if (segments.Length == 4) + { + // 123456789012:function:my-function:myalias + accountId = segments[0]; + functionName = $"{segments[2]}:{segments[3]}"; + } + else + { + agent?.Logger.Debug($"Unable to parse function name '{invocationName}'."); + return null; + } + if (string.IsNullOrEmpty(region)) + { + agent?.Logger.Debug($"Need region in order to resolve function '{invocationName}'"); + return null; + } + return $"arn:aws:lambda:{region}:{accountId}:function:{functionName}"; + } + } +} diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs index bc772687c1..f0cf737f9d 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs @@ -1,7 +1,9 @@ // Copyright 2020 New Relic, Inc. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +using System; using System.Collections.Generic; +using System.Threading.Tasks; using NewRelic.Agent.Api; using NewRelic.Agent.Extensions.Providers.Wrapper; @@ -19,6 +21,23 @@ public CanWrapResponse CanWrap(InstrumentedMethodInfo methodInfo) return new CanWrapResponse(WrapperName.Equals(methodInfo.RequestedWrapperName)); } + private string GetRegion(IAgent agent, dynamic requestContext) + { + try + { + var clientconfig = requestContext.ClientConfig; + var regionEndpoint = clientconfig.RegionEndpoint; + var systemName = regionEndpoint.SystemName; + return systemName; + } + catch (Exception e) + { + agent.Logger.Debug(e, $"AwsSdkPipelineWrapper: Unable to get region from requestContext."); + } + + return ""; + } + public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall instrumentedMethodCall, IAgent agent, ITransaction transaction) { // Get the IExecutionContext (the only parameter) @@ -54,6 +73,10 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins { return SQSRequestHandler.HandleSQSRequest(instrumentedMethodCall, agent, transaction, request, isAsync, executionContext); } + else if (requestType == "Amazon.Lambda.Model.InvokeRequest") + { + return LambdaInvokeRequestHandler.HandleInvokeRequest(instrumentedMethodCall, agent, transaction, request, isAsync, GetRegion(agent, requestContext)); + } if (_unsupportedRequestTypes.Add(requestType)) // log once per unsupported request type agent.Logger.Debug($"AwsSdkPipelineWrapper: Unsupported request type: {requestType}. Returning NoOp delegate."); diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs new file mode 100644 index 0000000000..66405f3eca --- /dev/null +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs @@ -0,0 +1,97 @@ +// Copyright 2020 New Relic, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +using System.Collections.Concurrent; +using System; +using System.Threading.Tasks; +using NewRelic.Agent.Api; +using NewRelic.Agent.Extensions.Providers.Wrapper; +using NewRelic.Reflection; +using NewRelic.Agent.Extensions.Helpers; + +namespace NewRelic.Providers.Wrapper.AwsSdk +{ + internal static class LambdaInvokeRequestHandler + { + private static ConcurrentDictionary> _getResultFromGenericTask = new(); + + private static object GetTaskResult(object task) + { + if (((Task)task).IsFaulted) + { + return null; + } + + var getResponse = _getResultFromGenericTask.GetOrAdd(task.GetType(), t => VisibilityBypasser.Instance.GeneratePropertyAccessor(t, "Result")); + return getResponse(task); + } + + private static void SetRequestIdIfAvailable(IAgent agent, ITransaction transaction, dynamic response) + { + try + { + dynamic metadata = response.ResponseMetadata; + string requestId = metadata.RequestId; + transaction.AddCloudSdkAttribute("aws.requestId", requestId); + } + catch (Exception e) + { + agent.Logger.Debug(e, "Unable to get RequestId from response metadata."); + } + } + + public static AfterWrappedMethodDelegate HandleInvokeRequest(InstrumentedMethodCall instrumentedMethodCall, IAgent agent, ITransaction transaction, dynamic request, bool isAsync, string region) + { + string functionName = request.FunctionName; + string qualifier = request.Qualifier; + if (!string.IsNullOrEmpty(qualifier) && !functionName.EndsWith(qualifier)) + { + functionName = $"{functionName}:{qualifier}"; + } + string arn = AwsSdkHelpers.ConstructArn(agent, functionName, region, ""); + var segment = transaction.StartTransactionSegment(instrumentedMethodCall.MethodCall, "InvokeRequest"); + + transaction.AddCloudSdkAttribute("cloud.platform", "aws_lambda"); + transaction.AddCloudSdkAttribute("aws.operation", "InvokeRequest"); + transaction.AddCloudSdkAttribute("aws.region", region); + + + if (!string.IsNullOrEmpty(arn)) + { + transaction.AddCloudSdkAttribute("cloud.resource_id", arn); + } + + if (isAsync) + { + return Delegates.GetAsyncDelegateFor(agent, segment, true, InvokeTryProcessResponse, TaskContinuationOptions.ExecuteSynchronously); + + void InvokeTryProcessResponse(Task responseTask) + { + try + { + if (responseTask.Status == TaskStatus.Faulted) + { + transaction.NoticeError(responseTask.Exception); + } + SetRequestIdIfAvailable(agent, transaction, GetTaskResult(responseTask)); + } + finally + { + segment?.End(); + } + } + } + else + { + return Delegates.GetDelegateFor( + onFailure: ex => segment.End(ex), + onSuccess: response => + { + SetRequestIdIfAvailable(agent, transaction, response); + segment.End(); + } + ); + } + } + } +} diff --git a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Helpers/AwsSdkHelperTests.cs b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Helpers/AwsSdkHelperTests.cs new file mode 100644 index 0000000000..322210b186 --- /dev/null +++ b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Helpers/AwsSdkHelperTests.cs @@ -0,0 +1,36 @@ +// Copyright 2020 New Relic, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +using NUnit.Framework; +using NewRelic.Agent.Extensions.Helpers; + +namespace Agent.Extensions.Tests.Helpers +{ + public class AwsSdkHelperTests + { + [Test] + [TestCase("myfunction", "us-west-2", "123456789012", "arn:aws:lambda:us-west-2:123456789012:function:myfunction")] + [TestCase("myfunction", "us-west-2", "", null)] + [TestCase("myfunction", "", "123456789012", null)] + [TestCase("myfunction:alias", "us-west-2", "123456789012", "arn:aws:lambda:us-west-2:123456789012:function:myfunction:alias")] + [TestCase("myfunction:alias", "us-west-2", "", null)] + [TestCase("123456789012:function:my-function", "us-west-2", "123456789012", "arn:aws:lambda:us-west-2:123456789012:function:my-function")] + [TestCase("123456789012:function:my-function:myalias", "us-west-2", "123456789012", "arn:aws:lambda:us-west-2:123456789012:function:my-function:myalias")] + [TestCase("123456789012:function:my-function:myalias:extra", "us-west-2", "123456789012", null)] + [TestCase("123456789012:function:my-function:myalias:extra:lots:of:extra:way:too:many", "us-west-2", "123456789012", null)] + [TestCase("arn:aws:", "us-west-2", "123456789012", null)] + [TestCase("arn:aws:lambda:us-west-2:123456789012:function:myfunction", "us-west-2", "123456789012", "arn:aws:lambda:us-west-2:123456789012:function:myfunction")] + [TestCase("arn:aws:lambda:us-west-2:123456789012:function:myfunction", "us-west-2", "", "arn:aws:lambda:us-west-2:123456789012:function:myfunction")] + [TestCase("arn:aws:lambda:us-west-2:123456789012:function:myfunction", "", "123456789012", "arn:aws:lambda:us-west-2:123456789012:function:myfunction")] + [TestCase("myfunction", "us-east-1", "987654321098", "arn:aws:lambda:us-east-1:987654321098:function:myfunction")] + [TestCase("myfunction:prod", "eu-west-1", "111122223333", "arn:aws:lambda:eu-west-1:111122223333:function:myfunction:prod")] + [TestCase("my-function", "ap-southeast-1", "444455556666", "arn:aws:lambda:ap-southeast-1:444455556666:function:my-function")] + [TestCase("my-function:beta", "ca-central-1", "777788889999", "arn:aws:lambda:ca-central-1:777788889999:function:my-function:beta")] + [TestCase("arn:aws:lambda:eu-central-1:222233334444:function:myfunction", "eu-central-1", "222233334444", "arn:aws:lambda:eu-central-1:222233334444:function:myfunction")] + public void ConstructArn(string name, string region, string accountId, string arn) + { + var constructedArn = AwsSdkHelpers.ConstructArn(null, name, region, accountId); + Assert.That(constructedArn, Is.EqualTo(arn), "Did not get expected ARN"); + } + } +} From 5475b4935ed5d696bdb4d799088a8470e1c29cd7 Mon Sep 17 00:00:00 2001 From: chynesNR Date: Mon, 30 Sep 2024 09:13:11 -0700 Subject: [PATCH 02/11] First pass at integration tests --- .../AwsSdk/InvokeLambdaTests.cs | 126 ++++++++++++++++++ .../MFALatestPackages.csproj | 5 +- .../MultiFunctionApplicationHelpers.csproj | 8 ++ .../AwsSdk/InvokeLambdaExerciser.cs | 93 +++++++++++++ 4 files changed, 231 insertions(+), 1 deletion(-) create mode 100644 tests/Agent/IntegrationTests/IntegrationTests/AwsSdk/InvokeLambdaTests.cs create mode 100644 tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/AwsSdk/InvokeLambdaExerciser.cs diff --git a/tests/Agent/IntegrationTests/IntegrationTests/AwsSdk/InvokeLambdaTests.cs b/tests/Agent/IntegrationTests/IntegrationTests/AwsSdk/InvokeLambdaTests.cs new file mode 100644 index 0000000000..3c959da12f --- /dev/null +++ b/tests/Agent/IntegrationTests/IntegrationTests/AwsSdk/InvokeLambdaTests.cs @@ -0,0 +1,126 @@ +// Copyright 2020 New Relic, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +using System; +using System.Collections.Generic; +using System.Drawing; +using System.Linq; +using NewRelic.Agent.IntegrationTestHelpers; +using NewRelic.Agent.IntegrationTestHelpers.RemoteServiceFixtures; +using Xunit; +using Xunit.Abstractions; + +namespace NewRelic.Agent.IntegrationTests.AwsSdk +{ + public abstract class InvokeLambdaTestBase : NewRelicIntegrationTest + where TFixture : ConsoleDynamicMethodFixture + { + private readonly TFixture _fixture; + private readonly string _function; + private readonly string _qualifier; + private readonly string _arn; + private readonly bool _isAsync; + + public InvokeLambdaTestBase(TFixture fixture, ITestOutputHelper output, bool async, string function, string qualifier, string arn) : base(fixture) + { + _function = function; + _qualifier = qualifier; + _arn = arn; + _isAsync = async; + + _fixture = fixture; + _fixture.SetTimeout(TimeSpan.FromMinutes(20)); + + _fixture.TestLogger = output; + _fixture.AddActions( + setupConfiguration: () => + { + new NewRelicConfigModifier(fixture.DestinationNewRelicConfigFilePath) + .ForceTransactionTraces() + .SetLogLevel("finest"); + }, + exerciseApplication: () => + { + _fixture.AgentLog.WaitForLogLines(AgentLogBase.TransactionTransformCompletedLogLineRegex, TimeSpan.FromMinutes(20), 2); + } + ); + + if (async) + { + _fixture.AddCommand($"InvokeLambdaExerciser InvokeLambdaAsync {_function}:{_qualifier} \"fakepayload\""); + _fixture.AddCommand($"InvokeLambdaExerciser InvokeLambdaAsyncWithQualifier {_function} {_qualifier} \"fakepayload\""); + } + else + { + _fixture.AddCommand($"InvokeLambdaExerciser InvokeLambdaSync {_function} \"fakepayload\""); + } + _fixture.AddCommand($"InvokeLambdaExerciser InvokeLambdaSync fakefunction fakepayload"); + + _fixture.Initialize(); + } + + [Fact] + public void InvokeLambda() + { + var metrics = _fixture.AgentLog.GetMetrics().ToList(); + + var expectedMetrics = new List + { + }; + + Assertions.MetricsExist(expectedMetrics, metrics); + + var transactions = _fixture.AgentLog.GetTransactionEvents(); + + Assert.NotNull(transactions); + + var spans = _fixture.AgentLog.GetSpanEvents() + .Where(e => e.AgentAttributes.ContainsKey("cloud.resource_id")) + .ToList(); + + Assert.Equal(_isAsync ? 2 : 1, spans.Count); + + foreach (var span in spans) + { + Assert.Equal(_arn, span.AgentAttributes["cloud.resource_id"]); + Assert.Equal("aws_lambda", span.AgentAttributes["cloud.platform"]); + Assert.Equal("InvokeRequest", span.AgentAttributes["aws.operation"]); + Assert.Equal("us-west-2", span.AgentAttributes["aws.region"]); + } + } + } + [NetFrameworkTest] + public class InvokeLambdaTest_Sync_FW462 : InvokeLambdaTestBase + { + public InvokeLambdaTest_Sync_FW462(ConsoleDynamicMethodFixtureFW462 fixture, ITestOutputHelper output) + : base(fixture, output, false, "342444490463:NotARealFunction", null, "arn:aws:lambda:us-west-2:342444490463:function:NotARealFunction") + { + } + } + [NetFrameworkTest] + public class InvokeLambdaTest_Sync_FWLatest : InvokeLambdaTestBase + { + public InvokeLambdaTest_Sync_FWLatest(ConsoleDynamicMethodFixtureFWLatest fixture, ITestOutputHelper output) + : base(fixture, output, false, "342444490463:NotARealFunction", null, "arn:aws:lambda:us-west-2:342444490463:function:NotARealFunction") + { + } + } + [NetCoreTest] + public class InvokeLambdaTest_Async_CoreOldest : InvokeLambdaTestBase + { + public InvokeLambdaTest_Async_CoreOldest(ConsoleDynamicMethodFixtureCoreOldest fixture, ITestOutputHelper output) + : base(fixture, output, true, "342444490463:NotARealFunction", "NotARealAlias", "arn:aws:lambda:us-west-2:342444490463:function:NotARealFunction:NotARealAlias") + { + } + } + + [NetCoreTest] + public class InvokeLambdaTest_Async_CoreLatest : InvokeLambdaTestBase + { + public InvokeLambdaTest_Async_CoreLatest(ConsoleDynamicMethodFixtureCoreLatest fixture, ITestOutputHelper output) + : base(fixture, output, true, "342444490463:NotARealFunction", "NotARealAlias", "arn:aws:lambda:us-west-2:342444490463:function:NotARealFunction:NotARealAlias") + { + } + } + +} diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj b/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj index fa488c6a6f..edf37575ac 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj @@ -7,7 +7,10 @@ - + + + + diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj index 13c4870720..57f498b5e1 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj @@ -272,6 +272,14 @@ + + + + + + + + diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/AwsSdk/InvokeLambdaExerciser.cs b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/AwsSdk/InvokeLambdaExerciser.cs new file mode 100644 index 0000000000..e02532ab8e --- /dev/null +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/AwsSdk/InvokeLambdaExerciser.cs @@ -0,0 +1,93 @@ +// Copyright 2020 New Relic, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +using System.IO; +using System.Security.AccessControl; +using System.Threading.Tasks; +using Amazon; +using NewRelic.Agent.IntegrationTests.Shared.ReflectionHelpers; +using NewRelic.Api.Agent; + +namespace MultiFunctionApplicationHelpers.NetStandardLibraries.AwsSdk +{ + [Library] + public class InvokeLambdaExerciser + { + [LibraryMethod] + [Transaction] + public void InvokeLambdaSync(string function, string payload) + { +#if NETFRAMEWORK + var client = new Amazon.Lambda.AmazonLambdaClient(RegionEndpoint.USWest2); + var request = new Amazon.Lambda.Model.InvokeRequest + { + FunctionName = function, + Payload = payload + }; + + // Note that we aren't invoking a lambda that exists! This is guaranteed to fail, but all we care + // about is that the agent is able to instrument the call. + try + { + var response = client.Invoke(request); + } + catch + { + } +#endif + } + + [LibraryMethod] + [Transaction] + public async Task InvokeLambdaAsync(string function, string payload) + { + var client = new Amazon.Lambda.AmazonLambdaClient(RegionEndpoint.USWest2); + var request = new Amazon.Lambda.Model.InvokeRequest + { + FunctionName = function, + Payload = payload + }; + + // Note that we aren't invoking a lambda that exists! This is guaranteed to fail, but all we care + // about is that the agent is able to instrument the call. + try + { + var response = await client.InvokeAsync(request); + MemoryStream stream = response.Payload; + string returnValue = System.Text.Encoding.UTF8.GetString(stream.ToArray()); + return returnValue; + } + catch + { + } + return null; + } + + [LibraryMethod] + [Transaction] + public async Task InvokeLambdaAsyncWithQualifier(string function, string qualifier, string payload) + { + var client = new Amazon.Lambda.AmazonLambdaClient(RegionEndpoint.USWest2); + var request = new Amazon.Lambda.Model.InvokeRequest + { + FunctionName = function, + Qualifier = qualifier, + Payload = payload + }; + + // Note that we aren't invoking a lambda that exists! This is guaranteed to fail, but all we care + // about is that the agent is able to instrument the call. + try + { + var response = await client.InvokeAsync(request); + MemoryStream stream = response.Payload; + string returnValue = System.Text.Encoding.UTF8.GetString(stream.ToArray()); + return returnValue; + } + catch + { + } + return null; + } + } +} From 3d213e6ed5ce3b0c37a46663a4bd0dd38f867661 Mon Sep 17 00:00:00 2001 From: chynesNR Date: Tue, 8 Oct 2024 08:32:18 -0700 Subject: [PATCH 03/11] Update ARN parsing logic --- .../Helpers/AwsSdk.cs | 93 +++++++++++++------ 1 file changed, 65 insertions(+), 28 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs index f473533221..4a66b9079e 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs @@ -1,59 +1,96 @@ // Copyright 2020 New Relic, Inc. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +using System.Collections.Generic; +using System.Linq; +using System.Text.RegularExpressions; using NewRelic.Agent.Api; namespace NewRelic.Agent.Extensions.Helpers { public static class AwsSdkHelpers { + private static Regex RegionRegex = new Regex(@"^[a-z]{2}((-gov)|(-iso([a-z]?)))?-[a-z]+-\d{1}$", RegexOptions.Compiled); + private static bool LooksLikeARegion(string text) => RegionRegex.IsMatch(text); + private static bool LooksLikeAnAccountId(string text) => (text.Length == 12) && text.All(c => c >= '0' && c <= '9'); + // Only log ARNs we can't parse once + private static HashSet BadInvocations = new HashSet(); + + // This is the full regex pattern for an ARN: + // (arn:(aws[a-zA-Z-]*)?:lambda:)?([a-z]{2}((-gov)|(-iso([a-z]?)))?-[a-z]+-\d{1}:)?(\d{12}:)?(function:)?([a-zA-Z0-9-_\.]+)(:(\$LATEST|[a-zA-Z0-9-_]+))? + + // If it's a full ARN, it has to start with 'arn:' + // A partial ARN can contain up to 5 segments separated by ':' + // 1. Region + // 2. Account ID + // 3. 'function' (fixed string) + // 4. Function name + // 5. Alias or version + // Only the function name is required, the reset are all optional. e.g. you could have region and function name and nothing else + + // Note that this will not catch functions where the name also looks like a region or account ID public static string ConstructArn(IAgent agent, string invocationName, string region, string accountId) { if (invocationName.StartsWith("arn:")) { - if (invocationName.StartsWith("arn:aws:lambda:")) - { - return invocationName; - } - agent?.Logger.Debug($"Unable to parse function name '{invocationName}'"); - return null; + return invocationName; } var segments = invocationName.Split(':'); - string functionName; + string functionName = null; + string alias = null; - if ((segments.Length == 1) || (segments.Length == 2)) + foreach (var segment in segments) { - // 'myfunction' or 'myfunction:alias' - // Need account ID to reconstruct ARN - if (string.IsNullOrEmpty(accountId)) + if (LooksLikeARegion(segment) && string.IsNullOrEmpty(region)) { - agent?.Logger.Debug($"Need account ID in order to resolve function '{invocationName}'"); + region = segment; + } + else if (LooksLikeAnAccountId(segment) && string.IsNullOrEmpty(accountId)) + { + accountId = segment; + } + else if (segment == "function") + { + continue; + } + else if (functionName == null) + { + functionName = segment; + } + else if (alias == null) + { + alias = segment; + } + else + { + if (BadInvocations.Add(invocationName)) + { + agent?.Logger.Debug($"Unable to parse function name '{invocationName}'"); + } return null; } - functionName = invocationName; - } - else if (segments.Length == 3) - { - // 123456789012:function:my-function' - accountId = segments[0]; - functionName = segments[2]; } - else if (segments.Length == 4) - { - // 123456789012:function:my-function:myalias - accountId = segments[0]; - functionName = $"{segments[2]}:{segments[3]}"; - } - else + + if (string.IsNullOrEmpty(accountId)) { - agent?.Logger.Debug($"Unable to parse function name '{invocationName}'."); + if (BadInvocations.Add(invocationName)) + { + agent?.Logger.Debug($"Need account ID in order to resolve function '{invocationName}'"); + } return null; } if (string.IsNullOrEmpty(region)) { - agent?.Logger.Debug($"Need region in order to resolve function '{invocationName}'"); + if (BadInvocations.Add(invocationName)) + { + agent?.Logger.Debug($"Need region in order to resolve function '{invocationName}'"); + } return null; } + if (!string.IsNullOrEmpty(alias)) + { + return $"arn:aws:lambda:{region}:{accountId}:function:{functionName}:{alias}"; + } return $"arn:aws:lambda:{region}:{accountId}:function:{functionName}"; } } From 7c33292808f731bb7bdf26f76ef1f9eaebd5a990 Mon Sep 17 00:00:00 2001 From: chynesNR Date: Tue, 8 Oct 2024 11:34:46 -0700 Subject: [PATCH 04/11] Updating ARN parsing logic --- .../Helpers/AwsSdk.cs | 43 ++++++++++++++++--- .../Helpers/AwsSdkHelperTests.cs | 16 ++++++- 2 files changed, 52 insertions(+), 7 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs index 4a66b9079e..716e18b5b9 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs @@ -27,8 +27,6 @@ public static class AwsSdkHelpers // 4. Function name // 5. Alias or version // Only the function name is required, the reset are all optional. e.g. you could have region and function name and nothing else - - // Note that this will not catch functions where the name also looks like a region or account ID public static string ConstructArn(IAgent agent, string invocationName, string region, string accountId) { if (invocationName.StartsWith("arn:")) @@ -38,16 +36,33 @@ public static string ConstructArn(IAgent agent, string invocationName, string re var segments = invocationName.Split(':'); string functionName = null; string alias = null; + string fallback = null; foreach (var segment in segments) { - if (LooksLikeARegion(segment) && string.IsNullOrEmpty(region)) + if (LooksLikeARegion(segment)) { - region = segment; + if (string.IsNullOrEmpty(region)) + { + region = segment; + } + else + { + fallback = segment; + } + continue; } - else if (LooksLikeAnAccountId(segment) && string.IsNullOrEmpty(accountId)) + else if (LooksLikeAnAccountId(segment)) { - accountId = segment; + if (string.IsNullOrEmpty(accountId)) + { + accountId = segment; + } + else + { + fallback = segment; + } + continue; } else if (segment == "function") { @@ -71,6 +86,22 @@ public static string ConstructArn(IAgent agent, string invocationName, string re } } + if (string.IsNullOrEmpty(functionName)) + { + if (!string.IsNullOrEmpty(fallback)) + { + functionName = fallback; + } + else + { + if (BadInvocations.Add(invocationName)) + { + agent?.Logger.Debug($"Unable to parse function name '{invocationName}'"); + } + return null; + } + } + if (string.IsNullOrEmpty(accountId)) { if (BadInvocations.Add(invocationName)) diff --git a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Helpers/AwsSdkHelperTests.cs b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Helpers/AwsSdkHelperTests.cs index 322210b186..1382e0d093 100644 --- a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Helpers/AwsSdkHelperTests.cs +++ b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Helpers/AwsSdkHelperTests.cs @@ -18,7 +18,7 @@ public class AwsSdkHelperTests [TestCase("123456789012:function:my-function:myalias", "us-west-2", "123456789012", "arn:aws:lambda:us-west-2:123456789012:function:my-function:myalias")] [TestCase("123456789012:function:my-function:myalias:extra", "us-west-2", "123456789012", null)] [TestCase("123456789012:function:my-function:myalias:extra:lots:of:extra:way:too:many", "us-west-2", "123456789012", null)] - [TestCase("arn:aws:", "us-west-2", "123456789012", null)] + [TestCase("arn:aws:", "us-west-2", "123456789012", "arn:aws:")] [TestCase("arn:aws:lambda:us-west-2:123456789012:function:myfunction", "us-west-2", "123456789012", "arn:aws:lambda:us-west-2:123456789012:function:myfunction")] [TestCase("arn:aws:lambda:us-west-2:123456789012:function:myfunction", "us-west-2", "", "arn:aws:lambda:us-west-2:123456789012:function:myfunction")] [TestCase("arn:aws:lambda:us-west-2:123456789012:function:myfunction", "", "123456789012", "arn:aws:lambda:us-west-2:123456789012:function:myfunction")] @@ -27,6 +27,20 @@ public class AwsSdkHelperTests [TestCase("my-function", "ap-southeast-1", "444455556666", "arn:aws:lambda:ap-southeast-1:444455556666:function:my-function")] [TestCase("my-function:beta", "ca-central-1", "777788889999", "arn:aws:lambda:ca-central-1:777788889999:function:my-function:beta")] [TestCase("arn:aws:lambda:eu-central-1:222233334444:function:myfunction", "eu-central-1", "222233334444", "arn:aws:lambda:eu-central-1:222233334444:function:myfunction")] + [TestCase("us-west-2:myfunction", null, "123456789012", "arn:aws:lambda:us-west-2:123456789012:function:myfunction")] + [TestCase("us-west-2:myfunction", "us-west-2", "123456789012", "arn:aws:lambda:us-west-2:123456789012:function:myfunction")] + [TestCase("us-west-2:myfunction", "us-west-2", "", null)] + [TestCase("us-west-2:myfunction:alias", "us-west-2", "123456789012", "arn:aws:lambda:us-west-2:123456789012:function:myfunction:alias")] + [TestCase("us-west-2:myfunction:alias", "us-west-2", "", null)] + [TestCase("123456789012:my-function", "us-west-2", "123456789012", "arn:aws:lambda:us-west-2:123456789012:function:my-function")] + [TestCase("123456789012:my-function:myalias", "us-west-2", "123456789012", "arn:aws:lambda:us-west-2:123456789012:function:my-function:myalias")] + [TestCase("123456789012:my-function:myalias:extra", "us-west-2", "123456789012", null)] + [TestCase("123456789012:my-function:myalias:extra:lots:of:extra:way:too:many", "us-west-2", "123456789012", null)] + [TestCase("eu-west-1:us-west-2", "eu-west-1", "123456789012", "arn:aws:lambda:eu-west-1:123456789012:function:us-west-2")] + // Edge cases: functions that look like account IDs or region names + [TestCase("123456789012:444455556666", "us-west-2", "123456789012", "arn:aws:lambda:us-west-2:123456789012:function:444455556666")] + [TestCase("444455556666", "us-west-2", "123456789012", "arn:aws:lambda:us-west-2:123456789012:function:444455556666")] + [TestCase("us-west-2", "us-west-2", "123456789012", "arn:aws:lambda:us-west-2:123456789012:function:us-west-2")] public void ConstructArn(string name, string region, string accountId, string arn) { var constructedArn = AwsSdkHelpers.ConstructArn(null, name, region, accountId); From c15328cb0ad56f25a253af843ba6d848aeff7bc0 Mon Sep 17 00:00:00 2001 From: chynesNR Date: Wed, 9 Oct 2024 14:34:04 -0700 Subject: [PATCH 05/11] Fixes to integration tests --- .../AwsSdk/InvokeLambdaTests.cs | 36 +++++++++++++------ .../AwsSdk/InvokeLambdaExerciser.cs | 3 ++ 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/tests/Agent/IntegrationTests/IntegrationTests/AwsSdk/InvokeLambdaTests.cs b/tests/Agent/IntegrationTests/IntegrationTests/AwsSdk/InvokeLambdaTests.cs index 3c959da12f..cb243612b7 100644 --- a/tests/Agent/IntegrationTests/IntegrationTests/AwsSdk/InvokeLambdaTests.cs +++ b/tests/Agent/IntegrationTests/IntegrationTests/AwsSdk/InvokeLambdaTests.cs @@ -41,7 +41,7 @@ public InvokeLambdaTestBase(TFixture fixture, ITestOutputHelper output, bool asy }, exerciseApplication: () => { - _fixture.AgentLog.WaitForLogLines(AgentLogBase.TransactionTransformCompletedLogLineRegex, TimeSpan.FromMinutes(20), 2); + _fixture.AgentLog.WaitForLogLines(AgentLogBase.TransactionTransformCompletedLogLineRegex, TimeSpan.FromMinutes(20),2); } ); @@ -53,9 +53,10 @@ public InvokeLambdaTestBase(TFixture fixture, ITestOutputHelper output, bool asy else { _fixture.AddCommand($"InvokeLambdaExerciser InvokeLambdaSync {_function} \"fakepayload\""); - } - _fixture.AddCommand($"InvokeLambdaExerciser InvokeLambdaSync fakefunction fakepayload"); + // This will fail without an ARN because there's no account ID + _fixture.AddCommand($"InvokeLambdaExerciser InvokeLambdaSync fakefunction fakepayload"); + } _fixture.Initialize(); } @@ -66,27 +67,40 @@ public void InvokeLambda() var expectedMetrics = new List { + new Assertions.ExpectedMetric {metricName = @"DotNet/InvokeRequest", CallCountAllHarvests = 2}, }; - Assertions.MetricsExist(expectedMetrics, metrics); var transactions = _fixture.AgentLog.GetTransactionEvents(); + Assert.Equal(2, transactions.Count()); - Assert.NotNull(transactions); + foreach (var transaction in transactions) + { + Assert.StartsWith("OtherTransaction/Custom/MultiFunctionApplicationHelpers.NetStandardLibraries.AwsSdk.InvokeLambdaExerciser/InvokeLambda", transaction.IntrinsicAttributes["name"].ToString()); + } - var spans = _fixture.AgentLog.GetSpanEvents() - .Where(e => e.AgentAttributes.ContainsKey("cloud.resource_id")) + var allSpans = _fixture.AgentLog.GetSpanEvents() + .Where(e => e.AgentAttributes.ContainsKey("cloud.platform")) .ToList(); + Assert.Equal(2, allSpans.Count); - Assert.Equal(_isAsync ? 2 : 1, spans.Count); - - foreach (var span in spans) + foreach (var span in allSpans) { - Assert.Equal(_arn, span.AgentAttributes["cloud.resource_id"]); Assert.Equal("aws_lambda", span.AgentAttributes["cloud.platform"]); Assert.Equal("InvokeRequest", span.AgentAttributes["aws.operation"]); Assert.Equal("us-west-2", span.AgentAttributes["aws.region"]); } + + // There should be one fewer span in this list, because there's one where there wasn't + // enough info to create an ARN + var spansWithArn = _fixture.AgentLog.GetSpanEvents() + .Where(e => e.AgentAttributes.ContainsKey("cloud.resource_id")) + .ToList(); + Assert.Equal(_isAsync ? 2 : 1, spansWithArn.Count); + foreach (var span in spansWithArn) + { + Assert.Equal(_arn, span.AgentAttributes["cloud.resource_id"]); + } } } [NetFrameworkTest] diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/AwsSdk/InvokeLambdaExerciser.cs b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/AwsSdk/InvokeLambdaExerciser.cs index e02532ab8e..574707aa0f 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/AwsSdk/InvokeLambdaExerciser.cs +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/AwsSdk/InvokeLambdaExerciser.cs @@ -1,6 +1,7 @@ // Copyright 2020 New Relic, Inc. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +using System; using System.IO; using System.Security.AccessControl; using System.Threading.Tasks; @@ -34,6 +35,8 @@ public void InvokeLambdaSync(string function, string payload) catch { } +#else + throw new Exception($"Synchronous calls are only supported on .NET Framework!"); #endif } From 8728cffd19cd7d140ae2381c35b8fa853fc2eb8b Mon Sep 17 00:00:00 2001 From: chynesNR Date: Wed, 9 Oct 2024 16:43:34 -0700 Subject: [PATCH 06/11] Forgot to add tests to workflow --- .github/workflows/all_solutions.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/all_solutions.yml b/.github/workflows/all_solutions.yml index 91b5ffcc09..5a3e975486 100644 --- a/.github/workflows/all_solutions.yml +++ b/.github/workflows/all_solutions.yml @@ -232,6 +232,7 @@ jobs: AwsLambda.Sns, AwsLambda.Sqs, AwsLambda.WebRequest, + AwsSdk, AzureFunction, BasicInstrumentation, CatInbound, From cdb0bf381cdcdf8f3abd60c2398903b4905c8547 Mon Sep 17 00:00:00 2001 From: chynesNR Date: Mon, 14 Oct 2024 14:58:36 -0700 Subject: [PATCH 07/11] Don't generate a segment for the HTTP request --- .../Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs index 66405f3eca..d4bd94e0f6 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs @@ -5,6 +5,7 @@ using System; using System.Threading.Tasks; using NewRelic.Agent.Api; +using NewRelic.Agent.Api.Experimental; using NewRelic.Agent.Extensions.Providers.Wrapper; using NewRelic.Reflection; using NewRelic.Agent.Extensions.Helpers; @@ -50,6 +51,7 @@ public static AfterWrappedMethodDelegate HandleInvokeRequest(InstrumentedMethodC } string arn = AwsSdkHelpers.ConstructArn(agent, functionName, region, ""); var segment = transaction.StartTransactionSegment(instrumentedMethodCall.MethodCall, "InvokeRequest"); + segment.GetExperimentalApi().MakeLeaf(); transaction.AddCloudSdkAttribute("cloud.platform", "aws_lambda"); transaction.AddCloudSdkAttribute("aws.operation", "InvokeRequest"); From 10a5e4bab1341a6627f5485728c717261649a384 Mon Sep 17 00:00:00 2001 From: chynesNR Date: Mon, 14 Oct 2024 15:08:23 -0700 Subject: [PATCH 08/11] Fix some typos --- .../NewRelic.Agent.Extensions/Helpers/AwsSdk.cs | 2 +- .../IntegrationTests/AwsSdk/InvokeLambdaTests.cs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs index 716e18b5b9..f9408211d1 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs @@ -26,7 +26,7 @@ public static class AwsSdkHelpers // 3. 'function' (fixed string) // 4. Function name // 5. Alias or version - // Only the function name is required, the reset are all optional. e.g. you could have region and function name and nothing else + // Only the function name is required, the rest are all optional. e.g. you could have region and function name and nothing else public static string ConstructArn(IAgent agent, string invocationName, string region, string accountId) { if (invocationName.StartsWith("arn:")) diff --git a/tests/Agent/IntegrationTests/IntegrationTests/AwsSdk/InvokeLambdaTests.cs b/tests/Agent/IntegrationTests/IntegrationTests/AwsSdk/InvokeLambdaTests.cs index cb243612b7..464a4128d2 100644 --- a/tests/Agent/IntegrationTests/IntegrationTests/AwsSdk/InvokeLambdaTests.cs +++ b/tests/Agent/IntegrationTests/IntegrationTests/AwsSdk/InvokeLambdaTests.cs @@ -21,15 +21,15 @@ public abstract class InvokeLambdaTestBase : NewRelicIntegrationTest { - _fixture.AgentLog.WaitForLogLines(AgentLogBase.TransactionTransformCompletedLogLineRegex, TimeSpan.FromMinutes(20),2); + _fixture.AgentLog.WaitForLogLines(AgentLogBase.TransactionTransformCompletedLogLineRegex, TimeSpan.FromMinutes(2),2); } ); - if (async) + if (useAsync) { _fixture.AddCommand($"InvokeLambdaExerciser InvokeLambdaAsync {_function}:{_qualifier} \"fakepayload\""); _fixture.AddCommand($"InvokeLambdaExerciser InvokeLambdaAsyncWithQualifier {_function} {_qualifier} \"fakepayload\""); From db2202ca4a23c84a7fa6d360998ca588e5ec4cd0 Mon Sep 17 00:00:00 2001 From: chynesNR Date: Tue, 15 Oct 2024 10:04:44 -0700 Subject: [PATCH 09/11] Cache constructed ARNs --- .../Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs index d4bd94e0f6..2beaef823d 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs @@ -15,6 +15,7 @@ namespace NewRelic.Providers.Wrapper.AwsSdk internal static class LambdaInvokeRequestHandler { private static ConcurrentDictionary> _getResultFromGenericTask = new(); + private static ConcurrentDictionary _arnCache = new ConcurrentDictionary(); private static object GetTaskResult(object task) { @@ -49,7 +50,12 @@ public static AfterWrappedMethodDelegate HandleInvokeRequest(InstrumentedMethodC { functionName = $"{functionName}:{qualifier}"; } - string arn = AwsSdkHelpers.ConstructArn(agent, functionName, region, ""); + string arn; + if (!_arnCache.TryGetValue(functionName, out arn)) + { + arn = AwsSdkHelpers.ConstructArn(agent, functionName, region, ""); + _arnCache.TryAdd(functionName, arn); + } var segment = transaction.StartTransactionSegment(instrumentedMethodCall.MethodCall, "InvokeRequest"); segment.GetExperimentalApi().MakeLeaf(); From a14adbbb7813116d4d1191a422d87128afc36851 Mon Sep 17 00:00:00 2001 From: chynesNR Date: Tue, 15 Oct 2024 14:55:09 -0700 Subject: [PATCH 10/11] PR feedback --- .../Agent/Core/Transactions/Transaction.cs | 6 ++-- .../Collections/ConcurrentHashSet.cs | 8 ++++- .../Helpers/AwsSdk.cs | 32 +++++++++---------- .../Wrapper/AwsLambda/HandlerMethodWrapper.cs | 6 ++-- .../AwsSdk/LambdaInvokeRequestHandler.cs | 25 +++++++++++---- .../MFALatestPackages.csproj | 4 +-- .../MultiFunctionApplicationHelpers.csproj | 6 ++-- .../Collections/ConcurrentHashSet.cs | 17 ++++++++++ 8 files changed, 68 insertions(+), 36 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Core/Transactions/Transaction.cs b/src/Agent/NewRelic/Agent/Core/Transactions/Transaction.cs index 3706e795d0..501fa9a2dd 100644 --- a/src/Agent/NewRelic/Agent/Core/Transactions/Transaction.cs +++ b/src/Agent/NewRelic/Agent/Core/Transactions/Transaction.cs @@ -1374,7 +1374,7 @@ public void AddLambdaAttribute(string name, object value) { if (string.IsNullOrWhiteSpace(name)) { - Log.Debug($"AddLambdaAttribute - Unable to set Lambda value on transaction because the key is null/empty"); + Log.Debug($"AddLambdaAttribute - Name cannot be null/empty"); return; } @@ -1386,7 +1386,7 @@ public void AddFaasAttribute(string name, object value) { if (string.IsNullOrWhiteSpace(name)) { - Log.Debug($"AddFaasAttribute - Unable to set FaaS value on transaction because the key is null/empty"); + Log.Debug($"AddFaasAttribute - Name cannot be null/empty"); return; } @@ -1398,7 +1398,7 @@ public void AddCloudSdkAttribute(string name, object value) { if (string.IsNullOrWhiteSpace(name)) { - Log.Debug($"AddCloudSdkAttribute - Unable to set Cloud value on transaction because the key is null/empty"); + Log.Debug($"AddCloudSdkAttribute - Name cannot be null/empty"); return; } diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Collections/ConcurrentHashSet.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Collections/ConcurrentHashSet.cs index 28e8133cf2..145d4391be 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Collections/ConcurrentHashSet.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Collections/ConcurrentHashSet.cs @@ -47,7 +47,13 @@ public void Add(T item) _hashSet.Add(item); } } - + public bool TryAdd(T item) + { + using (_writeLock()) + { + return _hashSet.Add(item); + } + } public void Clear() { using (_writeLock()) diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs index f9408211d1..036bd02f09 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs @@ -5,6 +5,7 @@ using System.Linq; using System.Text.RegularExpressions; using NewRelic.Agent.Api; +using NewRelic.Agent.Extensions.Collections; namespace NewRelic.Agent.Extensions.Helpers { @@ -14,7 +15,16 @@ public static class AwsSdkHelpers private static bool LooksLikeARegion(string text) => RegionRegex.IsMatch(text); private static bool LooksLikeAnAccountId(string text) => (text.Length == 12) && text.All(c => c >= '0' && c <= '9'); // Only log ARNs we can't parse once - private static HashSet BadInvocations = new HashSet(); + private static ConcurrentHashSet BadInvocations = new ConcurrentHashSet(); + private const int MAX_BAD_INVOCATIONS = 25; + + private static void ReportBadInvocation(IAgent agent, string invocationName) + { + if ((agent?.Logger.IsDebugEnabled == true) && (BadInvocations.Count < MAX_BAD_INVOCATIONS) && BadInvocations.TryAdd(invocationName)) + { + agent.Logger.Debug($"Unable to parse function name '{invocationName}'"); + } + } // This is the full regex pattern for an ARN: // (arn:(aws[a-zA-Z-]*)?:lambda:)?([a-z]{2}((-gov)|(-iso([a-z]?)))?-[a-z]+-\d{1}:)?(\d{12}:)?(function:)?([a-zA-Z0-9-_\.]+)(:(\$LATEST|[a-zA-Z0-9-_]+))? @@ -78,10 +88,7 @@ public static string ConstructArn(IAgent agent, string invocationName, string re } else { - if (BadInvocations.Add(invocationName)) - { - agent?.Logger.Debug($"Unable to parse function name '{invocationName}'"); - } + ReportBadInvocation(agent, invocationName); return null; } } @@ -94,28 +101,19 @@ public static string ConstructArn(IAgent agent, string invocationName, string re } else { - if (BadInvocations.Add(invocationName)) - { - agent?.Logger.Debug($"Unable to parse function name '{invocationName}'"); - } + ReportBadInvocation(agent, invocationName); return null; } } if (string.IsNullOrEmpty(accountId)) { - if (BadInvocations.Add(invocationName)) - { - agent?.Logger.Debug($"Need account ID in order to resolve function '{invocationName}'"); - } + ReportBadInvocation(agent, invocationName); return null; } if (string.IsNullOrEmpty(region)) { - if (BadInvocations.Add(invocationName)) - { - agent?.Logger.Debug($"Need region in order to resolve function '{invocationName}'"); - } + ReportBadInvocation(agent, invocationName); return null; } if (!string.IsNullOrEmpty(alias)) diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsLambda/HandlerMethodWrapper.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsLambda/HandlerMethodWrapper.cs index 1fdc696c79..ac2a32c79b 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsLambda/HandlerMethodWrapper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsLambda/HandlerMethodWrapper.cs @@ -193,10 +193,9 @@ private void InitLambdaData(InstrumentedMethodCall instrumentedMethodCall, IAgen { agent.Logger.Log(Agent.Extensions.Logging.Level.Debug, $"Supported Event Type found: {_functionDetails.EventType}"); } - else if (!_unsupportedInputTypes.Contains(name)) + else if (_unsupportedInputTypes.TryAdd(name)) { agent.Logger.Log(Agent.Extensions.Logging.Level.Warn, $"Unsupported input object type: {name}. Unable to provide additional instrumentation."); - _unsupportedInputTypes.Add(name); } } @@ -344,10 +343,9 @@ private void CaptureResponseData(ITransaction transaction, object response, IAge || (_functionDetails.EventType == AwsLambdaEventType.ApplicationLoadBalancerRequest && responseType != "Amazon.Lambda.ApplicationLoadBalancerEvents.ApplicationLoadBalancerResponse")) { - if (!_unexpectedResponseTypes.Contains(responseType)) + if (_unexpectedResponseTypes.TryAdd(responseType)) { agent.Logger.Log(Agent.Extensions.Logging.Level.Warn, $"Unexpected response type {responseType} for request event type {_functionDetails.EventType}. Not capturing any response data."); - _unexpectedResponseTypes.Add(responseType); } return; diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs index 2beaef823d..f2795dfd09 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs @@ -14,8 +14,9 @@ namespace NewRelic.Providers.Wrapper.AwsSdk { internal static class LambdaInvokeRequestHandler { - private static ConcurrentDictionary> _getResultFromGenericTask = new(); + private static Func _getResultFromGenericTask; private static ConcurrentDictionary _arnCache = new ConcurrentDictionary(); + private static bool _reportMissingRequestId = true; private static object GetTaskResult(object task) { @@ -24,7 +25,7 @@ private static object GetTaskResult(object task) return null; } - var getResponse = _getResultFromGenericTask.GetOrAdd(task.GetType(), t => VisibilityBypasser.Instance.GeneratePropertyAccessor(t, "Result")); + var getResponse = _getResultFromGenericTask ??= VisibilityBypasser.Instance.GeneratePropertyAccessor(task.GetType(), "Result"); return getResponse(task); } @@ -38,7 +39,11 @@ private static void SetRequestIdIfAvailable(IAgent agent, ITransaction transacti } catch (Exception e) { - agent.Logger.Debug(e, "Unable to get RequestId from response metadata."); + if (_reportMissingRequestId) + { + agent.Logger.Debug(e, "Unable to get RequestId from response metadata."); + _reportMissingRequestId = false; + } } } @@ -51,10 +56,18 @@ public static AfterWrappedMethodDelegate HandleInvokeRequest(InstrumentedMethodC functionName = $"{functionName}:{qualifier}"; } string arn; - if (!_arnCache.TryGetValue(functionName, out arn)) + if (functionName.StartsWith("arn:")) { - arn = AwsSdkHelpers.ConstructArn(agent, functionName, region, ""); - _arnCache.TryAdd(functionName, arn); + arn = functionName; + } + else + { + string key = $"{region}:{functionName}"; + if (!_arnCache.TryGetValue(key, out arn)) + { + arn = AwsSdkHelpers.ConstructArn(agent, functionName, region, ""); + _arnCache.TryAdd(key, arn); + } } var segment = transaction.StartTransactionSegment(instrumentedMethodCall.MethodCall, "InvokeRequest"); segment.GetExperimentalApi().MakeLeaf(); diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj b/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj index b4473527e2..02f30315c9 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj @@ -9,8 +9,8 @@ - - + + diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj index 604e2e7804..b57276e597 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj @@ -277,10 +277,10 @@ - + - - + + diff --git a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Collections/ConcurrentHashSet.cs b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Collections/ConcurrentHashSet.cs index 85a69a634d..3245e2e9e4 100644 --- a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Collections/ConcurrentHashSet.cs +++ b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Collections/ConcurrentHashSet.cs @@ -96,6 +96,23 @@ public void ConcurrentHashSet_IsThreadSafe() tasks.ForEach(task => task.Wait()); } + [Test] + [TestCase(new[] { 1, 2, 3 }, new[] { true, true, true })] + [TestCase(new[] { 1, 2, 2, 3 }, new[] { true, true, false, true })] + [TestCase(new[] { 4, 4, 4, 4 }, new[] { true, false, false, false })] + [TestCase(new[] { 5, 6, 7, 8, 9 }, new[] { true, true, true, true, true })] + [TestCase(new[] { 10, 10, 11, 11, 12 }, new[] { true, false, true, false, true })] + public void ConcurrentHashSet_TryAdd(int[] values, bool[] results) + { + for (var i = 0; i < values.Length; i++) + { + var value = values[i]; + var result = results[i]; + + Assert.That(_concurrentHashSet.TryAdd(value), Is.EqualTo(result)); + } + } + private static void ExerciseFullApi(ConcurrentHashSet hashSet, int[] numbersToAdd) { dynamic _; From 28a5ebe3da60907abbc07e97a989e89bfb5e3e65 Mon Sep 17 00:00:00 2001 From: chynesNR Date: Wed, 16 Oct 2024 09:10:03 -0700 Subject: [PATCH 11/11] Fix unit test --- .../Collections/ConcurrentHashSet.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Collections/ConcurrentHashSet.cs b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Collections/ConcurrentHashSet.cs index 3245e2e9e4..2f4e589362 100644 --- a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Collections/ConcurrentHashSet.cs +++ b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Collections/ConcurrentHashSet.cs @@ -104,12 +104,13 @@ public void ConcurrentHashSet_IsThreadSafe() [TestCase(new[] { 10, 10, 11, 11, 12 }, new[] { true, false, true, false, true })] public void ConcurrentHashSet_TryAdd(int[] values, bool[] results) { + ConcurrentHashSet set = new(); for (var i = 0; i < values.Length; i++) { var value = values[i]; var result = results[i]; - Assert.That(_concurrentHashSet.TryAdd(value), Is.EqualTo(result)); + Assert.That(set.TryAdd(value), Is.EqualTo(result)); } }