open-telemetry / opentelemetry-dotnet-instrumentation

OpenTelemetry .NET Automatic Instrumentation
https://opentelemetry.io
Apache License 2.0
363 stars 92 forks source link

OpenTelemetry.DotNet.Auto.psm1: command Register-OpenTelemetryForIIS should not remove existing env variables in Windows registry #3579

Closed muhaook closed 2 weeks ago

muhaook commented 3 weeks ago

Bug Report

Symptom

Describe the bug I am using powershell script OpenTelemetry.DotNet.Auto.psm1 to deploy dotnet-auto-instrumentation on windows. I found each time I run command Register-OpenTelemetryForIIS, it created a new Computer\HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\W3SVC\Environment in Windows registry. If customer already had some env variables in Windows registry before deploying dotnet-auto-instrumentation, those env variables will be cleaned up.

Paulo Janotti confirmed the issue: it should support only adding and removing or own env vars. Could you please create an issue for that.

slack: https://cloud-native.slack.com/archives/C01NR1YLSE7/p1723813954160729

Runtime environment:

RassK commented 3 weeks ago

It's intentionally cleaning up the environment if there are no variables left. The point is that when it was created, most of the variables were experimental (even today a lot), then not to create migration issues we decided to remove all OTel dotnet variables. (OTELDOTNET* is in the filter).

    [string[]] $filters = @(
        # .NET Framework
        "COR_ENABLE_PROFILING",
        "COR_PROFILER",
        "COR_PROFILER_PATH_32",
        "COR_PROFILER_PATH_64",
        # .NET Core
        "CORECLR_ENABLE_PROFILING",
        "CORECLR_PROFILER",
        "CORECLR_PROFILER_PATH_32",
        "CORECLR_PROFILER_PATH_64",
        # ASP.NET Core
        "ASPNETCORE_HOSTINGSTARTUPASSEMBLIES",
        # .NET Common
        "DOTNET_ADDITIONAL_DEPS",
        "DOTNET_SHARED_STORE",
        "DOTNET_STARTUP_HOOKS",
        # OpenTelemetry
        "OTEL_DOTNET_"
    )
muhaook commented 2 weeks ago

Thanks @RassK , what I found is Setup-Windows-Service will clean up everything in Environment before creating any new OTEL_* variables

RassK commented 2 weeks ago

@muhaook thanks for the details, I see now what you mean. Setup seems to miss migrate path for existing envs, currently it only performs set operation for the computed table, therefore overwriting all existing vars.

Should be kind of similar to remove operation. First get the current env table, we still need to clean up all OTEL_DOTNET_* before applying the computed set of installation vars.