Handle missing bytecode instrumentation (#1500)

* Avoid crash if bytecode instrumentation type is missing

* Renoved file created by VS

* Update changelog

* Minor clean-up on cor_profiler.cpp

* Fix implicit conversion of NULL const to mdToken

* Fix HResultStr helper on non-Windows

CHANGELOG.md

@@ -64,6 +64,8 @@ This component adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.h
  for the first few executions of the method to not be instrumented. See
  issue [#1242](https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/issues/1242).
 - Span kind for GraphQL instrumentation is set as span property instead of attribute.
+- Application crash if "wrapper type" from bytecode instrumentation is missing
+ [#1469](https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/issues/1469).
 
 ## [0.3.1-beta.1](https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/releases/tag/v0.3.1-beta.1)
 

src/OpenTelemetry.AutoInstrumentation.Native/cor_profiler.cpp

@@ -569,8 +569,18 @@ HRESULT STDMETHODCALLTYPE CorProfiler::ModuleLoadFinished(ModuleID module_id, HR
     // store module info for later lookup
     module_id_to_info_map_[module_id] = module_metadata;
 
-    // We call the function to analyze the module and request the ReJIT of integrations defined in this module.
-    CallTarget_RequestRejitForModule(module_id, module_metadata, integration_methods_);
+    if (module_info.assembly.name == managed_profiler_name)
+    {
+        // If we want to rewrite metadata tokens on the instrumentation assembly it will be
+        // necessary to ReJIT it. However, since that is not done at this moment it is not
+        // necessary to scan for targets to be instrumented on it.
+        managed_profiler_module_id_ = module_id;
+    }
+    else
+    {
+        // We call the function to analyze the module and request the ReJIT of integrations defined in this module.
+        CallTarget_RequestRejitForModule(module_id, module_metadata, integration_methods_);
+    }
 
     Logger::Debug("ModuleLoadFinished stored metadata for ", module_id, " ", module_info.assembly.name, " AppDomain ",
                   module_info.assembly.app_domain_id, " [", module_info.assembly.app_domain_name, "]");
@@ -587,7 +597,6 @@ HRESULT STDMETHODCALLTYPE CorProfiler::ModuleLoadFinished(ModuleID module_id, HR
     return S_OK;
 }
 
-
 HRESULT STDMETHODCALLTYPE CorProfiler::ModuleUnloadStarted(ModuleID module_id)
 {
     auto _ = trace::Stats::Instance()->ModuleUnloadStartedMeasure();
@@ -2453,13 +2462,60 @@ HRESULT CorProfiler::CallTarget_RewriterCallback(RejitHandlerModule* moduleHandl
 {
     auto _ = trace::Stats::Instance()->CallTargetRewriterCallbackMeasure();
 
+    FunctionInfo* caller = methodHandler->GetFunctionInfo();
+
+    // Ensure that the replacement is actually available and found.
+    if (!managed_profiler_module_id_)
+    {
+        Logger::Error("*** CallTarget_RewriterCallback() Error instrumenting: ", caller->type.name, ".", caller->name, "() ",
+                      "managed profiler module was not loaded yet.");
+        return S_FALSE;
+    }
+
+    HRESULT hr = S_OK;
+    MethodReplacement* method_replacement = methodHandler->GetMethodReplacement();
+    {
+        // keep this lock until we are done using the module,
+        // to prevent it from unloading while in use
+        std::lock_guard<std::mutex> guard(module_id_to_info_map_lock_);
+
+        ModuleMetadata* instrumentation_module_metadata = nullptr;
+        auto findRes = module_id_to_info_map_.find(managed_profiler_module_id_);
+        if (findRes != module_id_to_info_map_.end())
+        {
+            instrumentation_module_metadata = findRes->second;
+        }
+
+        if (instrumentation_module_metadata == nullptr)
+        {
+            Logger::Error("*** CallTarget_RewriterCallback() Error instrumenting: ", caller->type.name, ".", caller->name,
+                          "() managed profiler module metadata was not found.");
+            return S_FALSE;
+        }
+
+        auto wrapper = method_replacement->wrapper_method;
+        mdTypeDef wrapper_type_def = mdTypeDefNil;
+        hr = instrumentation_module_metadata->metadata_import->FindTypeDefByName(
+            wrapper.type_name.c_str(),
+            mdTokenNil, // The wrapper type can't be a nested type.
+            &wrapper_type_def);
+        if (FAILED(hr) || wrapper_type_def == mdTypeDefNil)
+        {
+            Logger::Warn("*** CallTarget_RewriterCallback() Failed for: ", caller->type.name, ".", caller->name,
+                        "() integration type not found on the managed profiler module HRESULT=", HResultStr(hr),
+                        " IntegrationType=", wrapper.type_name);
+            return S_FALSE;
+        }
+
+        // TODO: Use the method signature to validate also the method, tracking via
+        // https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/issue/1499 
+    }
+
     ModuleID module_id = moduleHandler->GetModuleId();
     ModuleMetadata* module_metadata = moduleHandler->GetModuleMetadata();
-    FunctionInfo* caller = methodHandler->GetFunctionInfo();
     CallTargetTokens* callTargetTokens = module_metadata->GetCallTargetTokens();
     mdToken function_token = caller->id;
     FunctionMethodArgument retFuncArg = caller->method_signature.GetRet();
-    MethodReplacement* method_replacement = methodHandler->GetMethodReplacement();
     unsigned int retFuncElementType;
     int retTypeFlags = retFuncArg.GetTypeFlags(retFuncElementType);
     bool isVoid = (retTypeFlags & TypeFlagVoid) > 0;
@@ -2494,7 +2550,7 @@ HRESULT CorProfiler::CallTarget_RewriterCallback(RejitHandlerModule* moduleHandl
     // *** Create rewriter
     ILRewriter rewriter(this->info_, methodHandler->GetFunctionControl(), module_id, function_token);
     bool modified = false;
-    auto hr = rewriter.Import();
+    hr = rewriter.Import();
     if (FAILED(hr))
     {
         Logger::Warn("*** CallTarget_RewriterCallback(): Call to ILRewriter.Import() failed for ", module_id, " ",

src/OpenTelemetry.AutoInstrumentation.Native/cor_profiler.h

@@ -57,6 +57,7 @@ private:
     //
     std::mutex module_id_to_info_map_lock_;
     std::unordered_map<ModuleID, ModuleMetadata*> module_id_to_info_map_;
+    ModuleID managed_profiler_module_id_ = 0;
 
     //
     // Helper methods

src/OpenTelemetry.AutoInstrumentation.Native/util.cpp

@@ -1,11 +1,14 @@
-#include "util.h"
+#include "util.h" // Keep first to avoid PCH warning.
 
-#include "pal.h"
 #include <cwctype>
+#include <iomanip>
 #include <iterator>
 #include <string>
 #include <vector>
 
+#include "pal.h"
+#include "string.h"
+
 #ifdef MACOS
 extern char** environ;
 #endif
@@ -174,4 +177,16 @@ WSTRING TokenStr(const mdToken* token)
     return s;
 }
 
+WSTRING HResultStr(const HRESULT hr)
+{
+    std::stringstream ss;
+    ss << "0x"
+       << std::setfill('0')
+       << std::setw(2*sizeof(HRESULT))
+       << std::hex
+       << hr;
+
+    return ToWSTRING(ss.str());
+}
+
 } // namespace trace

src/OpenTelemetry.AutoInstrumentation.Native/util.h

@@ -41,6 +41,9 @@ WSTRING HexStr(const void* data, int len);
 // Convert Token to string
 WSTRING TokenStr(const mdToken* token);
 
+// Convert HRESULT to a friendly string, e.g.: "0x80000002"
+WSTRING HResultStr(const HRESULT hr);
+
 template <class Container>
 bool Contains(const Container& items, const typename Container::value_type& value)
 {

test/IntegrationTests/StrongNamedTestsIntegrations.json

@@ -24,6 +24,28 @@
           "type": "OpenTelemetry.AutoInstrumentation.Instrumentations.Validations.StrongNamedValidation",
           "action": "CallTargetModification"
         }
+      },
+      {
+        "caller": {},
+        "target": {
+          "assembly": "TestLibrary.InstrumentationTarget",
+          "type": "TestLibrary.InstrumentationTarget.Command",
+          "method": "InstrumentationTargetMissingBytecodeInstrumentationType",
+          "signature_types": [
+            "System.Void"
+          ],
+          "minimum_major": 1,
+          "minimum_minor": 0,
+          "minimum_patch": 0,
+          "maximum_major": 1,
+          "maximum_minor": 65535,
+          "maximum_patch": 65535
+        },
+        "wrapper": {
+          "assembly": "OpenTelemetry.AutoInstrumentation",
+          "type": "OpenTelemetry.AutoInstrumentation.Instrumentations.Validations.MissingInstrumentationType",
+          "action": "CallTargetModification"
+        }
       }
     ]
   }

test/test-applications/integrations/TestApplication.StrongNamed/Program.cs

@@ -24,5 +24,6 @@ public static class Program
     {
         var command = new Command();
         command.Execute();
+        command.InstrumentationTargetMissingBytecodeInstrumentationType();
     }
 }

test/test-applications/integrations/dependency-libs/TestLibrary.InstrumentationTarget/Command.cs

@@ -24,4 +24,9 @@ public class Command
     {
         Thread.Yield(); // Just to have some call to outside code.
     }
+
+    public void InstrumentationTargetMissingBytecodeInstrumentationType()
+    {
+        Thread.Sleep(0); // Just to have some call to outside code.
+    }
 }