diff --git a/src/Agent/NewRelic/Agent/Core/Wrapper/WrapperService.cs b/src/Agent/NewRelic/Agent/Core/Wrapper/WrapperService.cs index f263789c0..0bff391d5 100644 --- a/src/Agent/NewRelic/Agent/Core/Wrapper/WrapperService.cs +++ b/src/Agent/NewRelic/Agent/Core/Wrapper/WrapperService.cs @@ -10,6 +10,8 @@ using NewRelic.Agent.Core.Utilities; using NewRelic.Agent.Extensions.Logging; using NewRelic.Agent.Api; +using System.Reflection; +using System.Threading.Tasks; namespace NewRelic.Agent.Core.Wrapper { @@ -88,6 +90,23 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(Type type, string methodNa return null; } + if (isAsync) + { + try + { + var returnType = type.GetMethod(methodName, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static).ReturnType; + if ((returnType != typeof(Task)) && + (!returnType.IsGenericType || (returnType.GetGenericTypeDefinition() != typeof(Task<>)))) + { + Log.Warn("Instrumenting async methods that return a type other than Task or Task<> is not supported and may result in inconsistent data. '{0}' has a return type of '{1}'.", methodName, returnType?.Name); + } + } + catch + { + // Since this is just for logging purposes it doesn't matter if it fails + } + } + _functionIdToWrapper[functionId] = new InstrumentedMethodInfoWrapper(instrumentedMethodInfo, trackedWrapper); GenerateLibraryVersionSupportabilityMetric(instrumentedMethodInfo); } diff --git a/tests/Agent/UnitTests/Core.UnitTest/Wrapper/WrapperService.cs b/tests/Agent/UnitTests/Core.UnitTest/Wrapper/WrapperService.cs index 391d51d85..f5bc7af5f 100644 --- a/tests/Agent/UnitTests/Core.UnitTest/Wrapper/WrapperService.cs +++ b/tests/Agent/UnitTests/Core.UnitTest/Wrapper/WrapperService.cs @@ -10,6 +10,7 @@ using System.Collections.Generic; using NewRelic.Agent.Core.Utilities; using NewRelic.Agent.Api; +using System.Threading.Tasks; namespace NewRelic.Agent.Core.Wrapper { @@ -17,6 +18,7 @@ namespace NewRelic.Agent.Core.Wrapper public class Class_WrapperService { private const uint EmptyTracerArgs = 0; + private const uint AsyncTracerArgs = 1 << 23; private WrapperService _wrapperService; @@ -279,5 +281,60 @@ public void BeforeWrappedMethod_ReturnsNoOp_IfTheCurrentSegmentIsLeaf() }); } } + + private class ValueTaskTestClass + { + public async ValueTask SomeAsyncWork() + { + await Task.Delay(1); + return 0; + } + } + + private class TaskTestClass + { + public async Task SomeOtherAsyncWork() + { + await Task.Delay(1); + return 0; + } + } + + [Test] + public void BeforeWrappedMethod_WarningAboutValueTask() + { + Mock.Arrange(() => _wrapperMap.Get(Arg.IsAny())).Returns(new TrackedWrapper(_transactionRequiredWrapper)); + + var type = typeof(ValueTaskTestClass); + const string methodName = "SomeAsyncWork"; + const string tracerFactoryName = "MyTracer"; + var target = new object(); + var arguments = new object[0]; + + using (var logging = new TestUtilities.Logging()) + { + var afterWrappedMethod = _wrapperService.BeforeWrappedMethod(type, methodName, string.Empty, target, arguments, tracerFactoryName, null, AsyncTracerArgs, 0); + + Assert.That(logging.HasMessageThatContains("is not supported"), Is.True); + } + } + [Test] + public void BeforeWrappedMethod_NoWarningAboutTask() + { + Mock.Arrange(() => _wrapperMap.Get(Arg.IsAny())).Returns(new TrackedWrapper(_transactionRequiredWrapper)); + + var type = typeof(TaskTestClass); + const string methodName = "SomeOtherAsyncWork"; + const string tracerFactoryName = "MyTracer"; + var target = new object(); + var arguments = new object[0]; + + using (var logging = new TestUtilities.Logging()) + { + var afterWrappedMethod = _wrapperService.BeforeWrappedMethod(type, methodName, string.Empty, target, arguments, tracerFactoryName, null, AsyncTracerArgs, 0); + + Assert.That(logging.HasMessageThatContains("is not supported"), Is.False); + } + } } }