Disable optimizations to avoid AutoMapper error caused by runtime bug (…

…#363) * add AutomapperTest project with Docker support * update .dockerignore file and add to solution items * autoformat code * move event mask into Initialize(), no longer constant * get ICorProfilerInfo3 earlier during Initialize() * add new env var to disable all CLR optimizations * move DisableOptimizations from cor_profiler to clr_helpers * fix parameter name in header file
DataDog · May 30, 2019 · 35e0fa0 · 35e0fa0
1 parent c447361
commit 35e0fa0
Show file tree

Hide file tree

Showing 12 changed files with 222 additions and 44 deletions.
diff --git a/.dockerignore b/.dockerignore
@@ -1,4 +1,11 @@
-.git/
 test/
-.vs/
 deploy/
+
+.circleci/
+.git/
+.github/
+.vs/
+.vscode/
+packages/
+**/bin/
+**/obj/
diff --git a/Datadog.Trace.sln b/Datadog.Trace.sln
@@ -22,6 +22,7 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Datadog.Trace.TestHelpers",
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{FEBCE7DC-9FD1-48A6-B911-71ABB240A030}"
 	ProjectSection(SolutionItems) = preProject
+		.dockerignore = .dockerignore
 		.editorconfig = .editorconfig
 		.gitattributes = .gitattributes
 		.gitignore = .gitignore
@@ -145,6 +146,11 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ExpenseItDemo", "reproducti
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Samples.Elasticsearch.V5", "samples\Samples.Elasticsearch.V5\Samples.Elasticsearch.V5.csproj", "{AD119B05-A092-41AD-B68E-4AE2DB5A96D9}"
 EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "AutomapperTest", "reproductions\AutomapperTest\AutomapperTest.csproj", "{2AD18622-9A02-41B4-915E-798BD64044D5}"
+	ProjectSection(ProjectDependencies) = postProject
+		{C0C8D381-D6B9-4C76-9428-F40F2FA93A9A} = {C0C8D381-D6B9-4C76-9428-F40F2FA93A9A}
+	EndProjectSection
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -485,6 +491,16 @@ Global
 		{AD119B05-A092-41AD-B68E-4AE2DB5A96D9}.Release|x64.Build.0 = Release|x64
 		{AD119B05-A092-41AD-B68E-4AE2DB5A96D9}.Release|x86.ActiveCfg = Release|x86
 		{AD119B05-A092-41AD-B68E-4AE2DB5A96D9}.Release|x86.Build.0 = Release|x86
+		{2AD18622-9A02-41B4-915E-798BD64044D5}.Debug|Any CPU.ActiveCfg = Debug|x86
+		{2AD18622-9A02-41B4-915E-798BD64044D5}.Debug|x64.ActiveCfg = Debug|x64
+		{2AD18622-9A02-41B4-915E-798BD64044D5}.Debug|x64.Build.0 = Debug|x64
+		{2AD18622-9A02-41B4-915E-798BD64044D5}.Debug|x86.ActiveCfg = Debug|x86
+		{2AD18622-9A02-41B4-915E-798BD64044D5}.Debug|x86.Build.0 = Debug|x86
+		{2AD18622-9A02-41B4-915E-798BD64044D5}.Release|Any CPU.ActiveCfg = Release|x86
+		{2AD18622-9A02-41B4-915E-798BD64044D5}.Release|x64.ActiveCfg = Release|x64
+		{2AD18622-9A02-41B4-915E-798BD64044D5}.Release|x64.Build.0 = Release|x64
+		{2AD18622-9A02-41B4-915E-798BD64044D5}.Release|x86.ActiveCfg = Release|x86
+		{2AD18622-9A02-41B4-915E-798BD64044D5}.Release|x86.Build.0 = Release|x86
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -520,6 +536,7 @@ Global
 		{99A62CCF-8E7F-4D57-8383-D38C371C8087} = {65DF5743-B7B5-4BC8-8AB5-9DE596AF3FB8}
 		{D1729F17-99A5-45AF-AB38-72FA6ECCE609} = {550AE553-2BBB-4021-B55A-137EF31A6B1F}
 		{AD119B05-A092-41AD-B68E-4AE2DB5A96D9} = {AA6F5582-3B71-49AC-AA39-8F7815AC46BE}
+		{2AD18622-9A02-41B4-915E-798BD64044D5} = {550AE553-2BBB-4021-B55A-137EF31A6B1F}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {160A1D00-1F5B-40F8-A155-621B4459D78F}

diff --git a/reproductions/AutomapperTest/AutomapperTest.csproj b/reproductions/AutomapperTest/AutomapperTest.csproj
@@ -0,0 +1,31 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <TargetFrameworks>netcoreapp2.2;netcoreapp2.1;netcoreapp2.0</TargetFrameworks>
+    <Platforms>x64;x86</Platforms>
+    <PlatformTarget>$(Platform)</PlatformTarget>
+    <DockerDefaultTargetOS>Linux</DockerDefaultTargetOS>
+    <DockerfileContext>..\..</DockerfileContext>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="AutoMapper" Version="8.1.0" />
+    <PackageReference Include="Microsoft.VisualStudio.Azure.Containers.Tools.Targets" Version="1.7.8" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="..\..\src\Datadog.Trace.ClrProfiler.Managed\Datadog.Trace.ClrProfiler.Managed.csproj" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <None Include="..\..\src\Datadog.Trace.ClrProfiler.Native\bin\$(Configuration)\$(Platform)\**"
+          CopyToOutputDirectory="Always"
+          CopyToPublishDirectory="Always"
+          Link="profiler-lib\%(RecursiveDir)\%(Filename)%(Extension)" />
+    <Content Include="..\..\integrations.json"
+             CopyToOutputDirectory="Always"
+             CopyToPublishDirectory="Always"
+             Link="profiler-lib\integrations.json" />
+  </ItemGroup>
+</Project>
diff --git a/reproductions/AutomapperTest/Dockerfile b/reproductions/AutomapperTest/Dockerfile
@@ -0,0 +1,29 @@
+FROM mcr.microsoft.com/dotnet/core/runtime:2.1-stretch-slim AS base
+ARG TRACER_VERSION=1.2.0
+RUN mkdir -p /opt/datadog
+RUN mkdir -p /var/log/datadog
+RUN curl -L https://github.com/DataDog/dd-trace-dotnet/releases/download/v$TRACER_VERSION/datadog-dotnet-apm-$TRACER_VERSION.tar.gz | tar xzf - -C /opt/datadog
+
+ENV CORECLR_ENABLE_PROFILING=1
+ENV CORECLR_PROFILER={846F5F1C-F9AE-4B07-969E-05C26BC060D8}
+ENV CORECLR_PROFILER_PATH=/opt/datadog/Datadog.Trace.ClrProfiler.Native.so
+ENV DD_INTEGRATIONS=/opt/datadog/integrations.json
+ENV DD_TRACE_ENABLED=true
+
+FROM mcr.microsoft.com/dotnet/core/sdk:2.1-stretch AS build
+WORKDIR "/src"
+COPY ["reproductions/AutomapperTest/AutomapperTest.csproj", "/src/reproductions/AutomapperTest/"]
+COPY ["src/Datadog.Trace.ClrProfiler.Managed/Datadog.Trace.ClrProfiler.Managed.csproj", "/src/src/Datadog.Trace.ClrProfiler.Managed/"]
+COPY ["src/Datadog.Trace/Datadog.Trace.csproj", "/src/src/Datadog.Trace/"]
+RUN dotnet restore "/src/reproductions/AutomapperTest/AutomapperTest.csproj"
+COPY . .
+WORKDIR "/src/reproductions/AutomapperTest"
+RUN dotnet build "AutomapperTest.csproj" -c Release -o /app
+
+FROM build as publish
+RUN dotnet publish "AutomapperTest.csproj" -c Release -o /app
+
+FROM base AS final
+WORKDIR /app
+COPY --from=publish /app .
+ENTRYPOINT ["dotnet", "AutomapperTest.dll"]
diff --git a/reproductions/AutomapperTest/Program.cs b/reproductions/AutomapperTest/Program.cs
@@ -0,0 +1,34 @@
+using System;
+using System.Collections.Generic;
+using AutoMapper;
+using Datadog.Trace.ClrProfiler;
+
+namespace AutomapperTest
+{
+    public class Program
+    {
+        public static void Main()
+        {
+            Console.WriteLine($"Profiler attached: {Instrumentation.ProfilerAttached}");
+
+            Mapper.Initialize(
+                configuration =>
+                {
+                    configuration.CreateMap<Model1, Model2>();
+                });
+
+            Console.WriteLine("Done");
+        }
+    }
+
+    public class Model1
+    {
+        public List<string> Items { get; set; }
+    }
+
+    public class Model2
+    {
+        // changing this to string[] avoids the problem
+        public List<string> Items { get; set; }
+    }
+}
diff --git a/reproductions/AutomapperTest/Properties/launchSettings.json b/reproductions/AutomapperTest/Properties/launchSettings.json
@@ -0,0 +1,22 @@
+{
+  "profiles": {
+    "AutomapperTest": {
+      "commandName": "Project",
+      "nativeDebugging": true,
+      "environmentVariables": {
+        "COR_ENABLE_PROFILING": "1",
+        "COR_PROFILER": "{846F5F1C-F9AE-4B07-969E-05C26BC060D8}",
+        "COR_PROFILER_PATH": "$(ProjectDir)$(OutputPath)profiler-lib\\Datadog.Trace.ClrProfiler.Native.dll",
+
+        "CORECLR_ENABLE_PROFILING": "1",
+        "CORECLR_PROFILER": "{846F5F1C-F9AE-4B07-969E-05C26BC060D8}",
+        "CORECLR_PROFILER_PATH": "$(ProjectDir)$(OutputPath)profiler-lib\\Datadog.Trace.ClrProfiler.Native.dll",
+
+        "DD_INTEGRATIONS": "$(ProjectDir)$(OutputPath)profiler-lib\\integrations.json"
+      }
+    },
+    "Docker": {
+      "commandName": "Docker"
+    }
+  }
+}
diff --git a/reproductions/Directory.Build.props b/reproductions/Directory.Build.props
@@ -0,0 +1,6 @@
+<Project>
+  <PropertyGroup>
+    <IsPackable>false</IsPackable>
+    <GenerateDocumentationFile>false</GenerateDocumentationFile>
+  </PropertyGroup>
+</Project>
diff --git a/src/Datadog.Trace.ClrProfiler.Native/clr_helpers.cpp b/src/Datadog.Trace.ClrProfiler.Native/clr_helpers.cpp
@@ -2,8 +2,10 @@
 
 #include <cstring>
 
+#include "environment_variables.h"
 #include "logging.h"
 #include "macros.h"
+#include "pal.h"
 
 namespace trace {
 
@@ -340,4 +342,26 @@ mdMethodSpec DefineMethodSpec(const ComPtr<IMetaDataEmit2>& metadata_emit,
   return spec;
 }
 
+bool DisableOptimizations() {
+  const auto clr_optimizations_enabled =
+      GetEnvironmentValue(environment::clr_disable_optimizations);
+
+  if (clr_optimizations_enabled == "1"_W ||
+      clr_optimizations_enabled == "true"_W) {
+    return true;
+  }
+
+  if (clr_optimizations_enabled == "0"_W ||
+      clr_optimizations_enabled == "false"_W) {
+    return false;
+  }
+
+#ifdef _WIN32
+  // default to false on Windows
+  return false;
+#else
+  // default to true on Linux
+  return true;
+#endif
+}
 }  // namespace trace
diff --git a/src/Datadog.Trace.ClrProfiler.Native/clr_helpers.h b/src/Datadog.Trace.ClrProfiler.Native/clr_helpers.h
@@ -276,7 +276,7 @@ std::vector<IntegrationMethod> FlattenIntegrations(
 // its not set to the module
 std::vector<IntegrationMethod> FilterIntegrationsByCaller(
     const std::vector<IntegrationMethod>& integrations,
-    const AssemblyInfo module_metadata);
+    const AssemblyInfo assembly);
 
 // FilterIntegrationsByTarget removes any integrations which have a target not
 // referenced by the module's assembly import
@@ -288,6 +288,8 @@ mdMethodSpec DefineMethodSpec(const ComPtr<IMetaDataEmit2>& metadata_emit,
                               const mdToken& token,
                               const MethodSignature& signature);
 
+bool DisableOptimizations();
+
 }  // namespace trace
 
 #endif  // DD_CLR_PROFILER_CLR_HELPERS_H_
diff --git a/src/Datadog.Trace.ClrProfiler.Native/cor_profiler.cpp b/src/Datadog.Trace.ClrProfiler.Native/cor_profiler.cpp
@@ -34,7 +34,8 @@ CorProfiler::Initialize(IUnknown* cor_profiler_info_unknown) {
                         environment::agent_port,
                         environment::env,
                         environment::service_name,
-                        environment::disabled_integrations};
+                        environment::disabled_integrations,
+                        environment::clr_disable_optimizations};
 
   for (auto&& env_var : env_vars) {
     Info("  ", env_var, "=", GetEnvironmentValue(env_var));
@@ -64,6 +65,14 @@ CorProfiler::Initialize(IUnknown* cor_profiler_info_unknown) {
     }
   }
 
+  // get Profiler interface
+  HRESULT hr = cor_profiler_info_unknown->QueryInterface<ICorProfilerInfo3>(
+      &this->info_);
+  if (FAILED(hr)) {
+    Warn("Failed to attach profiler: interface ICorProfilerInfo3 not found.");
+    return E_FAIL;
+  }
+
   // get path to integration definition JSON files
   const WSTRING integrations_paths =
       GetEnvironmentValue(environment::integrations_path);
@@ -92,16 +101,18 @@ CorProfiler::Initialize(IUnknown* cor_profiler_info_unknown) {
     return E_FAIL;
   }
 
-  // get Profiler interface
-  HRESULT hr = cor_profiler_info_unknown->QueryInterface<ICorProfilerInfo3>(
-      &this->info_);
-  if (FAILED(hr)) {
-    Warn("Failed to attach profiler: interface ICorProfilerInfo3 not found.");
-    return E_FAIL;
+  DWORD event_mask = COR_PRF_MONITOR_JIT_COMPILATION |
+                     COR_PRF_DISABLE_TRANSPARENCY_CHECKS_UNDER_FULL_TRUST |
+                     COR_PRF_DISABLE_INLINING | COR_PRF_MONITOR_MODULE_LOADS |
+                     COR_PRF_DISABLE_ALL_NGEN_IMAGES;
+
+  if (DisableOptimizations()) {
+    Info("Disabling all code optimizations.");
+    event_mask |= COR_PRF_DISABLE_OPTIMIZATIONS;
   }
 
   // set event mask to subscribe to events and disable NGEN images
-  hr = this->info_->SetEventMask(kEventMask);
+  hr = this->info_->SetEventMask(event_mask);
   if (FAILED(hr)) {
     Warn("Failed to attach profiler: unable to set event mask.");
     return E_FAIL;
@@ -202,23 +213,23 @@ HRESULT STDMETHODCALLTYPE CorProfiler::ModuleLoadFinished(ModuleID module_id,
                                    assembly_emit);
 
   for (const auto& integration : filtered_integrations) {
-      // for each wrapper assembly, emit an assembly reference
-      hr = metadata_builder.EmitAssemblyRef(
-          integration.replacement.wrapper_method.assembly);
-      if (FAILED(hr)) {
-        Warn("CorProfiler::ModuleLoadFinished: ", module_info.assembly.name,
-             ". Failed to emit wrapper assembly ref.");
-        return S_OK;
-      }
+    // for each wrapper assembly, emit an assembly reference
+    hr = metadata_builder.EmitAssemblyRef(
+        integration.replacement.wrapper_method.assembly);
+    if (FAILED(hr)) {
+      Warn("CorProfiler::ModuleLoadFinished: ", module_info.assembly.name,
+           ". Failed to emit wrapper assembly ref.");
+      return S_OK;
+    }
 
-      // for each method replacement in each enabled integration,
-      // emit a reference to the instrumentation wrapper methods
-      hr = metadata_builder.StoreWrapperMethodRef(integration.replacement);
-      if (FAILED(hr)) {
-        Warn("CorProfiler::ModuleLoadFinished: ", module_info.assembly.name,
-             ". Failed to emit and store wrapper method ref.");
-        return S_OK;
-      }
+    // for each method replacement in each enabled integration,
+    // emit a reference to the instrumentation wrapper methods
+    hr = metadata_builder.StoreWrapperMethodRef(integration.replacement);
+    if (FAILED(hr)) {
+      Warn("CorProfiler::ModuleLoadFinished: ", module_info.assembly.name,
+           ". Failed to emit and store wrapper method ref.");
+      return S_OK;
+    }
   }
 
   // store module info for later lookup

diff --git a/src/Datadog.Trace.ClrProfiler.Native/cor_profiler.h b/src/Datadog.Trace.ClrProfiler.Native/cor_profiler.h
@@ -9,25 +9,13 @@
 #include "corprof.h"
 
 #include "cor_profiler_base.h"
+#include "environment_variables.h"
 #include "integration.h"
 #include "module_metadata.h"
+#include "pal.h"
 
 namespace trace {
 
-const DWORD kEventMask =
-    COR_PRF_MONITOR_JIT_COMPILATION |
-    COR_PRF_DISABLE_TRANSPARENCY_CHECKS_UNDER_FULL_TRUST | /* helps the case
-                                                                where this
-                                                                profiler is
-                                                                used on Full
-                                                                CLR */
-    COR_PRF_DISABLE_INLINING |
-    COR_PRF_MONITOR_MODULE_LOADS |
-    // COR_PRF_MONITOR_ASSEMBLY_LOADS |
-    // COR_PRF_MONITOR_APPDOMAIN_LOADS |
-    // COR_PRF_ENABLE_REJIT |
-    COR_PRF_DISABLE_ALL_NGEN_IMAGES;
-
 class CorProfiler : public CorProfilerBase {
  private:
   bool is_attached_ = false;

diff --git a/src/Datadog.Trace.ClrProfiler.Native/environment_variables.h b/src/Datadog.Trace.ClrProfiler.Native/environment_variables.h
@@ -38,9 +38,9 @@ const WSTRING env = "DD_ENV"_W;
 // from application name (e.g. entry assembly or IIS application name).
 const WSTRING service_name = "DD_SERVICE_NAME"_W;
 
-// Sets a list of integrations to disable. All other integrations will remain enabled.
-// If not set (default), all integrations are enabled.
-// Supports multiple values separated with semi-colons, for example:
+// Sets a list of integrations to disable. All other integrations will remain
+// enabled. If not set (default), all integrations are enabled. Supports
+// multiple values separated with semi-colons, for example:
 // "ElasticsearchNet;AspNetWebApi2"
 const WSTRING disabled_integrations = "DD_DISABLED_INTEGRATIONS"_W;
 
@@ -50,6 +50,13 @@ const WSTRING disabled_integrations = "DD_DISABLED_INTEGRATIONS"_W;
 // "/var/log/datadog/dotnet-profiler.log" on Linux.
 const WSTRING log_path = "DD_TRACE_LOG_PATH"_W;
 
+// Sets whether to disable all optimizations.
+// Default is false on Windows.
+// Default is true on Linux to work around a bug in the JIT compiler.
+// https://github.com/dotnet/coreclr/issues/24676
+// https://github.com/dotnet/coreclr/issues/12468
+const WSTRING clr_disable_optimizations = "DD_CLR_DISABLE_OPTIMIZATIONS"_W;
+
 }  // namespace environment
 }  // namespace trace