default endpoint for otel exporter - http/protobuf

fixes #2857
open-telemetry · Feb 7, 2022 · 5705723 · 5705723
1 parent 563236b
commit 5705723
Show file tree

Hide file tree

Showing 7 changed files with 117 additions and 27 deletions.
diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md
@@ -6,6 +6,9 @@
 
 ## Unreleased
 
+* Fixed default endpoint for OTLP exporter over HTTP/Protobuf.
+  ([#2686](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2868))
+
 ## 1.2.0-rc2
 
 Released 2022-Feb-02

diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs
@@ -42,18 +42,20 @@ public class OtlpExporterOptions
         internal const string TracesExportPath = "v1/traces";
         internal const string MetricsExportPath = "v1/metrics";
 
+        private const string DefaultGrpcEndpoint = "http://localhost:4317";
+        private const string DefaultHttpProtobufEndpoint = "http://localhost:4318";
+        private const OtlpExportProtocol DefaultOtlpExportProtocol = OtlpExportProtocol.Grpc;
+
         internal readonly Func<HttpClient> DefaultHttpClientFactory;
+        private OtlpExportProtocol protocol;
+        private Uri endpoint;
+        private bool isCustomEndpointSet;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="OtlpExporterOptions"/> class.
         /// </summary>
         public OtlpExporterOptions()
         {
-            if (EnvironmentVariableHelper.LoadUri(EndpointEnvVarName, out Uri endpoint))
-            {
-                this.Endpoint = endpoint;
-            }
-
             if (EnvironmentVariableHelper.LoadString(HeadersEnvVarName, out string headersEnvVar))
             {
                 this.Headers = headersEnvVar;
@@ -66,20 +68,29 @@ public OtlpExporterOptions()
 
             if (EnvironmentVariableHelper.LoadString(ProtocolEnvVarName, out string protocolEnvVar))
             {
-                var protocol = protocolEnvVar.ToOtlpExportProtocol();
-                if (protocol.HasValue)
+                var parsedProtocol = protocolEnvVar.ToOtlpExportProtocol();
+                if (parsedProtocol.HasValue)
                 {
-                    this.Protocol = protocol.Value;
+                    this.Protocol = parsedProtocol.Value;
                 }
                 else
                 {
                     throw new FormatException($"{ProtocolEnvVarName} environment variable has an invalid value: '${protocolEnvVar}'");
                 }
             }
+            else
+            {
+                this.Protocol = DefaultOtlpExportProtocol;
+            }
+
+            if (EnvironmentVariableHelper.LoadUri(EndpointEnvVarName, out Uri parsedEndpoint))
+            {
+                this.SetEndpoint(parsedEndpoint, custom: true);
+            }
 
             this.HttpClientFactory = this.DefaultHttpClientFactory = () =>
             {
-                return new HttpClient()
+                return new HttpClient
                 {
                     Timeout = TimeSpan.FromMilliseconds(this.TimeoutMilliseconds),
                 };
@@ -89,9 +100,19 @@ 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 => this.endpoint;
+            set
+            {
+                this.SetEndpoint(value, custom: true);
+                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,10 +128,27 @@ 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 => this.protocol;
+            set
+            {
+                this.protocol = value;
+
+                switch (value)
+                {
+                    case OtlpExportProtocol.Grpc:
+                        this.SetEndpoint(new Uri(DefaultGrpcEndpoint));
+                        break;
+                    case OtlpExportProtocol.HttpProtobuf:
+                        this.SetEndpoint(new Uri(DefaultHttpProtobufEndpoint));
+                        break;
+                }
+            }
+        }
 
         /// <summary>
-        /// Gets or sets the export processor type to be used with the OpenTelemetry Protocol Exporter. The default value is <see cref="ExportProcessorType.Batch"/>.
+        /// Gets or sets the export processor type to be used with the OpenTelemetry Protocol Exporter. The default value is <see cref="Batch{T}"/>.
         /// </summary>
         public ExportProcessorType ExportProcessorType { get; set; } = ExportProcessorType.Batch;
 
@@ -167,5 +205,26 @@ public OtlpExporterOptions()
         /// </list>
         /// </remarks>
         public Func<HttpClient> HttpClientFactory { get; set; }
+
+        /// <summary>
+        /// Gets a value indicating whether <see cref="Endpoint" /> was programmatically modified.
+        /// </summary>
+        internal bool ProgrammaticallyModifiedEndpoint { get; private set; }
+
+        private void SetEndpoint(Uri value, bool custom = false)
+        {
+            if (custom)
+            {
+                this.isCustomEndpointSet = true;
+                this.endpoint = value;
+            }
+            else
+            {
+                if (!this.isCustomEndpointSet)
+                {
+                    this.endpoint = value;
+                }
+            }
+        }
     }
 }
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()
         {