Inject startup hook assembly if DOTNET_STARTUP_HOOKS not configured #4529

Rhynier · 2025-10-16T00:12:54Z

Resolves #4491

Why

To use the auto instrumentation customers currently have to set both the profiler environment variables as well as the DOTNET_STARTUP_HOOKS environment variable. This really does not add any additional value to setup and is just a redundant step. The profiler could just ensure that the auto instrumentation initialization occurs.

Tests

Checklist

CHANGELOG.md is updated.
Documentation is updated.
New features are covered by tests.

…s not configured

linux-foundation-easycla · 2025-10-16T00:13:00Z

The committers listed above are authorized under a signed CLA.

✅ login: Rhynier / name: Rhynier (0eca677, aa26c28, c9e216b, d770791, e99eeaf)

iskiselev · 2025-10-16T00:47:54Z

src/OpenTelemetry.AutoInstrumentation.Native/cor_profiler.cpp

 }

+// clang-format off
+// This method will generate new type __DDLoaderFixup__ in target module.


Should we use new prefixm not DD for generated code?

I thought __DD was some kind of convention, not arbitrarily chosen. If the idea is to have a random pair of letters as prefix I can change it.

iskiselev · 2025-10-16T00:50:42Z

src/OpenTelemetry.AutoInstrumentation.Native/pal.h

+        return EmptyWStr;
+    }
+
+#ifdef _WIN32


Are we linking old stdlib on linux that doesn't have path?

I don't know. Will have to look into this. Are you saying that older stdlib versions returned null or that they returned the module name without path?

It was a question, why can't we use std:path on Linux also?

iskiselev · 2025-10-16T01:04:07Z

src/OpenTelemetry.AutoInstrumentation.Native/pal.h

    return EmptyWStr;
 }

+inline WSTRING GetModuleFilePath(const WSTRING& moduleName)


As I understand, there are at least 3 possible layouts:

ZIP: OTEL_HOME/<RID>/<native_profler_lib> OTEL_HOME/net/OpenTelemetry.AutoInstrumentation.StartupHook.dll

NuGet, platform dependent deployment: <APP_FOLDER>/<native_profler_lib> <APP_FOLDER>/OpenTelemetry.AutoInstrumentation.StartupHook.dll

NuGet, platform independent deployment: (I suspect this is not validated correctly in IsStartupHookValid) <APP_FOLDER>/runtimes/<RID>/<native_profler_lib> <APP_FOLDER>/OpenTelemetry.AutoInstrumentation.StartupHook.dll

I'm also not sure, what is behaviour if OTEL_HOME envirinment variable not set, especially in case of zip deployment.

In any case, once we calculated the path - we should probe that file exists.

Probably, we should apply manual add of profiler hook only if OpenTelemetry.AutoInstrumentation.StartupHook.dll is not present in environment variable at all. If it is there, but points to unexpected location, we should just log it.
I really hope that noone will rename the startup hook file :)

This functionality is definitely just a placeholder for now in order to test that the overall idea works. I will update it to be more robust and take different layouts, etc. into consideration.

Regarding the second question, that is already done in the PR as it is right now. The startup_fix_required class member variable was added for that purpose.

I've seen it. Right now startup_fix_required is based on IsStartupHookValid. We probably need it to be even more flexible.
There are situations, that we may emit warning, as we have not identified hook path in environment varaible, but don't want to apply patch, as we still not sure - like we have dll name in env, but in unexpected path. Keep in mind, on windows check should be case insensitive.

iskiselev · 2025-10-16T01:15:20Z

src/OpenTelemetry.AutoInstrumentation.Native/cor_profiler.cpp

+
+    mdAssemblyRef corlib_ref = mdTokenNil;
+    // We need assemblyRef only when we generate type outside of mscorlib
+    if (module_info.assembly.name != mscorlib_assemblyName)


You never need corlib_ref, as that modification should be done inside corlib, as I understand - similat to ModifyAppDomainCreate.

iskiselev · 2025-10-16T01:21:15Z

src/OpenTelemetry.AutoInstrumentation.Native/cor_profiler.cpp

+                                                 &path_separator_token);
+            if (FAILED(hr))
+            {
+                Logger::Warn("GenerateHookFixup: DefineUserString ; failed");


Ptobably typo "; "

iskiselev · 2025-10-16T01:29:06Z

src/OpenTelemetry.AutoInstrumentation.Native/cor_profiler.cpp

+
+        // Generate IL for the method body
+        // C# equivalent:
+        // if (startupHooks == null)


I'll will add the comment, making sure it matches the generated code.

iskiselev · 2025-10-16T01:30:22Z

src/OpenTelemetry.AutoInstrumentation.Native/otel_profiler_constants.h

+#ifdef _WIN32
+const WSTRING opentelemetry_autoinstrumentation_startuphook_filepath = WStr("net\\OpenTelemetry.AutoInstrumentation.StartupHook.dll");
+#else
+const WSTRING opentelemetry_autoinstrumentation_startuphook_filepath = WStr("net/OpenTelemetry.AutoInstrumentation.StartupHook.dll");


Should we use that constant in IsStartupHookValid if we define it?

Good point. I will update the code to use the same constants.

iskiselev · 2025-10-16T01:33:09Z

src/OpenTelemetry.AutoInstrumentation.Native/cor_profiler.cpp

+        ILInstr* pFirstInstr = rewriter.GetILList()->m_pNext;
+        ILInstr* pNewInstr   = NULL;
+
+        // ldarga.s 0  ; Load the address of the first argument (startupHooks string)


Would it be easier to do all checks/modifications here, instead of creating additional class / method?

In theory, might we not have issues with method body resizing if we add too much code? I know there are things to be aware of but don't know the ins and outs as well as you do.

Other than that it is a good idea. I had originally planned to have more logic in the generated method but then realized I can resolve the assembly name in C++ and just hardcode it in the IL.

I'm not aware of any issues with changing method body size.

RassK · 2025-10-16T13:28:42Z

src/OpenTelemetry.AutoInstrumentation.Native/cor_profiler.cpp

        if (!IsStartupHookValid(startup_hooks, home_path))
        {
-            FailProfiler(Error, "The required StartupHook was not configured correctly. No telemetry will be captured.")
+            Logger::Info("The StartupHook was not configured. Will patch ProcessStartupHooks.");


Is it too late to inject process level env in this point?
Just looking all that code that is necessary if there are simpler workarounds.

Unfortunately the CLR loads environment variables and caches them before it initializes the profiler. Injection is the only way we can overcome that.

Add ability to inject startup hook assembly if DOTNET_STARTUP_HOOKS i…

0eca677

…s not configured

Rhynier and others added 2 commits October 15, 2025 17:18

Remove unnecessary todo

d770791

Merge branch 'main' into auto-startup-hook

aa26c28

iskiselev reviewed Oct 16, 2025

View reviewed changes

RassK reviewed Oct 16, 2025

View reviewed changes

Rhynier and others added 2 commits October 16, 2025 12:19

Merge branch 'main' into auto-startup-hook

e99eeaf

Merge branch 'main' into auto-startup-hook

c9e216b

Inject startup hook assembly if DOTNET_STARTUP_HOOKS not configured #4529

Are you sure you want to change the base?

Inject startup hook assembly if DOTNET_STARTUP_HOOKS not configured #4529

Uh oh!

Conversation

Rhynier commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

Tests

Checklist

Uh oh!

linux-foundation-easycla bot commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iskiselev Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Rhynier commented Oct 16, 2025 •

edited

Loading

linux-foundation-easycla bot commented Oct 16, 2025 •

edited

Loading

iskiselev Oct 16, 2025 •

edited

Loading