integration test refactor, support SqlClient on .NET Core (#113)

* refactor DynamicMethodBuilder.CreateMethodCallDelegate() to get the parameter and return types from the delegate type * restore the stylecop suppression I removed earlier because it's stupid * refactor integration tests to further use the TestHelper as a base class * clean up code * add support for "DD_" prefixes in environment variables (we should phase out "DATADOG_" soon) * cast to CommandBehavior * taking stylecop down a notch * handle instance methods in DynamicMethodBuilder * use DynamicMethodBuilder for SqlServer integration, intercept ExecuteReader(CommandBehavior) * fix Core MVC integration * only output standard output or standard error is there is any text * cache the delegate for the dynamically generated method * inline small CreateDelegate()
DataDog · Sep 11, 2018 · adf6bf4 · adf6bf4
1 parent e930392
commit adf6bf4
Show file tree

Hide file tree

Showing 12 changed files with 179 additions and 104 deletions.
diff --git a/GlobalSuppressions.cs b/GlobalSuppressions.cs
@@ -9,3 +9,5 @@
 [assembly: SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1633:File must have header", Justification = "Reviewed.")]
 [assembly: SuppressMessage("StyleCop.CSharp.ReadabilityRules", "SA1101:Prefix local calls with this", Justification = "Reviewed.")]
 [assembly: SuppressMessage("StyleCop.CSharp.OrderingRules", "SA1200:UsingDirectivesMustBePlacedWithinNamespace", Justification = "Reviewed.")]
+[assembly: SuppressMessage("StyleCop.CSharp.LayoutRules", "SA1512:Single-line comments must not be followed by blank line", Justification = "Reviewed.")]
+[assembly: SuppressMessage("StyleCop.CSharp.LayoutRules", "SA1515:Single-line comment must be preceded by blank line", Justification = "Reviewed.")]
diff --git a/integrations.json b/integrations.json
@@ -84,13 +84,13 @@
           "assembly": "System.Data",
           "type": "System.Data.SqlClient.SqlCommand",
           "method": "ExecuteReader",
-          "signature": [ 32, 2, 12, 82, 8, 11, 82, 91, 14 ]
+          "signature": [ 32, 1, 12, 87, 12, 11, 92 ]
         },
         "wrapper": {
           "assembly": "Datadog.Trace.ClrProfiler.Managed, Version=0.2.2.0, Culture=neutral, PublicKeyToken=def86d061d0d2eeb",
           "type": "Datadog.Trace.ClrProfiler.Integrations.SqlServer",
           "method": "ExecuteReader",
-          "signature": [ 0, 3, 28, 28, 8, 14 ]
+          "signature": [ 0, 2, 28, 28, 8 ]
         }
       }
     ]

diff --git a/src/Datadog.Trace.ClrProfiler.Managed/Datadog.Trace.ClrProfiler.Managed.csproj b/src/Datadog.Trace.ClrProfiler.Managed/Datadog.Trace.ClrProfiler.Managed.csproj
@@ -4,6 +4,7 @@
     <TargetFrameworks>net45;netcoreapp2.0</TargetFrameworks>
     <Version>0.2.2-alpha</Version>
     <RootNamespace>Datadog.Trace.ClrProfiler</RootNamespace>
+    <LangVersion>7.3</LangVersion>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(OS)' != 'Windows_NT'">

diff --git a/src/Datadog.Trace.ClrProfiler.Managed/DynamicMethodBuilder.cs b/src/Datadog.Trace.ClrProfiler.Managed/DynamicMethodBuilder.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Linq;
 using System.Reflection;
 using System.Reflection.Emit;
 
@@ -16,18 +17,43 @@ public static class DynamicMethodBuilder
         /// <typeparam name="TDelegate">A <see cref="Delegate"/> type with the signature of the method to call.</typeparam>
         /// <param name="type">The <see cref="Type"/> that contains the method.</param>
         /// <param name="methodName">The name of the method.</param>
-        /// <param name="returnType">The return <see cref="Type"/> of the method.</param>
-        /// <param name="parameterTypes">An array with the <see cref="Type"/> of each of the method's parameters, in order.</param>
-        /// <param name="isVirtual"><c>true</c> if the dyanmic method should use a virtual method call, <c>false</c> otherwise.</param>
+        /// <param name="isStatic"><c>true</c> if the method is static, <c>false</c> otherwise.</param>
         /// <returns>A <see cref="Delegate"/> that can be used to execute the dynamic method.</returns>
-        public static Delegate CreateMethodCallDelegate<TDelegate>(
+        public static TDelegate CreateMethodCallDelegate<TDelegate>(
             Type type,
             string methodName,
-            Type returnType,
-            Type[] parameterTypes,
-            bool isVirtual)
+            bool isStatic)
+            where TDelegate : Delegate
         {
-            MethodInfo methodInfo = type.GetMethod(methodName, BindingFlags.Static | BindingFlags.Public);
+            Type delegateType = typeof(TDelegate);
+            Type[] genericTypeArguments = delegateType.GenericTypeArguments;
+
+            Type returnType;
+            Type[] parameterTypes;
+
+            if (delegateType.Name.StartsWith("Func`"))
+            {
+                // last generic type argument is the return type
+                returnType = genericTypeArguments.Last();
+                parameterTypes = genericTypeArguments.Take(genericTypeArguments.Length - 1).ToArray();
+            }
+            else if (delegateType.Name.StartsWith("Action`"))
+            {
+                returnType = typeof(void);
+                parameterTypes = genericTypeArguments;
+            }
+            else
+            {
+                throw new Exception($"Only Func<> or Action<> are supported in {nameof(CreateMethodCallDelegate)}.");
+            }
+
+            // find any method that matches by name and parameter types
+            MethodInfo methodInfo = type.GetMethod(
+                methodName,
+                BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static,
+                null,
+                isStatic ? parameterTypes : parameterTypes.Skip(1).ToArray(),
+                null);
 
             if (methodInfo == null)
             {
@@ -67,10 +93,10 @@ public static Delegate CreateMethodCallDelegate<TDelegate>(
                 }
             }
 
-            ilGenerator.Emit(isVirtual ? OpCodes.Callvirt : OpCodes.Call, methodInfo);
+            ilGenerator.Emit(methodInfo.IsVirtual ? OpCodes.Callvirt : OpCodes.Call, methodInfo);
             ilGenerator.Emit(OpCodes.Ret);
 
-            return dynamicMethod.CreateDelegate(typeof(TDelegate));
+            return (TDelegate)dynamicMethod.CreateDelegate(delegateType);
         }
     }
 }
diff --git a/src/Datadog.Trace.ClrProfiler.Managed/Integrations/AspNetCoreMvc2Integration.cs b/src/Datadog.Trace.ClrProfiler.Managed/Integrations/AspNetCoreMvc2Integration.cs
@@ -83,7 +83,11 @@ public static void BeforeAction(
                 {
                     Assembly assembly = actionDescriptor.GetType().GetTypeInfo().Assembly;
                     Type type = assembly.GetType("Microsoft.AspNetCore.Mvc.Internal.MvcCoreDiagnosticSourceExtensions");
-                    _beforeAction = CreateDelegate(type, "BeforeAction");
+
+                    _beforeAction = DynamicMethodBuilder.CreateMethodCallDelegate<Action<object, object, object, object>>(
+                        type,
+                        "BeforeAction",
+                        isStatic: true);
                 }
             }
             catch
@@ -134,7 +138,11 @@ public static void AfterAction(
                 if (_afterAction == null)
                 {
                     Type type = actionDescriptor.GetType().Assembly.GetType("Microsoft.AspNetCore.Mvc.Internal.MvcCoreDiagnosticSourceExtensions");
-                    _afterAction = CreateDelegate(type, "AfterAction");
+
+                    _afterAction = DynamicMethodBuilder.CreateMethodCallDelegate<Action<object, object, object, object>>(
+                        type,
+                        "AfterAction",
+                        isStatic: true);
                 }
             }
             catch
@@ -185,25 +193,5 @@ public void Dispose()
                 _scope?.Dispose();
             }
         }
-
-        private static Action<object, object, object, object> CreateDelegate(Type type, string methodName)
-        {
-            Type returnType = typeof(void);
-
-            Type[] parameterTypes =
-            {
-                typeof(object),
-                typeof(object),
-                typeof(object),
-                typeof(object),
-            };
-
-            return (Action<object, object, object, object>)DynamicMethodBuilder.CreateMethodCallDelegate<Action<object, object, object, object>>(
-                type,
-                methodName,
-                returnType,
-                parameterTypes,
-                false);
-        }
     }
 }
diff --git a/src/Datadog.Trace.ClrProfiler.Managed/Integrations/SqlServer.cs b/src/Datadog.Trace.ClrProfiler.Managed/Integrations/SqlServer.cs
@@ -1,9 +1,6 @@
 using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Reflection;
-using System.Text;
-using System.Threading.Tasks;
+using System.Data;
+using System.Data.Common;
 
 namespace Datadog.Trace.ClrProfiler.Integrations
 {
@@ -13,36 +10,43 @@ namespace Datadog.Trace.ClrProfiler.Integrations
     public static class SqlServer
     {
         private const string OperationName = "sqlserver.query";
+        private static Func<object, CommandBehavior, object> _executeReader;
 
         /// <summary>
         /// ExecuteReader traces any SQL call.
         /// </summary>
         /// <param name="this">The "this" pointer for the method call.</param>
         /// <param name="behavior">The behavior.</param>
-        /// <param name="method">The method.</param>
         /// <returns>The original methods return.</returns>
-        public static object ExecuteReader(dynamic @this, int behavior, string method)
+        public static object ExecuteReader(dynamic @this, int behavior)
         {
+            var command = (DbCommand)@this;
+
+            if (_executeReader == null)
+            {
+                _executeReader = DynamicMethodBuilder.CreateMethodCallDelegate<Func<object, CommandBehavior, object>>(
+                    command.GetType(),
+                    "ExecuteReader",
+                    isStatic: false);
+            }
+
             using (var scope = Tracer.Instance.StartActive(OperationName))
             {
                 // set the scope properties
                 // - row count is not supported so we don't set it
-                scope.Span.ResourceName = @this.CommandText;
+                scope.Span.ResourceName = command.CommandText;
                 scope.Span.Type = SpanTypes.Sql;
-                scope.Span.SetTag(Tags.SqlDatabase, @this.Connection.ConnectionString);
+                scope.Span.SetTag(Tags.SqlDatabase, command.Connection?.ConnectionString);
 
-                dynamic result;
                 try
                 {
-                    result = @this.ExecuteReader(behavior, method);
+                    return _executeReader(command, (CommandBehavior)behavior);
                 }
                 catch (Exception ex)
                 {
                     scope.Span.SetException(ex);
                     throw;
                 }
-
-                return result;
             }
         }
     }

diff --git a/test/Datadog.Trace.ClrProfiler.IntegrationTests/ConsoleCoreTests.cs b/test/Datadog.Trace.ClrProfiler.IntegrationTests/ConsoleCoreTests.cs
@@ -1,10 +1,3 @@
-using System;
-using System.Collections.Generic;
-using System.Diagnostics;
-using System.Diagnostics.CodeAnalysis;
-using System.IO;
-using System.Linq;
-using Datadog.Trace.TestHelpers;
 using Newtonsoft.Json;
 using Xunit;
 using Xunit.Abstractions;
@@ -13,43 +6,22 @@ namespace Datadog.Trace.ClrProfiler.IntegrationTests
 {
     public class ConsoleCoreTests : TestHelper
     {
-        private readonly ITestOutputHelper _output;
-
         public ConsoleCoreTests(ITestOutputHelper output)
+            : base("ConsoleCore", output)
         {
-            _output = output;
         }
 
         [Fact]
-        public void ProfilerAttached()
+        public void ProfilerAttached_MethodReplaced()
         {
-            string standardOutput;
-            string standardError;
-            int exitCode;
-
-            _output.WriteLine($"Platform: {GetPlatform()}");
-            _output.WriteLine($"Configuration: {BuildParameters.Configuration}");
-            _output.WriteLine($"TargetFramework: {BuildParameters.TargetFramework}");
-            _output.WriteLine($".NET Core: {BuildParameters.CoreClr}");
-            _output.WriteLine($"Application: {GetSampleDllPath("ConsoleCore")}");
-            _output.WriteLine($"Profiler DLL: {GetProfilerDllPath()}");
-
-            using (Process process = StartSample("ConsoleCore"))
+            using (ProcessResult processResult = RunSampleAndWaitForExit())
             {
-                standardOutput = process.StandardOutput.ReadToEnd();
-                standardError = process.StandardError.ReadToEnd();
-                process.WaitForExit();
-                exitCode = process.ExitCode;
-            }
+                Assert.True(processResult.ExitCode >= 0, $"Process exited with code {processResult.ExitCode}");
 
-            _output.WriteLine($"StandardOutput:{Environment.NewLine}{standardOutput}");
-            _output.WriteLine($"StandardError:{Environment.NewLine}{standardError}");
-
-            Assert.True(exitCode >= 0, $"Process exited with code {exitCode}");
-
-            dynamic output = JsonConvert.DeserializeObject(standardOutput);
-            Assert.True((bool)output.ProfilerAttached);
-            Assert.Equal(6, (int)output.AddResult);
+                dynamic output = JsonConvert.DeserializeObject(processResult.StandardOutput);
+                Assert.True((bool)output.ProfilerAttached);
+                Assert.Equal(6, (int)output.AddResult);
+            }
         }
     }
 }
diff --git a/test/Datadog.Trace.ClrProfiler.IntegrationTests/ProcessResult.cs b/test/Datadog.Trace.ClrProfiler.IntegrationTests/ProcessResult.cs
@@ -0,0 +1,33 @@
+using System;
+using System.Diagnostics;
+
+namespace Datadog.Trace.ClrProfiler.IntegrationTests
+{
+    public class ProcessResult : IDisposable
+    {
+        public ProcessResult(
+            Process process,
+            string standardOutput,
+            string standardError,
+            int exitCode)
+        {
+            Process = process;
+            StandardOutput = standardOutput;
+            StandardError = standardError;
+            ExitCode = exitCode;
+        }
+
+        public Process Process { get; }
+
+        public string StandardOutput { get; }
+
+        public string StandardError { get; }
+
+        public int ExitCode { get; }
+
+        public void Dispose()
+        {
+            Process?.Dispose();
+        }
+    }
+}
diff --git a/test/Datadog.Trace.ClrProfiler.IntegrationTests/SqlServerTests.cs b/test/Datadog.Trace.ClrProfiler.IntegrationTests/SqlServerTests.cs
@@ -1,4 +1,5 @@
 using Xunit;
+using Xunit.Abstractions;
 
 // EFCore targets netstandard2.0, so it requires net461 or higher or netcoreapp2.0 or higher
 #if !NET452
@@ -7,16 +8,19 @@ namespace Datadog.Trace.ClrProfiler.IntegrationTests
 {
     public class SqlServerTests : TestHelper
     {
+        public SqlServerTests(ITestOutputHelper output)
+            : base("SqlServer", output)
+        {
+        }
+
         [Fact]
         [Trait("Category", "EndToEnd")]
         public void SubmitsTraces()
         {
             using (var agent = new MockTracerAgent())
+            using (ProcessResult processResult = RunSampleAndWaitForExit())
             {
-                using (var process = StartSample("SqlServer"))
-                {
-                    process.WaitForExit();
-                }
+                Assert.True(processResult.ExitCode >= 0, $"Process exited with code {processResult.ExitCode}");
 
                 var spans = agent.GetSpans();
                 Assert.True(spans.Count > 1);