From 94d4855b86c5b61d9334b95015de831a89ec966a Mon Sep 17 00:00:00 2001
From: Marty Tippin <120425148+tippmar-nr@users.noreply.github.com>
Date: Fri, 20 Sep 2024 09:59:59 -0500
Subject: [PATCH] PR feedback, unit test update, code cleanup/formatting
---
.../AzureFunction/AzureFunction.csproj | 1 -
.../FunctionsHttpProxyingMiddlewareWrapper.cs | 14 +-
.../InvokeFunctionAsyncWrapper.cs | 468 +++++++++---------
.../AzureFunctionHttpTriggerTests.cs | 420 ++++++++--------
...ureFunctionInstrumentationDisabledTests.cs | 13 +-
.../AzureFunctionQueueTriggerTests.cs | 191 ++++---
.../AzureFunctionApplicationFixture.cs | 89 ++--
.../Transactions/TransactionTests.cs | 22 +
8 files changed, 617 insertions(+), 601 deletions(-)
diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AzureFunction/AzureFunction.csproj b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AzureFunction/AzureFunction.csproj
index e6de41b2ba..070dd10f1a 100644
--- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AzureFunction/AzureFunction.csproj
+++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AzureFunction/AzureFunction.csproj
@@ -13,7 +13,6 @@
-
diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AzureFunction/FunctionsHttpProxyingMiddlewareWrapper.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AzureFunction/FunctionsHttpProxyingMiddlewareWrapper.cs
index e26016a5c2..4c32e63e70 100644
--- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AzureFunction/FunctionsHttpProxyingMiddlewareWrapper.cs
+++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AzureFunction/FunctionsHttpProxyingMiddlewareWrapper.cs
@@ -1,10 +1,11 @@
// Copyright 2020 New Relic, Inc. All rights reserved.
// SPDX-License-Identifier: Apache-2.0
-using Microsoft.AspNetCore.Http;
using NewRelic.Agent.Api;
using NewRelic.Agent.Extensions.Providers.Wrapper;
+namespace NewRelic.Providers.Wrapper.AzureFunction;
+
public class FunctionsHttpProxyingMiddlewareWrapper : IWrapper
{
private const string WrapperName = "FunctionsHttpProxyingMiddlewareWrapper";
@@ -24,10 +25,11 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins
{
if (agent.Configuration.AzureFunctionModeEnabled)
{
+ dynamic httpContext;
switch (instrumentedMethodCall.MethodCall.Method.MethodName)
{
case "AddHttpContextToFunctionContext":
- var httpContext = (HttpContext)instrumentedMethodCall.MethodCall.MethodArguments[1];
+ httpContext = instrumentedMethodCall.MethodCall.MethodArguments[1];
agent.CurrentTransaction.SetRequestMethod(httpContext.Request.Method);
agent.CurrentTransaction.SetUri(httpContext.Request.Path);
@@ -35,18 +37,14 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins
case "TryHandleHttpResult":
if (!agent.CurrentTransaction.HasHttpResponseStatusCode) // these handlers seem to get called more than once; only set the status code one time
{
- object result = instrumentedMethodCall.MethodCall.MethodArguments[0];
-
- httpContext = (HttpContext)instrumentedMethodCall.MethodCall.MethodArguments[2];
- bool isInvocationResult = (bool)instrumentedMethodCall.MethodCall.MethodArguments[3];
-
+ httpContext = instrumentedMethodCall.MethodCall.MethodArguments[2];
agent.CurrentTransaction.SetHttpResponseStatusCode(httpContext.Response.StatusCode);
}
break;
case "TryHandleOutputBindingsHttpResult":
if (!agent.CurrentTransaction.HasHttpResponseStatusCode) // these handlers seem to get called more than once; only set the status code one time
{
- httpContext = (HttpContext)instrumentedMethodCall.MethodCall.MethodArguments[1];
+ httpContext = instrumentedMethodCall.MethodCall.MethodArguments[1];
agent.CurrentTransaction.SetHttpResponseStatusCode(httpContext.Response.StatusCode);
}
break;
diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AzureFunction/InvokeFunctionAsyncWrapper.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AzureFunction/InvokeFunctionAsyncWrapper.cs
index 47ac643df9..457219d2a8 100644
--- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AzureFunction/InvokeFunctionAsyncWrapper.cs
+++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AzureFunction/InvokeFunctionAsyncWrapper.cs
@@ -12,318 +12,316 @@
using NewRelic.Agent.Extensions.Providers.Wrapper;
using NewRelic.Reflection;
-namespace NewRelic.Providers.Wrapper.AzureFunction
+namespace NewRelic.Providers.Wrapper.AzureFunction;
+
+public class InvokeFunctionAsyncWrapper : IWrapper
{
- public class InvokeFunctionAsyncWrapper : IWrapper
- {
- private static MethodInfo _getInvocationResultMethod;
- private static bool _loggedDisabledMessage;
- private const string WrapperName = "AzureFunctionInvokeAsyncWrapper";
+ private static MethodInfo _getInvocationResultMethod;
+ private static bool _loggedDisabledMessage;
+ private const string WrapperName = "AzureFunctionInvokeAsyncWrapper";
- private static bool _coldStart = true;
- private static bool IsColdStart => _coldStart && !(_coldStart = false);
+ private static bool _coldStart = true;
+ private static bool IsColdStart => _coldStart && !(_coldStart = false);
- public bool IsTransactionRequired => false;
+ public bool IsTransactionRequired => false;
- private const string FunctionContextBindingFeatureExtensionsTypeName = "Microsoft.Azure.Functions.Worker.FunctionContextBindingFeatureExtensions";
+ private const string FunctionContextBindingFeatureExtensionsTypeName = "Microsoft.Azure.Functions.Worker.FunctionContextBindingFeatureExtensions";
- public CanWrapResponse CanWrap(InstrumentedMethodInfo methodInfo)
- {
- return new CanWrapResponse(WrapperName.Equals(methodInfo.RequestedWrapperName));
- }
+ public CanWrapResponse CanWrap(InstrumentedMethodInfo methodInfo)
+ {
+ return new CanWrapResponse(WrapperName.Equals(methodInfo.RequestedWrapperName));
+ }
- public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall instrumentedMethodCall, IAgent agent,
- ITransaction transaction)
+ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall instrumentedMethodCall, IAgent agent,
+ ITransaction transaction)
+ {
+ if (!agent.Configuration.AzureFunctionModeEnabled) // bail early if azure function mode isn't enabled
{
- if (!agent.Configuration.AzureFunctionModeEnabled) // bail early if azure function mode isn't enabled
+ if (!_loggedDisabledMessage)
{
- if (!_loggedDisabledMessage)
- {
- agent.Logger.Info("Azure Function mode is not enabled; Azure Functions will not be instrumented.");
- _loggedDisabledMessage = true;
- }
-
- return Delegates.NoOp;
+ agent.Logger.Info("Azure Function mode is not enabled; Azure Functions will not be instrumented.");
+ _loggedDisabledMessage = true;
}
- dynamic functionContext = instrumentedMethodCall.MethodCall.MethodArguments[0];
+ return Delegates.NoOp;
+ }
- if (functionContext == null)
- {
- agent.Logger.Debug($"{WrapperName}: FunctionContext is null, can't instrument this invocation.");
- throw new ArgumentNullException("functionContext");
- }
+ dynamic functionContext = instrumentedMethodCall.MethodCall.MethodArguments[0];
- var functionDetails = new FunctionDetails(functionContext, agent);
- if (!functionDetails.IsValid())
- {
- agent.Logger.Debug($"{WrapperName}: FunctionDetails are invalid, can't instrument this invocation.");
- throw new Exception("FunctionDetails are missing some require information.");
- }
+ if (functionContext == null)
+ {
+ agent.Logger.Debug($"{WrapperName}: FunctionContext is null, can't instrument this invocation.");
+ throw new ArgumentNullException("functionContext");
+ }
- transaction = agent.CreateTransaction(
- isWeb: functionDetails.IsWebTrigger,
- category: "AzureFunction",
- transactionDisplayName: functionDetails.FunctionName,
- doNotTrackAsUnitOfWork: true);
+ var functionDetails = new FunctionDetails(functionContext, agent);
+ if (!functionDetails.IsValid())
+ {
+ agent.Logger.Debug($"{WrapperName}: FunctionDetails are invalid, can't instrument this invocation.");
+ throw new Exception("FunctionDetails are missing some require information.");
+ }
- if (instrumentedMethodCall.IsAsync)
- {
- transaction.AttachToAsync();
- transaction.DetachFromPrimary(); //Remove from thread-local type storage
- }
+ transaction = agent.CreateTransaction(
+ isWeb: functionDetails.IsWebTrigger,
+ category: "AzureFunction",
+ transactionDisplayName: functionDetails.FunctionName,
+ doNotTrackAsUnitOfWork: true);
- if (IsColdStart) // only report this attribute if it's a cold start
- {
- transaction.AddFaasAttribute("faas.coldStart", true);
- }
+ if (instrumentedMethodCall.IsAsync)
+ {
+ transaction.AttachToAsync();
+ transaction.DetachFromPrimary(); //Remove from thread-local type storage
+ }
- transaction.AddFaasAttribute("cloud.resource_id", agent.Configuration.AzureFunctionResourceIdWithFunctionName(functionDetails.FunctionName));
- transaction.AddFaasAttribute("faas.name", functionDetails.FunctionName);
- transaction.AddFaasAttribute("faas.trigger", functionDetails.Trigger);
- transaction.AddFaasAttribute("faas.invocation_id", functionDetails.InvocationId);
+ if (IsColdStart) // only report this attribute if it's a cold start
+ {
+ transaction.AddFaasAttribute("faas.coldStart", true);
+ }
- if (functionDetails.IsWebTrigger && !string.IsNullOrEmpty(functionDetails.RequestMethod))
- {
- transaction.SetRequestMethod(functionDetails.RequestMethod);
- transaction.SetUri(functionDetails.RequestPath);
- }
+ transaction.AddFaasAttribute("cloud.resource_id", agent.Configuration.AzureFunctionResourceIdWithFunctionName(functionDetails.FunctionName));
+ transaction.AddFaasAttribute("faas.name", functionDetails.FunctionName);
+ transaction.AddFaasAttribute("faas.trigger", functionDetails.Trigger);
+ transaction.AddFaasAttribute("faas.invocation_id", functionDetails.InvocationId);
+
+ if (functionDetails.IsWebTrigger && !string.IsNullOrEmpty(functionDetails.RequestMethod))
+ {
+ transaction.SetRequestMethod(functionDetails.RequestMethod);
+ transaction.SetUri(functionDetails.RequestPath);
+ }
- var segment = transaction.StartTransactionSegment(instrumentedMethodCall.MethodCall, functionDetails.FunctionName);
+ var segment = transaction.StartTransactionSegment(instrumentedMethodCall.MethodCall, functionDetails.FunctionName);
- return Delegates.GetAsyncDelegateFor(
- agent,
- segment,
- false,
- InvokeFunctionAsyncResponse,
- TaskContinuationOptions.ExecuteSynchronously);
+ return Delegates.GetAsyncDelegateFor(
+ agent,
+ segment,
+ false,
+ InvokeFunctionAsyncResponse,
+ TaskContinuationOptions.ExecuteSynchronously);
- void InvokeFunctionAsyncResponse(Task responseTask)
+ void InvokeFunctionAsyncResponse(Task responseTask)
+ {
+ try
{
- try
+ if (responseTask.IsFaulted)
{
- if (responseTask.IsFaulted)
- {
- transaction.NoticeError(responseTask.Exception);
- return;
- }
+ transaction.NoticeError(responseTask.Exception);
+ return;
+ }
- // only pull response status code here if it's a web trigger and the Microsoft.Azure.Functions.Worker.Extensions.Http.AspNetCore assembly is not loaded.
- if (functionDetails.IsWebTrigger && functionDetails.HasAspNetCoreExtensionReference != null && !functionDetails.HasAspNetCoreExtensionReference.Value)
+ // only pull response status code here if it's a web trigger and the Microsoft.Azure.Functions.Worker.Extensions.Http.AspNetCore assembly is not loaded.
+ if (functionDetails.IsWebTrigger && functionDetails.HasAspNetCoreExtensionReference != null && !functionDetails.HasAspNetCoreExtensionReference.Value)
+ {
+ if (_getInvocationResultMethod == null)
{
- if (_getInvocationResultMethod == null)
- {
- // GetInvocationResult is a static extension method
- // there are multiple GetInvocationResult methods in this type; we want the one without any generic parameters
- Type type = functionContext.GetType().Assembly.GetType(FunctionContextBindingFeatureExtensionsTypeName);
- _getInvocationResultMethod = type.GetMethods().Single(m => m.Name == "GetInvocationResult" && !m.ContainsGenericParameters);
- }
+ // GetInvocationResult is a static extension method
+ // there are multiple GetInvocationResult methods in this type; we want the one without any generic parameters
+ Type type = functionContext.GetType().Assembly.GetType(FunctionContextBindingFeatureExtensionsTypeName);
+ _getInvocationResultMethod = type.GetMethods().Single(m => m.Name == "GetInvocationResult" && !m.ContainsGenericParameters);
+ }
- dynamic invocationResult = _getInvocationResultMethod.Invoke(null, new[] { functionContext });
- var result = invocationResult?.Value;
+ dynamic invocationResult = _getInvocationResultMethod.Invoke(null, new[] { functionContext });
+ var result = invocationResult?.Value;
- if (result != null && result.StatusCode != null)
- {
- var statusCode = result.StatusCode;
- transaction.SetHttpResponseStatusCode((int)statusCode);
- }
+ if (result != null && result.StatusCode != null)
+ {
+ var statusCode = result.StatusCode;
+ transaction.SetHttpResponseStatusCode((int)statusCode);
}
}
- catch (Exception ex)
- {
- agent.Logger.Warn(ex, "Error processing Azure Function response.");
- throw;
- }
- finally
- {
- segment.End();
- transaction.End();
- }
+ }
+ catch (Exception ex)
+ {
+ agent.Logger.Warn(ex, "Error processing Azure Function response.");
+ throw;
+ }
+ finally
+ {
+ segment.End();
+ transaction.End();
}
}
}
+}
- internal class FunctionDetails
- {
- private static MethodInfo _bindFunctionInputAsync;
- private static MethodInfo _genericFunctionInputBindingFeatureGetter;
- private static bool? _hasAspNetCoreExtensionsReference;
+internal class FunctionDetails
+{
+ private static MethodInfo _bindFunctionInputAsync;
+ private static MethodInfo _genericFunctionInputBindingFeatureGetter;
+ private static bool? _hasAspNetCoreExtensionsReference;
- private static readonly ConcurrentDictionary _functionTriggerCache = new();
- private static Func