Fix the default endpoint for OTLP exporter when using http/protobuf p…

…rotocol (#2868)
open-telemetry · Feb 15, 2022 · 73b99b6 · 73b99b6
1 parent 84821ff
commit 73b99b6
Show file tree

Hide file tree

Showing 8 changed files with 89 additions and 21 deletions.
diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/ApiCompatBaseline.txt b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/ApiCompatBaseline.txt
@@ -0,0 +1,4 @@
+Compat issues with assembly OpenTelemetry.Exporter.OpenTelemetryProtocol:
+CannotRemoveAttribute : Attribute 'System.Runtime.CompilerServices.CompilerGeneratedAttribute' exists on 'OpenTelemetry.Exporter.OtlpExporterOptions.Endpoint.get()' in the contract but not the implementation.
+CannotRemoveAttribute : Attribute 'System.Runtime.CompilerServices.CompilerGeneratedAttribute' exists on 'OpenTelemetry.Exporter.OtlpExporterOptions.Endpoint.set(System.Uri)' in the contract but not the implementation.
+Total Issues: 2
diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md
@@ -3,7 +3,11 @@
 ## Unreleased
 
 * LogExporter bug fix to handle null EventName.
-  ([#2870](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2871))
+  ([#2871](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2871))
+
+* Fixed the default endpoint for OTLP exporter over HTTP/Protobuf.
+  The default value is `http://localhost:4318`.
+  ([#2868](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2868))
 
 ## 1.2.0-rc2
 

diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs
@@ -44,14 +44,20 @@ public class OtlpExporterOptions
 
         internal readonly Func<HttpClient> DefaultHttpClientFactory;
 
+        private const string DefaultGrpcEndpoint = "http://localhost:4317";
+        private const string DefaultHttpEndpoint = "http://localhost:4318";
+        private const OtlpExportProtocol DefaultOtlpExportProtocol = OtlpExportProtocol.Grpc;
+
+        private Uri endpoint;
+
         /// <summary>
         /// Initializes a new instance of the <see cref="OtlpExporterOptions"/> class.
         /// </summary>
         public OtlpExporterOptions()
         {
-            if (EnvironmentVariableHelper.LoadUri(EndpointEnvVarName, out Uri endpoint))
+            if (EnvironmentVariableHelper.LoadUri(EndpointEnvVarName, out Uri parsedEndpoint))
             {
-                this.Endpoint = endpoint;
+                this.endpoint = parsedEndpoint;
             }
 
             if (EnvironmentVariableHelper.LoadString(HeadersEnvVarName, out string headersEnvVar))
@@ -79,7 +85,7 @@ public OtlpExporterOptions()
 
             this.HttpClientFactory = this.DefaultHttpClientFactory = () =>
             {
-                return new HttpClient()
+                return new HttpClient
                 {
                     Timeout = TimeSpan.FromMilliseconds(this.TimeoutMilliseconds),
                 };
@@ -89,9 +95,30 @@ public OtlpExporterOptions()
         /// <summary>
         /// Gets or sets the target to which the exporter is going to send telemetry.
         /// Must be a valid Uri with scheme (http or https) and host, and
-        /// may contain a port and path. The default value is http://localhost:4317.
+        /// may contain a port and path. The default value is
+        /// * http://localhost:4317 for <see cref="OtlpExportProtocol.Grpc"/>
+        /// * http://localhost:4318 for <see cref="OtlpExportProtocol.HttpProtobuf"/>.
         /// </summary>
-        public Uri Endpoint { get; set; } = new Uri("http://localhost:4317");
+        public Uri Endpoint
+        {
+            get
+            {
+                if (this.endpoint == null)
+                {
+                    this.endpoint = this.Protocol == OtlpExportProtocol.Grpc
+                        ? new Uri(DefaultGrpcEndpoint)
+                        : new Uri(DefaultHttpEndpoint);
+                }
+
+                return this.endpoint;
+            }
+
+            set
+            {
+                this.endpoint = value;
+                this.ProgrammaticallyModifiedEndpoint = true;
+            }
+        }
 
         /// <summary>
         /// Gets or sets optional headers for the connection. Refer to the <a href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#specifying-headers-via-environment-variables">
@@ -107,7 +134,7 @@ public OtlpExporterOptions()
         /// <summary>
         /// Gets or sets the the OTLP transport protocol. Supported values: Grpc and HttpProtobuf.
         /// </summary>
-        public OtlpExportProtocol Protocol { get; set; } = OtlpExportProtocol.Grpc;
+        public OtlpExportProtocol Protocol { get; set; } = DefaultOtlpExportProtocol;
 
         /// <summary>
         /// Gets or sets the export processor type to be used with the OpenTelemetry Protocol Exporter. The default value is <see cref="ExportProcessorType.Batch"/>.
@@ -167,5 +194,10 @@ public OtlpExporterOptions()
         /// </list>
         /// </remarks>
         public Func<HttpClient> HttpClientFactory { get; set; }
+
+        /// <summary>
+        /// Gets a value indicating whether <see cref="Endpoint" /> was modified via its setter.
+        /// </summary>
+        internal bool ProgrammaticallyModifiedEndpoint { get; private set; }
     }
 }
diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptionsExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptionsExtensions.cs
@@ -155,12 +155,12 @@ public static void TryEnableIHttpClientFactoryIntegration(this OtlpExporterOptio
             }
         }
 
-        internal static void AppendExportPath(this OtlpExporterOptions options, Uri initialEndpoint, string exportRelativePath)
+        internal static void AppendExportPath(this OtlpExporterOptions options, string exportRelativePath)
         {
             // The exportRelativePath is only appended when the options.Endpoint property wasn't set by the user,
             // the protocol is HttpProtobuf and the OTEL_EXPORTER_OTLP_ENDPOINT environment variable
             // is present. If the user provides a custom value for options.Endpoint that value is taken as is.
-            if (ReferenceEquals(initialEndpoint, options.Endpoint))
+            if (!options.ProgrammaticallyModifiedEndpoint)
             {
                 if (options.Protocol == OtlpExportProtocol.HttpProtobuf)
                 {

diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpMetricExporterExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpMetricExporterExtensions.cs
@@ -52,13 +52,11 @@ private static MeterProviderBuilder AddOtlpExporter(
             Action<OtlpExporterOptions> configure,
             IServiceProvider serviceProvider)
         {
-            var initialEndpoint = options.Endpoint;
-
             configure?.Invoke(options);
 
             options.TryEnableIHttpClientFactoryIntegration(serviceProvider, "OtlpMetricExporter");
 
-            options.AppendExportPath(initialEndpoint, OtlpExporterOptions.MetricsExportPath);
+            options.AppendExportPath(OtlpExporterOptions.MetricsExportPath);
 
             var metricExporter = new OtlpMetricExporter(options);
 

diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpTraceExporterHelperExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpTraceExporterHelperExtensions.cs
@@ -52,13 +52,11 @@ private static TracerProviderBuilder AddOtlpExporter(
             Action<OtlpExporterOptions> configure,
             IServiceProvider serviceProvider)
         {
-            var originalEndpoint = exporterOptions.Endpoint;
-
             configure?.Invoke(exporterOptions);
 
             exporterOptions.TryEnableIHttpClientFactoryIntegration(serviceProvider, "OtlpTraceExporter");
 
-            exporterOptions.AppendExportPath(originalEndpoint, OtlpExporterOptions.TracesExportPath);
+            exporterOptions.AppendExportPath(OtlpExporterOptions.TracesExportPath);
 
             var otlpExporter = new OtlpTraceExporter(exporterOptions);
 

diff --git a/.../OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterOptionsExtensionsTests.cs b/.../OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterOptionsExtensionsTests.cs
@@ -173,9 +173,9 @@ public void AppendExportPath_EndpointNotSet_EnvironmentVariableNotDefined_NotApp
 
             var options = new OtlpExporterOptions { Protocol = OtlpExportProtocol.HttpProtobuf };
 
-            options.AppendExportPath(options.Endpoint, "test/path");
+            options.AppendExportPath("test/path");
 
-            Assert.Equal("http://localhost:4317/", options.Endpoint.AbsoluteUri);
+            Assert.Equal("http://localhost:4318/", options.Endpoint.AbsoluteUri);
         }
 
         [Fact]
@@ -185,7 +185,7 @@ public void AppendExportPath_EndpointNotSet_EnvironmentVariableDefined_Appended(
 
             var options = new OtlpExporterOptions { Protocol = OtlpExportProtocol.HttpProtobuf };
 
-            options.AppendExportPath(options.Endpoint, "test/path");
+            options.AppendExportPath("test/path");
 
             Assert.Equal("http://test:8888/test/path", options.Endpoint.AbsoluteUri);
 
@@ -198,10 +198,9 @@ public void AppendExportPath_EndpointSetEqualToEnvironmentVariable_EnvironmentVa
             Environment.SetEnvironmentVariable(OtlpExporterOptions.EndpointEnvVarName, "http://test:8888");
 
             var options = new OtlpExporterOptions { Protocol = OtlpExportProtocol.HttpProtobuf };
-            var originalEndpoint = options.Endpoint;
             options.Endpoint = new Uri("http://test:8888");
 
-            options.AppendExportPath(originalEndpoint, "test/path");
+            options.AppendExportPath("test/path");
 
             Assert.Equal("http://test:8888/", options.Endpoint.AbsoluteUri);
 
@@ -219,7 +218,7 @@ public void AppendExportPath_EndpointSet_EnvironmentVariableNotDefined_NotAppend
             var originalEndpoint = options.Endpoint;
             options.Endpoint = new Uri(endpoint);
 
-            options.AppendExportPath(originalEndpoint, "test/path");
+            options.AppendExportPath("test/path");
 
             Assert.Equal(endpoint, options.Endpoint.AbsoluteUri);
         }

diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterOptionsTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterOptionsTests.cs
@@ -42,6 +42,19 @@ public void OtlpExporterOptions_Defaults()
             Assert.Equal(OtlpExportProtocol.Grpc, options.Protocol);
         }
 
+        [Fact]
+        public void OtlpExporterOptions_DefaultsForHttpProtobuf()
+        {
+            var options = new OtlpExporterOptions
+            {
+                Protocol = OtlpExportProtocol.HttpProtobuf,
+            };
+            Assert.Equal(new Uri("http://localhost:4318"), options.Endpoint);
+            Assert.Null(options.Headers);
+            Assert.Equal(10000, options.TimeoutMilliseconds);
+            Assert.Equal(OtlpExportProtocol.HttpProtobuf, options.Protocol);
+        }
+
         [Fact]
         public void OtlpExporterOptions_EnvironmentVariableOverride()
         {
@@ -104,6 +117,26 @@ public void OtlpExporterOptions_SetterOverridesEnvironmentVariable()
             Assert.Equal(OtlpExportProtocol.HttpProtobuf, options.Protocol);
         }
 
+        [Fact]
+        public void OtlpExporterOptions_ProtocolSetterDoesNotOverrideCustomEndpointFromEnvVariables()
+        {
+            Environment.SetEnvironmentVariable(OtlpExporterOptions.EndpointEnvVarName, "http://test:8888");
+
+            var options = new OtlpExporterOptions { Protocol = OtlpExportProtocol.Grpc };
+
+            Assert.Equal(new Uri("http://test:8888"), options.Endpoint);
+            Assert.Equal(OtlpExportProtocol.Grpc, options.Protocol);
+        }
+
+        [Fact]
+        public void OtlpExporterOptions_ProtocolSetterDoesNotOverrideCustomEndpointFromSetter()
+        {
+            var options = new OtlpExporterOptions { Endpoint = new Uri("http://test:8888"), Protocol = OtlpExportProtocol.Grpc };
+
+            Assert.Equal(new Uri("http://test:8888"), options.Endpoint);
+            Assert.Equal(OtlpExportProtocol.Grpc, options.Protocol);
+        }
+
         [Fact]
         public void OtlpExporterOptions_EnvironmentVariableNames()
         {