Fix bug in OTEL_EXPORTER_OTLP_*_PROTOCOL handling for all signals (#4627)

Co-authored-by: Rasmus Kuusmann <[email protected]>

Author: xiang17

CHANGELOG.md

@@ -13,7 +13,7 @@ This component adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.h
 - Support for [ASP.NET Core 10 metrics](https://learn.microsoft.com/en-us/aspnet/core/log-mon/metrics/built-in?view=aspnetcore-10.0).
 - Support for ASP.NET Core 10 Blazor traces from
   `Microsoft.AspNetCore.Components`
-  and `"Microsoft.AspNetCore.Components.Server.Circuits`.
+  and `Microsoft.AspNetCore.Components.Server.Circuits`.
 - Experimental support for file-based configuration.
 - Experimental support for configuration based instrumentation.
 - IL rewrite for SqlCommand on .NET Framework to ensure `CommandText` is
@@ -102,6 +102,10 @@ and runtime, then loads the correct version of dependency assemblies.
   See [#4269](https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/issues/4269)
   for details.
 - Fixed rule engine check for .NET 9 to reflect longer support for [STS channel](https://devblogs.microsoft.com/dotnet/dotnet-sts-releases-supported-for-24-months/).
+- Fix bug in signal specific OTLP exporter variables: `OTEL_EXPORTER_OTLP_TRACES_PROTOCOL`,
+  `OTEL_EXPORTER_OTLP_METRICS_PROTOCOL` and `OTEL_EXPORTER_OTLP_LOGS_PROTOCOL`.
+  See [#4593](https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/issues/4593)
+  for details.
 
 ## [1.13.0-beta.1](https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/releases/tag/v1.13.0-beta.1)
 

src/OpenTelemetry.AutoInstrumentation/AutoInstrumentationEventSource.cs

@@ -40,7 +40,7 @@ public void Information(string message)
         WriteEvent(4, message);
     }
 
-    /// <summary>Logs as Warning level message.</summary>
+    /// <summary>Logs as Verbose level message.</summary>
     /// <param name="message">Message to log.</param>
     [Event(5, Message = "{0}", Level = EventLevel.Verbose)]
     public void Verbose(string message)

src/OpenTelemetry.AutoInstrumentation/Configurations/Otlp/OtlpSettings.cs

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 using OpenTelemetry.AutoInstrumentation.Configurations.FileBasedConfiguration;
+using OpenTelemetry.AutoInstrumentation.Logging;
 using OpenTelemetry.Exporter;
 
 namespace OpenTelemetry.AutoInstrumentation.Configurations.Otlp;
@@ -12,6 +13,8 @@ namespace OpenTelemetry.AutoInstrumentation.Configurations.Otlp;
 /// </summary>
 internal class OtlpSettings
 {
+    private static readonly IOtelLogger Logger = OtelLogging.GetLogger();
+
     public OtlpSettings(OtlpSignalType signalType, Configuration configuration)
     {
         Protocol = GetExporterOtlpProtocol(signalType, configuration);
@@ -156,18 +159,37 @@ private static Uri GetOtlpHttpEndpoint(string? endpoint, OtlpSignalType signalTy
 
     private static OtlpExportProtocol? GetExporterOtlpProtocol(OtlpSignalType signalType, Configuration configuration)
     {
-        // the default in SDK is grpc. http/protobuf should be default for our purposes
+        // http/protobuf should be default for our purposes. Always set a value to avoid relying on SDK, because the default in SDK is grpc.
         var priorityVar = OtlpSpecConfigDefinitions.GetProtocolEnvVar(signalType);
-        var exporterOtlpProtocol = configuration.GetString(priorityVar) ??
-            configuration.GetString(OtlpSpecConfigDefinitions.DefaultProtocolEnvVarName);
+        var defaultVar = OtlpSpecConfigDefinitions.DefaultProtocolEnvVarName;
+        string? usedEnvVarName = priorityVar;
+        var exporterOtlpProtocol = configuration.GetString(priorityVar);
+        if (exporterOtlpProtocol == null)
+        {
+            exporterOtlpProtocol = configuration.GetString(defaultVar);
+            usedEnvVarName = defaultVar;
+        }
 
-        if (string.IsNullOrEmpty(exporterOtlpProtocol))
+        if (!string.IsNullOrEmpty(exporterOtlpProtocol))
         {
-            // override settings only for http/protobuf
-            return OtlpExportProtocol.HttpProtobuf;
+            switch (exporterOtlpProtocol)
+            {
+                case "grpc":
+#if NETFRAMEWORK
+                    Logger.Warning($"OTLP protocol 'grpc' is not supported on .NET Framework in environment variable '{usedEnvVarName}'. Changing to 'http/protobuf' instead.");
+                    return OtlpExportProtocol.HttpProtobuf;
+#else
+                    return OtlpExportProtocol.Grpc;
+#endif
+                case "http/protobuf":
+                    return OtlpExportProtocol.HttpProtobuf;
+                default:
+                    Logger.Warning($"Invalid OTLP protocol value '{exporterOtlpProtocol}' in environment variable '{usedEnvVarName}'. Supported values are 'grpc' and 'http/protobuf'. Defaulting to 'http/protobuf'.");
+                    return OtlpExportProtocol.HttpProtobuf;
+            }
         }
 
-        // null value here means that it will be handled by OTEL .NET SDK
-        return null;
+        // In case of absent value, it will fall back to default value
+        return OtlpExportProtocol.HttpProtobuf;
     }
 }

test/OpenTelemetry.AutoInstrumentation.Tests/Configurations/SettingsTests.cs

@@ -1,6 +1,7 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
+using System.Reflection;
 using OpenTelemetry.AutoInstrumentation.Configurations;
 using OpenTelemetry.Exporter;
 using Xunit;
@@ -379,9 +380,13 @@ internal void IncludeFormattedMessage_DependsOnCorrespondingEnvVariable(string i
     [Theory]
     [InlineData("", OtlpExportProtocol.HttpProtobuf)]
     [InlineData(null, OtlpExportProtocol.HttpProtobuf)]
-    [InlineData("http/protobuf", null)]
-    [InlineData("grpc", null)]
-    [InlineData("nonExistingProtocol", null)]
+    [InlineData("http/protobuf", OtlpExportProtocol.HttpProtobuf)]
+#if NETFRAMEWORK
+    [InlineData("grpc", OtlpExportProtocol.HttpProtobuf)]
+#else
+    [InlineData("grpc", OtlpExportProtocol.Grpc)]
+#endif
+    [InlineData("nonExistingProtocol", OtlpExportProtocol.HttpProtobuf)]
     internal void OtlpExportProtocol_DependsOnCorrespondingEnvVariable(string? otlpProtocol, OtlpExportProtocol? expectedOtlpExportProtocol)
     {
         Environment.SetEnvironmentVariable(AutoOtlpDefinitions.DefaultProtocolEnvVarName, otlpProtocol);
@@ -393,6 +398,78 @@ internal void OtlpExportProtocol_DependsOnCorrespondingEnvVariable(string? otlpP
         Assert.Equal(expectedOtlpExportProtocol, settings.OtlpSettings.Protocol);
     }
 
+    [Theory]
+    [InlineData("http/protobuf", "1", "key1=value1,key2=value2", OtlpExportProtocol.HttpProtobuf, 1)]
+#if NETFRAMEWORK
+    [InlineData("grpc", "15000", "a=b,c=d", OtlpExportProtocol.HttpProtobuf, 15000)]
+#else
+    [InlineData("grpc", "15000", "a=b,c=d", OtlpExportProtocol.Grpc, 15000)]
+#endif
+    [InlineData("invalid", "42", "x=y", OtlpExportProtocol.HttpProtobuf, 42)]
+    internal void OtlpExportProtocol_CheckPriorityEnvIsSet_Traces(string? protocol, string timeout, string headers, OtlpExportProtocol? expectedProtocol, int expectedTimeout)
+    {
+        var unexpectedProtocolForDistraction = expectedProtocol == OtlpExportProtocol.HttpProtobuf ? "grpc" : "http/protobuf";
+        Environment.SetEnvironmentVariable(AutoOtlpDefinitions.DefaultProtocolEnvVarName, unexpectedProtocolForDistraction); // set a different default to verify priority
+        Environment.SetEnvironmentVariable(AutoOtlpDefinitions.TracesProtocolEnvVarName, protocol);
+        Environment.SetEnvironmentVariable(AutoOtlpDefinitions.TracesTimeoutEnvVarName, timeout);
+        Environment.SetEnvironmentVariable(AutoOtlpDefinitions.TracesHeadersEnvVarName, headers);
+
+        var settings = Settings.FromDefaultSources<TracerSettings>(false).OtlpSettings;
+
+        Assert.NotNull(settings);
+        Assert.Equal(expectedProtocol, settings.Protocol);
+        Assert.Equal(expectedTimeout, settings.TimeoutMilliseconds);
+        Assert.Equal(headers, settings.Headers);
+    }
+
+    [Theory]
+    [InlineData("http/protobuf", "1", "key1=value1,key2=value2", OtlpExportProtocol.HttpProtobuf, 1)]
+#if NETFRAMEWORK
+    [InlineData("grpc", "25000", "m=n", OtlpExportProtocol.HttpProtobuf, 25000)]
+#else
+    [InlineData("grpc", "25000", "m=n", OtlpExportProtocol.Grpc, 25000)]
+#endif
+    [InlineData("invalid", "100", "a=b", OtlpExportProtocol.HttpProtobuf, 100)]
+    internal void OtlpExportProtocol_CheckPriorityEnvIsSet_Metrics(string? protocol, string timeout, string headers, OtlpExportProtocol? expectedProtocol, int expectedTimeout)
+    {
+        var unexpectedProtocolForDistraction = expectedProtocol == OtlpExportProtocol.HttpProtobuf ? "grpc" : "http/protobuf";
+        Environment.SetEnvironmentVariable(AutoOtlpDefinitions.DefaultProtocolEnvVarName, unexpectedProtocolForDistraction); // set a different default to verify priority
+        Environment.SetEnvironmentVariable(AutoOtlpDefinitions.MetricsProtocolEnvVarName, protocol);
+        Environment.SetEnvironmentVariable(AutoOtlpDefinitions.MetricsTimeoutEnvVarName, timeout);
+        Environment.SetEnvironmentVariable(AutoOtlpDefinitions.MetricsHeadersEnvVarName, headers);
+
+        var settings = Settings.FromDefaultSources<MetricSettings>(false).OtlpSettings;
+
+        Assert.NotNull(settings);
+        Assert.Equal(expectedProtocol, settings.Protocol);
+        Assert.Equal(expectedTimeout, settings.TimeoutMilliseconds);
+        Assert.Equal(headers, settings.Headers);
+    }
+
+    [Theory]
+    [InlineData("http/protobuf", "1", "key1=value1,key2=value2", OtlpExportProtocol.HttpProtobuf, 1)]
+#if NETFRAMEWORK
+    [InlineData("grpc", "5000", "l=m", OtlpExportProtocol.HttpProtobuf, 5000)]
+#else
+    [InlineData("grpc", "5000", "l=m", OtlpExportProtocol.Grpc, 5000)]
+#endif
+    [InlineData("invalid", "77", "z=zz", OtlpExportProtocol.HttpProtobuf, 77)]
+    internal void OtlpExportProtocol_CheckPriorityEnvIsSet_Logs(string? protocol, string timeout, string headers, OtlpExportProtocol? expectedProtocol, int expectedTimeout)
+    {
+        var unexpectedProtocolForDistraction = expectedProtocol == OtlpExportProtocol.HttpProtobuf ? "grpc" : "http/protobuf";
+        Environment.SetEnvironmentVariable(AutoOtlpDefinitions.DefaultProtocolEnvVarName, unexpectedProtocolForDistraction); // set a different default to verify priority
+        Environment.SetEnvironmentVariable(AutoOtlpDefinitions.LogsProtocolEnvVarName, protocol);
+        Environment.SetEnvironmentVariable(AutoOtlpDefinitions.LogsTimeoutEnvVarName, timeout);
+        Environment.SetEnvironmentVariable(AutoOtlpDefinitions.LogsHeadersEnvVarName, headers);
+
+        var settings = Settings.FromDefaultSources<LogSettings>(false).OtlpSettings;
+
+        Assert.NotNull(settings);
+        Assert.Equal(expectedProtocol, settings.Protocol);
+        Assert.Equal(expectedTimeout, settings.TimeoutMilliseconds);
+        Assert.Equal(headers, settings.Headers);
+    }
+
     [Theory]
     [InlineData("true", true)]
     [InlineData("false", false)]
@@ -477,5 +554,20 @@ private static void ClearEnvVars()
         Environment.SetEnvironmentVariable(ConfigurationKeys.Traces.InstrumentationOptions.HttpInstrumentationCaptureRequestHeaders, null);
         Environment.SetEnvironmentVariable(ConfigurationKeys.Traces.InstrumentationOptions.HttpInstrumentationCaptureResponseHeaders, null);
         Environment.SetEnvironmentVariable(ConfigurationKeys.Traces.InstrumentationOptions.OracleMdaSetDbStatementForText, null);
+
+        // Cleanup OTLP env vars
+        foreach (var envVar in GetAllOtlpEnvVarNames())
+        {
+            Environment.SetEnvironmentVariable(envVar, null);
+        }
+    }
+
+    private static IEnumerable<string> GetAllOtlpEnvVarNames()
+    {
+        return typeof(AutoOtlpDefinitions)
+            .GetFields(BindingFlags.Public | BindingFlags.Static)
+            .Where(f => f.FieldType == typeof(string))
+            .Select(f => (string)f.GetValue(null)!)
+            .ToList() ?? Enumerable.Empty<string>();
     }
 }