From 4434a8c5a869c80ca68a0ce6720da1125905b789 Mon Sep 17 00:00:00 2001 From: chynesNR Date: Thu, 26 Sep 2024 14:03:36 -0700 Subject: [PATCH 01/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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)); } } From 434018102c5417004da9d7ccba7214bdcbbdbedb Mon Sep 17 00:00:00 2001 From: chynesNR Date: Mon, 28 Oct 2024 12:46:20 -0700 Subject: [PATCH 12/23] First pass at incorporating account ID --- .../Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs | 23 ++++++++++++++++++- .../AwsSdk/LambdaInvokeRequestHandler.cs | 6 ++--- 2 files changed, 25 insertions(+), 4 deletions(-) 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 3fa75fd2dd..84e2178c0f 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using NewRelic.Agent.Api; using NewRelic.Agent.Extensions.Providers.Wrapper; @@ -12,6 +13,7 @@ namespace NewRelic.Providers.Wrapper.AwsSdk public class AwsSdkPipelineWrapper : IWrapper { public bool IsTransactionRequired => true; + private string _accountId = null; private const string WrapperName = "AwsSdkPipelineWrapper"; private static HashSet _unsupportedRequestTypes = new(); @@ -38,6 +40,24 @@ private string GetRegion(IAgent agent, dynamic requestContext) return ""; } + private string GetAccountId(IAgent agent) + { + if (_accountId != null) + { + return _accountId; + } + _accountId = agent.Configuration.AwsAccountId; + if (_accountId == null) + { + _accountId = ""; + } + else if ((_accountId.Length != 12) || _accountId.Any(c => (c < '0') || (c > '9'))) + { + agent.Logger.Warn("Supplied AWS Account Id appears to be invalid: {0}", _accountId); + } + return _accountId; + } + public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall instrumentedMethodCall, IAgent agent, ITransaction transaction) { // Get the IExecutionContext (the only parameter) @@ -67,6 +87,7 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins } dynamic request = requestContext.OriginalRequest; string requestType = request.GetType().FullName; + string accountId = agent.Configuration.AwsAccountId; if (requestType.StartsWith("Amazon.SQS")) { @@ -74,7 +95,7 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins } else if (requestType == "Amazon.Lambda.Model.InvokeRequest") { - return LambdaInvokeRequestHandler.HandleInvokeRequest(instrumentedMethodCall, agent, transaction, request, isAsync, GetRegion(agent, requestContext)); + return LambdaInvokeRequestHandler.HandleInvokeRequest(instrumentedMethodCall, agent, transaction, request, isAsync, GetRegion(agent, requestContext), accountId); } if (_unsupportedRequestTypes.Add(requestType)) // log once per unsupported request type 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 f2795dfd09..1a0afe865f 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs @@ -47,7 +47,7 @@ private static void SetRequestIdIfAvailable(IAgent agent, ITransaction transacti } } - public static AfterWrappedMethodDelegate HandleInvokeRequest(InstrumentedMethodCall instrumentedMethodCall, IAgent agent, ITransaction transaction, dynamic request, bool isAsync, string region) + public static AfterWrappedMethodDelegate HandleInvokeRequest(InstrumentedMethodCall instrumentedMethodCall, IAgent agent, ITransaction transaction, dynamic request, bool isAsync, string region, string accountId) { string functionName = request.FunctionName; string qualifier = request.Qualifier; @@ -65,12 +65,12 @@ public static AfterWrappedMethodDelegate HandleInvokeRequest(InstrumentedMethodC string key = $"{region}:{functionName}"; if (!_arnCache.TryGetValue(key, out arn)) { - arn = AwsSdkHelpers.ConstructArn(agent, functionName, region, ""); + arn = AwsSdkHelpers.ConstructArn(agent, functionName, region, accountId); _arnCache.TryAdd(key, arn); } } var segment = transaction.StartTransactionSegment(instrumentedMethodCall.MethodCall, "InvokeRequest"); - segment.GetExperimentalApi().MakeLeaf(); + //segment.GetExperimentalApi().MakeLeaf(); transaction.AddCloudSdkAttribute("cloud.platform", "aws_lambda"); transaction.AddCloudSdkAttribute("aws.operation", "InvokeRequest"); From 2d1e5c010663c411a368b38c62749dff372fd217 Mon Sep 17 00:00:00 2001 From: chynesNR Date: Mon, 11 Nov 2024 14:29:27 -0800 Subject: [PATCH 13/23] First pass at ArnBuilder --- .../AwsSdk/ArnBuilder.cs | 52 +++++++++++++++++++ .../Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs | 37 +++++++------ .../AwsSdk/LambdaInvokeRequestHandler.cs | 9 ++-- 3 files changed, 75 insertions(+), 23 deletions(-) create mode 100644 src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs new file mode 100644 index 0000000000..1a64f68fdb --- /dev/null +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs @@ -0,0 +1,52 @@ +// Copyright 2020 New Relic, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +namespace NewRelic.Agent.Extensions.AwsSdk +{ + public class ArnBuilder + { + private string _partition; + public string Partition + { + private set + { + _partition = value; + } + get => _partition; + } + + private string _region; + public string Region + { + private set + { + _region = value; + } + get => _region; + } + private string _accountId; + public string AccountId + { + private set + { + _accountId = value; + } + get => _accountId; + } + public ArnBuilder(string partition, string region, string accountId) + { + _partition = partition; + _region = region; + _accountId = accountId; + } + + public string Build(string service, string resource) + { + if (string.IsNullOrEmpty(_partition) || string.IsNullOrEmpty(_region) || string.IsNullOrEmpty(_accountId)) + { + return null; + } + return "arn:" + _partition + ":" + service + ":" + _region + ":" + _accountId + ":" + resource; + } + } +} 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 84e2178c0f..6108f1bbf9 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Threading.Tasks; using NewRelic.Agent.Api; +using NewRelic.Agent.Extensions.AwsSdk; using NewRelic.Agent.Extensions.Providers.Wrapper; namespace NewRelic.Providers.Wrapper.AwsSdk @@ -13,7 +14,7 @@ namespace NewRelic.Providers.Wrapper.AwsSdk public class AwsSdkPipelineWrapper : IWrapper { public bool IsTransactionRequired => true; - private string _accountId = null; + private ArnBuilder _arnBuilder = null; private const string WrapperName = "AwsSdkPipelineWrapper"; private static HashSet _unsupportedRequestTypes = new(); @@ -23,39 +24,37 @@ public CanWrapResponse CanWrap(InstrumentedMethodInfo methodInfo) return new CanWrapResponse(WrapperName.Equals(methodInfo.RequestedWrapperName)); } - private string GetRegion(IAgent agent, dynamic requestContext) + private ArnBuilder CreateArnBuilder(IAgent agent, dynamic requestContext) { + string + if (_arnBuilder != null) + { + return _arnBuilder; + } try { var clientconfig = requestContext.ClientConfig; var regionEndpoint = clientconfig.RegionEndpoint; var systemName = regionEndpoint.SystemName; - return systemName; + var partition = regionEndpoint.PartitionName; + _arnBuilder = new ArnBuilder(partition, systemName, GetAccountId(agent)); } catch (Exception e) { - agent.Logger.Debug(e, $"AwsSdkPipelineWrapper: Unable to get region from requestContext."); + agent.Logger.Debug(e, $"AwsSdkPipelineWrapper: Unable to get required ARN components from requestContext."); } - return ""; + return _arnBuilder; } private string GetAccountId(IAgent agent) { - if (_accountId != null) - { - return _accountId; - } - _accountId = agent.Configuration.AwsAccountId; - if (_accountId == null) - { - _accountId = ""; - } - else if ((_accountId.Length != 12) || _accountId.Any(c => (c < '0') || (c > '9'))) + string accountId = agent.Configuration.AwsAccountId; + if ((accountId.Length != 12) || accountId.Any(c => (c < '0') || (c > '9'))) { - agent.Logger.Warn("Supplied AWS Account Id appears to be invalid: {0}", _accountId); + agent.Logger.Warn("Supplied AWS Account Id appears to be invalid"); } - return _accountId; + return accountId; } public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall instrumentedMethodCall, IAgent agent, ITransaction transaction) @@ -87,7 +86,7 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins } dynamic request = requestContext.OriginalRequest; string requestType = request.GetType().FullName; - string accountId = agent.Configuration.AwsAccountId; + ArnBuilder builder = CreateArnBuilder(agent, requestContext); if (requestType.StartsWith("Amazon.SQS")) { @@ -95,7 +94,7 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins } else if (requestType == "Amazon.Lambda.Model.InvokeRequest") { - return LambdaInvokeRequestHandler.HandleInvokeRequest(instrumentedMethodCall, agent, transaction, request, isAsync, GetRegion(agent, requestContext), accountId); + return LambdaInvokeRequestHandler.HandleInvokeRequest(instrumentedMethodCall, agent, transaction, request, isAsync, builder); } if (_unsupportedRequestTypes.Add(requestType)) // log once per unsupported request type 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 1a0afe865f..cff7d864c4 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs @@ -9,6 +9,7 @@ using NewRelic.Agent.Extensions.Providers.Wrapper; using NewRelic.Reflection; using NewRelic.Agent.Extensions.Helpers; +using NewRelic.Agent.Extensions.AwsSdk; namespace NewRelic.Providers.Wrapper.AwsSdk { @@ -47,7 +48,7 @@ private static void SetRequestIdIfAvailable(IAgent agent, ITransaction transacti } } - public static AfterWrappedMethodDelegate HandleInvokeRequest(InstrumentedMethodCall instrumentedMethodCall, IAgent agent, ITransaction transaction, dynamic request, bool isAsync, string region, string accountId) + public static AfterWrappedMethodDelegate HandleInvokeRequest(InstrumentedMethodCall instrumentedMethodCall, IAgent agent, ITransaction transaction, dynamic request, bool isAsync, ArnBuilder builder) { string functionName = request.FunctionName; string qualifier = request.Qualifier; @@ -62,10 +63,10 @@ public static AfterWrappedMethodDelegate HandleInvokeRequest(InstrumentedMethodC } else { - string key = $"{region}:{functionName}"; + string key = $"{builder.Region}:{functionName}"; if (!_arnCache.TryGetValue(key, out arn)) { - arn = AwsSdkHelpers.ConstructArn(agent, functionName, region, accountId); + arn = builder.Build("function", functionName); _arnCache.TryAdd(key, arn); } } @@ -74,7 +75,7 @@ public static AfterWrappedMethodDelegate HandleInvokeRequest(InstrumentedMethodC transaction.AddCloudSdkAttribute("cloud.platform", "aws_lambda"); transaction.AddCloudSdkAttribute("aws.operation", "InvokeRequest"); - transaction.AddCloudSdkAttribute("aws.region", region); + transaction.AddCloudSdkAttribute("aws.region", builder.Region); if (!string.IsNullOrEmpty(arn)) From eb4a73e154e38ee49bf33dba1cbc97bd54ffc8fa Mon Sep 17 00:00:00 2001 From: chynesNR Date: Thu, 14 Nov 2024 16:12:56 -0800 Subject: [PATCH 14/23] Another pass at generic ARN construction --- .../AwsSdk/ArnBuilder.cs | 133 +++++++++++++++++- .../Helpers/AwsSdk.cs | 126 ----------------- .../Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs | 8 +- .../AwsSdk/LambdaInvokeRequestHandler.cs | 15 +- .../Helpers/AwsSdkHelperTests.cs | 28 +++- 5 files changed, 168 insertions(+), 142 deletions(-) delete mode 100644 src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs index 1a64f68fdb..0073b1a7c6 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs @@ -1,6 +1,10 @@ // Copyright 2020 New Relic, Inc. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +using System.Linq; +using System.Text.RegularExpressions; +using NewRelic.Agent.Extensions.Collections; + namespace NewRelic.Agent.Extensions.AwsSdk { public class ArnBuilder @@ -40,13 +44,134 @@ public ArnBuilder(string partition, string region, string accountId) _accountId = accountId; } - public string Build(string service, string resource) + public string Build(string service, string resource) => ConstructArn(_partition, service, _region, _accountId, resource); + + // This is the full regex pattern for a Lambda 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 rest are all optional. e.g. you could have region and function name and nothing else + public string BuildFromPartialLambdaArn(string invocationName) + { + if (invocationName.StartsWith("arn:")) + { + return invocationName; + } + var segments = invocationName.Split(':'); + string functionName = null; + string alias = null; + string fallback = null; + string region = null; + string accountId = null; + + // If there's only one string, assume it's the function name + if (segments.Length == 1) + { + functionName = segments[0]; + } + else + { + // All we should need is the function name, but if we find a region or account ID, we'll use it + // since it should be more accurate + foreach (var segment in segments) + { + // A string that looks like a region or account ID could also be the function name + // Assume it's the former, unless we never find a function name + if (LooksLikeARegion(segment)) + { + if (string.IsNullOrEmpty(region)) + { + region = segment; + } + else + { + fallback = segment; + } + continue; + } + else if (LooksLikeAnAccountId(segment)) + { + if (string.IsNullOrEmpty(accountId)) + { + accountId = segment; + } + else + { + fallback = segment; + } + continue; + } + else if (segment == "function") + { + continue; + } + else if (functionName == null) + { + functionName = segment; + } + else if (alias == null) + { + alias = segment; + } + else + { + return null; + } + } + } + + if (string.IsNullOrEmpty(functionName)) + { + if (!string.IsNullOrEmpty(fallback)) + { + functionName = fallback; + } + else + { + return null; + } + } + + accountId = !string.IsNullOrEmpty(accountId) ? accountId : _accountId; + if (string.IsNullOrEmpty(accountId)) + { + return null; + } + + region = !string.IsNullOrEmpty(region) ? region : _region; + if (string.IsNullOrEmpty(region)) + { + return null; + } + + + if (!string.IsNullOrEmpty(alias)) + { + return ConstructArn(_partition, "lambda", region, accountId, $"function:{functionName}:{alias}"); + + } + return ConstructArn(_partition, "lambda", region, accountId, $"function:{functionName}"); + } + + 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'); + + private string ConstructArn(string partition, string service, string region, string accountId, string resource) { - if (string.IsNullOrEmpty(_partition) || string.IsNullOrEmpty(_region) || string.IsNullOrEmpty(_accountId)) + if (string.IsNullOrEmpty(partition) || string.IsNullOrEmpty(region) || string.IsNullOrEmpty(accountId) + || string.IsNullOrEmpty(service) || string.IsNullOrEmpty(resource)) { return null; } - return "arn:" + _partition + ":" + service + ":" + _region + ":" + _accountId + ":" + resource; - } + return "arn:" + partition + ":" + service + ":" + region + ":" + accountId + ":" + resource; + } + } } 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 deleted file mode 100644 index 036bd02f09..0000000000 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/AwsSdk.cs +++ /dev/null @@ -1,126 +0,0 @@ -// 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; -using NewRelic.Agent.Extensions.Collections; - -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 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-_]+))? - - // 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 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:")) - { - return invocationName; - } - var segments = invocationName.Split(':'); - string functionName = null; - string alias = null; - string fallback = null; - - foreach (var segment in segments) - { - if (LooksLikeARegion(segment)) - { - if (string.IsNullOrEmpty(region)) - { - region = segment; - } - else - { - fallback = segment; - } - continue; - } - else if (LooksLikeAnAccountId(segment)) - { - if (string.IsNullOrEmpty(accountId)) - { - accountId = segment; - } - else - { - fallback = segment; - } - continue; - } - else if (segment == "function") - { - continue; - } - else if (functionName == null) - { - functionName = segment; - } - else if (alias == null) - { - alias = segment; - } - else - { - ReportBadInvocation(agent, invocationName); - return null; - } - } - - if (string.IsNullOrEmpty(functionName)) - { - if (!string.IsNullOrEmpty(fallback)) - { - functionName = fallback; - } - else - { - ReportBadInvocation(agent, invocationName); - return null; - } - } - - if (string.IsNullOrEmpty(accountId)) - { - ReportBadInvocation(agent, invocationName); - return null; - } - if (string.IsNullOrEmpty(region)) - { - ReportBadInvocation(agent, invocationName); - return null; - } - if (!string.IsNullOrEmpty(alias)) - { - return $"arn:aws:lambda:{region}:{accountId}:function:{functionName}:{alias}"; - } - 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 6108f1bbf9..f5f3e11327 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs @@ -26,7 +26,6 @@ public CanWrapResponse CanWrap(InstrumentedMethodInfo methodInfo) private ArnBuilder CreateArnBuilder(IAgent agent, dynamic requestContext) { - string if (_arnBuilder != null) { return _arnBuilder; @@ -50,9 +49,12 @@ private ArnBuilder CreateArnBuilder(IAgent agent, dynamic requestContext) private string GetAccountId(IAgent agent) { string accountId = agent.Configuration.AwsAccountId; - if ((accountId.Length != 12) || accountId.Any(c => (c < '0') || (c > '9'))) + if (accountId != null) { - agent.Logger.Warn("Supplied AWS Account Id appears to be invalid"); + if ((accountId.Length != 12) || accountId.Any(c => (c < '0') || (c > '9'))) + { + agent.Logger.Warn("Supplied AWS Account Id appears to be invalid"); + } } return accountId; } 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 cff7d864c4..ee0c97d687 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs @@ -18,6 +18,7 @@ internal static class LambdaInvokeRequestHandler private static Func _getResultFromGenericTask; private static ConcurrentDictionary _arnCache = new ConcurrentDictionary(); private static bool _reportMissingRequestId = true; + private static bool _reportBadInvocationName = true; private static object GetTaskResult(object task) { @@ -63,15 +64,14 @@ public static AfterWrappedMethodDelegate HandleInvokeRequest(InstrumentedMethodC } else { - string key = $"{builder.Region}:{functionName}"; - if (!_arnCache.TryGetValue(key, out arn)) + if (!_arnCache.TryGetValue(functionName, out arn)) { - arn = builder.Build("function", functionName); - _arnCache.TryAdd(key, arn); + arn = builder.BuildFromPartialLambdaArn(functionName); + _arnCache.TryAdd(functionName, arn); } } var segment = transaction.StartTransactionSegment(instrumentedMethodCall.MethodCall, "InvokeRequest"); - //segment.GetExperimentalApi().MakeLeaf(); + //segment.GetExperimentalApi().MakeLeaf(); // TODO: Leaf-or-not decision has not yet been finalized transaction.AddCloudSdkAttribute("cloud.platform", "aws_lambda"); transaction.AddCloudSdkAttribute("aws.operation", "InvokeRequest"); @@ -82,6 +82,11 @@ public static AfterWrappedMethodDelegate HandleInvokeRequest(InstrumentedMethodC { transaction.AddCloudSdkAttribute("cloud.resource_id", arn); } + else if (_reportBadInvocationName) + { + agent.Logger.Debug($"Unable to parse lambda invocation named '{functionName}''"); + _reportBadInvocationName = false; + } if (isAsync) { 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 1382e0d093..b95edf7ce9 100644 --- a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Helpers/AwsSdkHelperTests.cs +++ b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Helpers/AwsSdkHelperTests.cs @@ -2,11 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 using NUnit.Framework; -using NewRelic.Agent.Extensions.Helpers; +using NewRelic.Agent.Extensions.AwsSdk; namespace Agent.Extensions.Tests.Helpers { - public class AwsSdkHelperTests + public class AwsSdkArnBuilderTests { [Test] [TestCase("myfunction", "us-west-2", "123456789012", "arn:aws:lambda:us-west-2:123456789012:function:myfunction")] @@ -41,10 +41,30 @@ public class AwsSdkHelperTests [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) + public void ConstructLambdaArn(string name, string region, string accountId, string arn) { - var constructedArn = AwsSdkHelpers.ConstructArn(null, name, region, accountId); + var arnBuilder = new ArnBuilder("aws", region, accountId); + var constructedArn = arnBuilder.BuildFromPartialLambdaArn(name); Assert.That(constructedArn, Is.EqualTo(arn), "Did not get expected ARN"); } + + [Test] + [TestCase("aws", "s3", "us-west-2", "123456789012", "bucket_name", "arn:aws:s3:us-west-2:123456789012:bucket_name")] + [TestCase("aws", "dynamodb", "us-east-1", "987654321098", "table_name", "arn:aws:dynamodb:us-east-1:987654321098:table_name")] + [TestCase("aws", "dynamodb", "us-east-1", "987654321098", "table/tabletName", "arn:aws:dynamodb:us-east-1:987654321098:table/tabletName")] + [TestCase("aws", "ec2", "eu-west-1", "111122223333", "instance_id", "arn:aws:ec2:eu-west-1:111122223333:instance_id")] + [TestCase("aws", "sqs", "ap-southeast-1", "444455556666", "queue_name", "arn:aws:sqs:ap-southeast-1:444455556666:queue_name")] + [TestCase("aws", "sns", "ca-central-1", "777788889999", "topic_name", "arn:aws:sns:ca-central-1:777788889999:topic_name")] + [TestCase("aws-cn", "lambda", "cn-north-1", "222233334444", "function_name", "arn:aws-cn:lambda:cn-north-1:222233334444:function_name")] + [TestCase("aws-us-gov", "iam", "us-gov-west-1", "555566667777", "role_name", "arn:aws-us-gov:iam:us-gov-west-1:555566667777:role_name")] + [TestCase("aws", "rds", "sa-east-1", "888899990000", "db_instance", "arn:aws:rds:sa-east-1:888899990000:db_instance")] + [TestCase("aws", "s3", "", "123456789012", "bucket_name", null)] + [TestCase("aws", "s3", "us-west-2", "", "bucket_name", null)] + public void ConstructGenericArn(string partition, string service, string region, string accountId, string resource, string expectedArn) + { + var arnBuilder = new ArnBuilder(partition, region, accountId); + var constructedArn = arnBuilder.Build(service, resource); + Assert.That(constructedArn, Is.EqualTo(expectedArn), "Did not get expected ARN"); + } } } From 35dad1c58b295ac01451db1c99127f01b4eef77e Mon Sep 17 00:00:00 2001 From: chynesNR Date: Fri, 15 Nov 2024 12:56:22 -0800 Subject: [PATCH 15/23] Increasing code coverage --- .../AwsSdk/ArnBuilder.cs | 46 +++++-------------- .../Helpers/AwsSdkHelperTests.cs | 1 + 2 files changed, 12 insertions(+), 35 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs index 0073b1a7c6..751d7f1220 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs @@ -9,42 +9,18 @@ namespace NewRelic.Agent.Extensions.AwsSdk { public class ArnBuilder { - private string _partition; - public string Partition - { - private set - { - _partition = value; - } - get => _partition; - } + public readonly string Partition; + public readonly string Region; + public readonly string AccountId; - private string _region; - public string Region - { - private set - { - _region = value; - } - get => _region; - } - private string _accountId; - public string AccountId - { - private set - { - _accountId = value; - } - get => _accountId; - } public ArnBuilder(string partition, string region, string accountId) { - _partition = partition; - _region = region; - _accountId = accountId; + Partition = partition; + Region = region; + AccountId = accountId; } - public string Build(string service, string resource) => ConstructArn(_partition, service, _region, _accountId, resource); + public string Build(string service, string resource) => ConstructArn(Partition, service, Region, AccountId, resource); // This is the full regex pattern for a Lambda 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-_]+))? @@ -138,13 +114,13 @@ public string BuildFromPartialLambdaArn(string invocationName) } } - accountId = !string.IsNullOrEmpty(accountId) ? accountId : _accountId; + accountId = !string.IsNullOrEmpty(accountId) ? accountId : AccountId; if (string.IsNullOrEmpty(accountId)) { return null; } - region = !string.IsNullOrEmpty(region) ? region : _region; + region = !string.IsNullOrEmpty(region) ? region : Region; if (string.IsNullOrEmpty(region)) { return null; @@ -153,10 +129,10 @@ public string BuildFromPartialLambdaArn(string invocationName) if (!string.IsNullOrEmpty(alias)) { - return ConstructArn(_partition, "lambda", region, accountId, $"function:{functionName}:{alias}"); + return ConstructArn(Partition, "lambda", region, accountId, $"function:{functionName}:{alias}"); } - return ConstructArn(_partition, "lambda", region, accountId, $"function:{functionName}"); + return ConstructArn(Partition, "lambda", region, accountId, $"function:{functionName}"); } private static Regex RegionRegex = new Regex(@"^[a-z]{2}((-gov)|(-iso([a-z]?)))?-[a-z]+-\d{1}$", RegexOptions.Compiled); 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 b95edf7ce9..4995e9a8a6 100644 --- a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Helpers/AwsSdkHelperTests.cs +++ b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Helpers/AwsSdkHelperTests.cs @@ -37,6 +37,7 @@ public class AwsSdkArnBuilderTests [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")] + [TestCase("", "us-west-2", "123456789012", null)] // 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")] From d81184990fdc9196de71ad47f8b089e7665b19e8 Mon Sep 17 00:00:00 2001 From: chynesNR Date: Fri, 15 Nov 2024 16:15:34 -0800 Subject: [PATCH 16/23] Add supportability metric --- .../Agent/Core/AgentHealth/AgentHealthReporter.cs | 9 ++++++++- src/Agent/NewRelic/Agent/Core/Metrics/MetricNames.cs | 1 + .../AgentHealth/AgentHealthReporterTests.cs | 8 ++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/Agent/NewRelic/Agent/Core/AgentHealth/AgentHealthReporter.cs b/src/Agent/NewRelic/Agent/Core/AgentHealth/AgentHealthReporter.cs index 53fce6b123..85f4f4edaf 100644 --- a/src/Agent/NewRelic/Agent/Core/AgentHealth/AgentHealthReporter.cs +++ b/src/Agent/NewRelic/Agent/Core/AgentHealth/AgentHealthReporter.cs @@ -684,6 +684,7 @@ private void CollectOneTimeMetrics() ReportIfLoggingDisabled(); ReportIfInstrumentationIsDisabled(); ReportIfGCSamplerV2IsEnabled(); + ReportIfAwsAccountIdProvided(); } public void CollectMetrics() @@ -846,8 +847,14 @@ private void ReportIfGCSamplerV2IsEnabled() { ReportSupportabilityCountMetric(MetricNames.SupportabilityGCSamplerV2Enabled); } - } + private void ReportIfAwsAccountIdProvided() + { + if (!string.IsNullOrEmpty(_configuration.AwsAccountId)) + { + ReportSupportabilityCountMetric(MetricNames.SupportabilityAwsAccountIdProvided); + } + } } } diff --git a/src/Agent/NewRelic/Agent/Core/Metrics/MetricNames.cs b/src/Agent/NewRelic/Agent/Core/Metrics/MetricNames.cs index 2ea94148d3..b28ca695ec 100644 --- a/src/Agent/NewRelic/Agent/Core/Metrics/MetricNames.cs +++ b/src/Agent/NewRelic/Agent/Core/Metrics/MetricNames.cs @@ -838,6 +838,7 @@ public static string GetSupportabilityInstallType(string installType) public const string SupportabilityIgnoredInstrumentation = SupportabilityDotnetPs + "IgnoredInstrumentation"; public const string SupportabilityGCSamplerV2Enabled = SupportabilityDotnetPs + "GCSamplerV2/Enabled"; + public const string SupportabilityAwsAccountIdProvided = SupportabilityDotnetPs + "AwsAccountId/Config"; #endregion Supportability diff --git a/tests/Agent/UnitTests/Core.UnitTest/AgentHealth/AgentHealthReporterTests.cs b/tests/Agent/UnitTests/Core.UnitTest/AgentHealth/AgentHealthReporterTests.cs index b25163cee6..76acfea007 100644 --- a/tests/Agent/UnitTests/Core.UnitTest/AgentHealth/AgentHealthReporterTests.cs +++ b/tests/Agent/UnitTests/Core.UnitTest/AgentHealth/AgentHealthReporterTests.cs @@ -61,6 +61,7 @@ private IConfiguration GetDefaultConfiguration() Mock.Arrange(() => configuration.LoggingEnabled).Returns(() => _enableLogging); Mock.Arrange(() => configuration.IgnoredInstrumentation).Returns(() => _ignoredInstrumentation); Mock.Arrange(() => configuration.GCSamplerV2Enabled).Returns(true); + Mock.Arrange(() => configuration.AwsAccountId).Returns("123456789012"); return configuration; } @@ -531,5 +532,12 @@ public void GCSamplerV2EnabledSupportabiliityMetricPresent() _agentHealthReporter.CollectMetrics(); Assert.That(_publishedMetrics.Any(x => x.MetricNameModel.Name == "Supportability/Dotnet/GCSamplerV2/Enabled"), Is.True); } + + [Test] + public void AwsAccountIdSupportabiliityMetricPresent() + { + _agentHealthReporter.CollectMetrics(); + Assert.That(_publishedMetrics.Any(x => x.MetricNameModel.Name == "Supportability/Dotnet/AwsAccountId/Config"), Is.True); + } } } From d47b5a49c802dfa5b6c29ea9851ff73036fab08f Mon Sep 17 00:00:00 2001 From: chynesNR Date: Mon, 25 Nov 2024 12:37:21 -0800 Subject: [PATCH 17/23] Some cleanup and better logging --- .../AwsSdk/ArnBuilder.cs | 6 ++-- .../Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs | 30 ++++++++++++------- .../AwsSdk/LambdaInvokeRequestHandler.cs | 7 +++-- 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs index 751d7f1220..08301de978 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs @@ -46,7 +46,7 @@ public string BuildFromPartialLambdaArn(string invocationName) string region = null; string accountId = null; - // If there's only one string, assume it's the function name + // If there's only one segment, assume it's the function name if (segments.Length == 1) { functionName = segments[0]; @@ -126,11 +126,9 @@ public string BuildFromPartialLambdaArn(string invocationName) return null; } - if (!string.IsNullOrEmpty(alias)) { - return ConstructArn(Partition, "lambda", region, accountId, $"function:{functionName}:{alias}"); - + functionName += $":{alias}"; } return ConstructArn(Partition, "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 f5f3e11327..4e4c3c62b8 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs @@ -14,10 +14,11 @@ namespace NewRelic.Providers.Wrapper.AwsSdk public class AwsSdkPipelineWrapper : IWrapper { public bool IsTransactionRequired => true; - private ArnBuilder _arnBuilder = null; private const string WrapperName = "AwsSdkPipelineWrapper"; private static HashSet _unsupportedRequestTypes = new(); + private static bool _reportBadAccountId = true; + private static bool _reportBadArnBuilder = false; public CanWrapResponse CanWrap(InstrumentedMethodInfo methodInfo) { @@ -26,24 +27,27 @@ public CanWrapResponse CanWrap(InstrumentedMethodInfo methodInfo) private ArnBuilder CreateArnBuilder(IAgent agent, dynamic requestContext) { - if (_arnBuilder != null) - { - return _arnBuilder; - } + string partition = ""; + string systemName = ""; + string accountId = ""; try { + accountId = GetAccountId(agent); var clientconfig = requestContext.ClientConfig; var regionEndpoint = clientconfig.RegionEndpoint; - var systemName = regionEndpoint.SystemName; - var partition = regionEndpoint.PartitionName; - _arnBuilder = new ArnBuilder(partition, systemName, GetAccountId(agent)); + systemName = regionEndpoint.SystemName; + partition = regionEndpoint.PartitionName; } catch (Exception e) { - agent.Logger.Debug(e, $"AwsSdkPipelineWrapper: Unable to get required ARN components from requestContext."); + if (_reportBadArnBuilder) + { + agent.Logger.Debug(e, $"AwsSdkPipelineWrapper: Unable to get required ARN components from requestContext."); + _reportBadArnBuilder = false; + } } - return _arnBuilder; + return new ArnBuilder(partition, systemName, accountId); ; } private string GetAccountId(IAgent agent) @@ -53,7 +57,11 @@ private string GetAccountId(IAgent agent) { if ((accountId.Length != 12) || accountId.Any(c => (c < '0') || (c > '9'))) { - agent.Logger.Warn("Supplied AWS Account Id appears to be invalid"); + if (_reportBadAccountId) + { + agent.Logger.Warn("Supplied AWS Account ID appears to be invalid"); + _reportBadAccountId = false; + } } } return accountId; 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 ee0c97d687..e52b526e48 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs @@ -19,6 +19,7 @@ internal static class LambdaInvokeRequestHandler private static ConcurrentDictionary _arnCache = new ConcurrentDictionary(); private static bool _reportMissingRequestId = true; private static bool _reportBadInvocationName = true; + private const int MAX_CACHE_SIZE = 25; // Shouldn't ever get this big, but just in case private static object GetTaskResult(object task) { @@ -67,11 +68,13 @@ public static AfterWrappedMethodDelegate HandleInvokeRequest(InstrumentedMethodC if (!_arnCache.TryGetValue(functionName, out arn)) { arn = builder.BuildFromPartialLambdaArn(functionName); - _arnCache.TryAdd(functionName, arn); + if (_arnCache.Count < MAX_CACHE_SIZE) + { + _arnCache.TryAdd(functionName, arn); + } } } var segment = transaction.StartTransactionSegment(instrumentedMethodCall.MethodCall, "InvokeRequest"); - //segment.GetExperimentalApi().MakeLeaf(); // TODO: Leaf-or-not decision has not yet been finalized transaction.AddCloudSdkAttribute("cloud.platform", "aws_lambda"); transaction.AddCloudSdkAttribute("aws.operation", "InvokeRequest"); From f229fc1e78d0600a4534afeadf4e577fab116bae Mon Sep 17 00:00:00 2001 From: chynesNR Date: Mon, 25 Nov 2024 14:36:20 -0800 Subject: [PATCH 18/23] More logging improvements --- .../AwsSdk/ArnBuilder.cs | 15 ++++++++++++--- .../Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs | 6 +++--- .../Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs | 2 +- .../Helpers/AwsSdkHelperTests.cs | 12 ++++++++++++ 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs index 08301de978..bb0830c1c5 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs @@ -15,9 +15,9 @@ public class ArnBuilder public ArnBuilder(string partition, string region, string accountId) { - Partition = partition; - Region = region; - AccountId = accountId; + Partition = partition ?? ""; + Region = region ?? ""; + AccountId = accountId ?? ""; } public string Build(string service, string resource) => ConstructArn(Partition, service, Region, AccountId, resource); @@ -133,6 +133,15 @@ public string BuildFromPartialLambdaArn(string invocationName) return ConstructArn(Partition, "lambda", region, accountId, $"function:{functionName}"); } + public override string ToString() + { + string partition = string.IsNullOrEmpty(Partition) ? "[Missing]" : Partition; + string region = string.IsNullOrEmpty(Region) ? "[Missing]" : Region; + string accountId = string.IsNullOrEmpty(AccountId) ? "[Missing]" : "[Present]"; + + return $"Partition: {partition}, Region: {region}, AccountId: {accountId}"; + } + 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'); 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 6d100ad09f..0d3d2c80aa 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/AwsSdkPipelineWrapper.cs @@ -28,9 +28,9 @@ public CanWrapResponse CanWrap(InstrumentedMethodInfo methodInfo) private ArnBuilder CreateArnBuilder(IAgent agent, dynamic requestContext) { - string partition = ""; - string systemName = ""; - string accountId = ""; + string partition = null; + string systemName = null; + string accountId = null; try { accountId = GetAccountId(agent); 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 e52b526e48..cb41437d2c 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AwsSdk/LambdaInvokeRequestHandler.cs @@ -87,7 +87,7 @@ public static AfterWrappedMethodDelegate HandleInvokeRequest(InstrumentedMethodC } else if (_reportBadInvocationName) { - agent.Logger.Debug($"Unable to parse lambda invocation named '{functionName}''"); + agent.Logger.Debug($"Unable to resolve Lambda invocation named '{functionName}' [{builder.ToString()}]"); _reportBadInvocationName = false; } 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 4995e9a8a6..c342e4fbf7 100644 --- a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Helpers/AwsSdkHelperTests.cs +++ b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Helpers/AwsSdkHelperTests.cs @@ -67,5 +67,17 @@ public void ConstructGenericArn(string partition, string service, string region, var constructedArn = arnBuilder.Build(service, resource); Assert.That(constructedArn, Is.EqualTo(expectedArn), "Did not get expected ARN"); } + + [Test] + [TestCase("aws", "us-west-2", "123456789012", "Partition: aws, Region: us-west-2, AccountId: [Present]")] + [TestCase("aws", "", "123456789012", "Partition: aws, Region: [Missing], AccountId: [Present]")] + [TestCase("aws", "us-west-2", "", "Partition: aws, Region: us-west-2, AccountId: [Missing]")] + [TestCase("aws", "us-west-2", null, "Partition: aws, Region: us-west-2, AccountId: [Missing]")] + [TestCase("", "", "", "Partition: [Missing], Region: [Missing], AccountId: [Missing]")] + public void ArnBuilderToString(string partition, string region, string accountId, string expected) + { + var arnBuilder = new ArnBuilder(partition, region, accountId); + Assert.That(arnBuilder.ToString(), Is.EqualTo(expected), "Did not get expected string"); + } } } From 8af3e951d8bfd22c48dc32084c80a692b8a6af10 Mon Sep 17 00:00:00 2001 From: chynesNR Date: Mon, 25 Nov 2024 15:10:56 -0800 Subject: [PATCH 19/23] Fix bad merge --- .../Common/MFALatestPackages/MFALatestPackages.csproj | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj b/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj index 2bbec636b3..624d00e705 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj @@ -12,9 +12,6 @@ - - - From 272ace422678666c035465ed425af31609d9aec5 Mon Sep 17 00:00:00 2001 From: chynesNR Date: Mon, 25 Nov 2024 16:11:38 -0800 Subject: [PATCH 20/23] Fixing more merge errors --- .../Common/MFALatestPackages/MFALatestPackages.csproj | 4 ++-- .../MultiFunctionApplicationHelpers.csproj | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj b/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj index 624d00e705..03efc80501 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj @@ -10,7 +10,7 @@ - + @@ -34,7 +34,7 @@ - + diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj index 91c80022f4..8b02a93245 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj @@ -274,7 +274,7 @@ - + From 0b91f6ad8d5ad370de9bf9bf06c181664395a4cd Mon Sep 17 00:00:00 2001 From: chynesNR Date: Tue, 26 Nov 2024 09:44:34 -0800 Subject: [PATCH 21/23] Trying to address CodeQL and CodeCov issues --- .../AwsSdk/ArnBuilder.cs | 4 +-- .../Transactions/TransactionTests.cs | 31 +++++++++++++++++++ .../Helpers/AwsSdkHelperTests.cs | 1 + 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs index bb0830c1c5..b067b5b06b 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/AwsSdk/ArnBuilder.cs @@ -137,9 +137,9 @@ public override string ToString() { string partition = string.IsNullOrEmpty(Partition) ? "[Missing]" : Partition; string region = string.IsNullOrEmpty(Region) ? "[Missing]" : Region; - string accountId = string.IsNullOrEmpty(AccountId) ? "[Missing]" : "[Present]"; + string idPresent = string.IsNullOrEmpty(AccountId) ? "[Missing]" : "[Present]"; - return $"Partition: {partition}, Region: {region}, AccountId: {accountId}"; + return $"Partition: {partition}, Region: {region}, AccountId: {idPresent}"; } private static Regex RegionRegex = new Regex(@"^[a-z]{2}((-gov)|(-iso([a-z]?)))?-[a-z]+-\d{1}$", RegexOptions.Compiled); diff --git a/tests/Agent/UnitTests/Core.UnitTest/Transactions/TransactionTests.cs b/tests/Agent/UnitTests/Core.UnitTest/Transactions/TransactionTests.cs index 929a38be34..f14db94c45 100644 --- a/tests/Agent/UnitTests/Core.UnitTest/Transactions/TransactionTests.cs +++ b/tests/Agent/UnitTests/Core.UnitTest/Transactions/TransactionTests.cs @@ -591,4 +591,35 @@ public void HasHttpResponseStatusCode_ReturnsFalse_WhenStatusCodeIsNotSet() // Assert Assert.That(result, Is.False); } + public void AddCloudSdkAttribute_SetAttributeInTransactionMetadata() + { + // Arrange + var key = "TestAttribute"; + var value = "TestValue"; + + // Act + _transaction.AddCloudSdkAttribute(key, value); + + // Assert + var allAttributeValuesDic = _transaction.TransactionMetadata.UserAndRequestAttributes.GetAllAttributeValuesDic(); + + var attributeValue = allAttributeValuesDic[key]; + Assert.That(attributeValue, Is.EqualTo(value)); + } + + [TestCase(" ")] + [TestCase("")] + [TestCase(null)] + public void AddCloudSdkAttribute_DoesNotSetAttribute_WhenKeyIsBad(string key) + { + // Arrange + var value = "TestValue"; + + // Act + _transaction.AddCloudSdkAttribute(key, value); + + // Assert + Assert.That(_transaction.TransactionMetadata.UserAndRequestAttributes.Count, Is.EqualTo(0)); + } + } 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 c342e4fbf7..126be85b63 100644 --- a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Helpers/AwsSdkHelperTests.cs +++ b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Helpers/AwsSdkHelperTests.cs @@ -74,6 +74,7 @@ public void ConstructGenericArn(string partition, string service, string region, [TestCase("aws", "us-west-2", "", "Partition: aws, Region: us-west-2, AccountId: [Missing]")] [TestCase("aws", "us-west-2", null, "Partition: aws, Region: us-west-2, AccountId: [Missing]")] [TestCase("", "", "", "Partition: [Missing], Region: [Missing], AccountId: [Missing]")] + [TestCase(null, null, null, "Partition: [Missing], Region: [Missing], AccountId: [Missing]")] public void ArnBuilderToString(string partition, string region, string accountId, string expected) { var arnBuilder = new ArnBuilder(partition, region, accountId); From 17f810f053c7203fd993a9df6cd9e9f923e8e29a Mon Sep 17 00:00:00 2001 From: chynesNR Date: Tue, 26 Nov 2024 13:38:56 -0800 Subject: [PATCH 22/23] Only pass on expected error --- .../NetStandardLibraries/AwsSdk/InvokeLambdaExerciser.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 574707aa0f..371741d585 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/AwsSdk/InvokeLambdaExerciser.cs +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/AwsSdk/InvokeLambdaExerciser.cs @@ -6,6 +6,7 @@ using System.Security.AccessControl; using System.Threading.Tasks; using Amazon; +using Amazon.Lambda.Model; using NewRelic.Agent.IntegrationTests.Shared.ReflectionHelpers; using NewRelic.Api.Agent; @@ -32,7 +33,7 @@ public void InvokeLambdaSync(string function, string payload) { var response = client.Invoke(request); } - catch + catch (ResourceNotFoundException) { } #else @@ -60,7 +61,7 @@ public async Task InvokeLambdaAsync(string function, string payload) string returnValue = System.Text.Encoding.UTF8.GetString(stream.ToArray()); return returnValue; } - catch + catch (ResourceNotFoundException) { } return null; @@ -87,7 +88,7 @@ public async Task InvokeLambdaAsyncWithQualifier(string function, string string returnValue = System.Text.Encoding.UTF8.GetString(stream.ToArray()); return returnValue; } - catch + catch (ResourceNotFoundException) { } return null; From 641eafc0f615ebfabc1eebb3f45be47978ae1a28 Mon Sep 17 00:00:00 2001 From: chynesNR Date: Tue, 3 Dec 2024 11:03:30 -0800 Subject: [PATCH 23/23] Revert change (expected error is not consistent) --- .../NetStandardLibraries/AwsSdk/InvokeLambdaExerciser.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 371741d585..269e812fcb 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/AwsSdk/InvokeLambdaExerciser.cs +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/AwsSdk/InvokeLambdaExerciser.cs @@ -33,7 +33,7 @@ public void InvokeLambdaSync(string function, string payload) { var response = client.Invoke(request); } - catch (ResourceNotFoundException) + catch { } #else @@ -61,7 +61,7 @@ public async Task InvokeLambdaAsync(string function, string payload) string returnValue = System.Text.Encoding.UTF8.GetString(stream.ToArray()); return returnValue; } - catch (ResourceNotFoundException) + catch { } return null; @@ -88,7 +88,7 @@ public async Task InvokeLambdaAsyncWithQualifier(string function, string string returnValue = System.Text.Encoding.UTF8.GetString(stream.ToArray()); return returnValue; } - catch (ResourceNotFoundException) + catch { } return null;