feat: real stack #239

src/Elastic.Apm/Helpers/StacktraceHelper.cs

@@ -2,15 +2,20 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
+using System.Reflection;
+using System.Runtime.CompilerServices;
 using Elastic.Apm.Logging;
 using Elastic.Apm.Model;
 
 namespace Elastic.Apm.Helpers
 {
 	internal static class StacktraceHelper
 	{
+		private const string DefaultAsyncMethodName = "MoveNext";
+
 		/// <summary>
-		/// Turns a System.Diagnostic.StackFrame[] into a <see cref="CapturedStackFrame" /> list which can be reported to the APM Server
+		/// Turns a System.Diagnostic.StackFrame[] into a <see cref="CapturedStackFrame" /> list which can be reported to the APM
+		/// Server
 		/// </summary>
 		/// <param name="frames">The stack frames to rewrite into APM stack traces</param>
 		/// <param name="logger">The logger to emit exceptions on should one occur</param>
@@ -23,13 +28,13 @@ internal static List<CapturedStackFrame> GenerateApmStackTrace(StackFrame[] fram
 			try
 			{
 				retVal.AddRange(from item in frames
-					let fileName = item?.GetMethod()?.DeclaringType?.Assembly?.GetName()?.Name
-					where !string.IsNullOrEmpty(fileName)
 					select new CapturedStackFrame
 					{
-						Function = item?.GetMethod()?.Name,
-						FileName = fileName,
-						Module = item?.GetMethod()?.ReflectedType?.Name,
+						Function = GetRealMethodName(item?.GetMethod()),
+						FileName =
+							item?.GetMethod()
+								?.ReflectedType?.FullName, //see: https://github.com/elastic/apm-agent-dotnet/pull/240#discussion_r289619196
+						Module = item?.GetMethod()?.ReflectedType?.Assembly.FullName,
 						LineNo = item?.GetFileLineNumber() ?? 0
 					});
 			}
@@ -42,7 +47,8 @@ internal static List<CapturedStackFrame> GenerateApmStackTrace(StackFrame[] fram
 		}
 
 		/// <summary>
-		///  Turns an <see cref="Exception"/> into a <see cref="CapturedStackFrame" /> list which can be reported to the APM Server
+		///  Turns an <see cref="Exception" /> into a <see cref="CapturedStackFrame" /> list which can be reported to the APM
+		/// Server
 		/// </summary>
 		/// <param name="exception">The exception to rewrite into APM stack traces</param>
 		/// <param name="logger">The logger to emit exceptions on should one occur</param>
@@ -61,5 +67,41 @@ internal static List<CapturedStackFrame> GenerateApmStackTrace(Exception excepti
 
 			return null;
 		}
+
+		/// <summary>
+		///  Finds real method name even for Async methods, full description of the issue is available here
+		/// https://stackoverflow.com/a/28633192
+		/// </summary>
+		/// <param name="inputMethod">Method to discover</param>
+		/// <returns>A real method name (even for async methods)</returns>
+		private static string GetRealMethodName(MethodBase inputMethod)
+		{
+			if (inputMethod == null)
+				return "";
+
+			if (inputMethod.Name != DefaultAsyncMethodName)
+				return inputMethod.Name;
+
+			var declaredType = inputMethod.DeclaringType;
+
+			if (declaredType == null)
+				return DefaultAsyncMethodName;
+
+			if (declaredType.GetInterfaces().All(i => i != typeof(IAsyncStateMachine)))
+				return DefaultAsyncMethodName;
+
+			var generatedType = inputMethod.DeclaringType;
+			var originalType = generatedType?.DeclaringType;
+
+			if (originalType == null)
+				return DefaultAsyncMethodName;
+
+			var foundMethod = originalType.GetMethods(BindingFlags.Instance | BindingFlags.Static |
+					BindingFlags.Public | BindingFlags.NonPublic |
+					BindingFlags.DeclaredOnly)
+				.FirstOrDefault(m => m.GetCustomAttribute<AsyncStateMachineAttribute>()?.StateMachineType == generatedType);
+
+			return foundMethod?.Name ?? DefaultAsyncMethodName;
+		}
 	}
 }

test/Elastic.Apm.Tests/StackTraceTests.cs

@@ -1,5 +1,8 @@
+using System;
+using System.Diagnostics;
+using System.Linq;
 using System.Net.Http;
-using System.Runtime.InteropServices.ComTypes;
+using System.Runtime.CompilerServices;
 using System.Threading.Tasks;
 using Elastic.Apm.Model;
 using Elastic.Apm.Tests.Mocks;
@@ -36,5 +39,116 @@ public async Task HttpClientStackTrace()
 
 			stackFrames.Should().NotBeEmpty().And.Contain(frame => frame.LineNo != 0);
 		}
+
+		/// <summary>
+		/// Makes sure that the name of the async method is captured correctly
+		/// </summary>
+		[Fact]
+		public async Task AsyncCallStackTest()
+		{
+			var payloadSender = new MockPayloadSender();
+			var agent = new ApmAgent(new TestAgentComponents(payloadSender: payloadSender));
+
+			await Assert.ThrowsAsync<Exception>(async () =>
+			{
+				await agent.Tracer.CaptureTransaction("TestTransaction", "Test", async () =>
+				{
+					var classWithAsync = new ClassWithAsync();
+					await classWithAsync.TestMethodAsync();
+				});
+			});
+
+			payloadSender.Errors.Should().NotBeEmpty();
+			(payloadSender.Errors.First() as Error).Should().NotBeNull();
+			(payloadSender.Errors.First() as Error)?.Exception.Stacktrace.Should().Contain(m => m.Function == nameof(ClassWithAsync.TestMethodAsync));
+		}
+
+		/// <summary>
+		/// Makes sure that if a non-async method is named 'MoveNext', it does not cause any trouble
+		/// </summary>
+		[Fact]
+		public void CallStackWithMoveNextWithoutAsync()
+		{
+			var payloadSender = new MockPayloadSender();
+			var agent = new ApmAgent(new TestAgentComponents(payloadSender: payloadSender));
+
+			Assert.Throws<Exception>(() =>
+			{
+				agent.Tracer.CaptureTransaction("TestTransaction", "Test", () =>
+				{
+					var classWithSyncMethods = new ClassWithSyncMethods();
+					classWithSyncMethods.MoveNext();
+				});
+			});
+
+			payloadSender.Errors.Should().NotBeEmpty();
+			(payloadSender.Errors.First() as Error).Should().NotBeNull();
+			(payloadSender.Errors.First() as Error)?.Exception.Stacktrace.Should().Contain(m => m.Function == nameof(ClassWithSyncMethods.MoveNext));
+			(payloadSender.Errors.First() as Error)?.Exception.Stacktrace.Should().Contain(m => m.Function == nameof(ClassWithSyncMethods.M2));
+		}
+
+		/// <summary>
+		/// Makes sure that the typename and the method name are captured correctly
+		/// </summary>
+		[Fact]
+		public void TypeAndMethodNameTest()
+		{
+			var payloadSender = new MockPayloadSender();
+			var agent = new ApmAgent(new TestAgentComponents(payloadSender: payloadSender));
+
+			Assert.Throws<Exception>(() =>
+			{
+				agent.Tracer.CaptureTransaction("TestTransaction", "Test", () =>
+				{
+					BaseTestClass testClass = new DerivedTestClass();
+					testClass.Method1();
+				});
+			});
+
+			payloadSender.Errors.Should().NotBeEmpty();
+			(payloadSender.Errors.First() as Error).Should().NotBeNull();
+
+			(payloadSender.Errors.First() as Error)?.Exception.Stacktrace.Should()
+				.Contain(m => m.FileName == typeof(BaseTestClass).FullName
+					&& m.Function == nameof(BaseTestClass.Method1)
+					&& m.Module == typeof(BaseTestClass).Assembly.FullName
+				);
+
+			(payloadSender.Errors.First() as Error)?.Exception.Stacktrace.Should()
+				.Contain(m => m.FileName == typeof(DerivedTestClass).FullName
+					&& m.Function == nameof(DerivedTestClass.TestMethod)
+					&& m.Module == typeof(DerivedTestClass).Assembly.FullName);
+		}
+
+		private class ClassWithSyncMethods
+		{
+			[MethodImpl(MethodImplOptions.NoInlining)]
+			internal void MoveNext() => M2();
+
+			[MethodImpl(MethodImplOptions.NoInlining)]
+			internal void M2() => throw new Exception("bamm");
+		}
+
+		private class ClassWithAsync
+		{
+			internal async Task TestMethodAsync()
+			{
+				await Task.Delay(5);
+				throw new Exception("bamm");
+			}
+		}
+	}
+
+	internal class BaseTestClass
+	{
+		internal void Method1() => TestMethod();
+
+		internal virtual void TestMethod()
+			=> Debug.WriteLine("test");
+	}
+
+	internal class DerivedTestClass : BaseTestClass
+	{
+		internal override void TestMethod() => throw new Exception("TestException");
 	}
 }