Fix defaults for http/protobuf via base export client

src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/BaseOtlpHttpExportClient.cs

@@ -26,13 +26,16 @@ namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClie
     /// <typeparam name="TRequest">Type of export request.</typeparam>
     internal abstract class BaseOtlpHttpExportClient<TRequest> : IExportClient<TRequest>
     {
-        protected BaseOtlpHttpExportClient(OtlpExporterOptions options, HttpClient httpClient)
+        protected BaseOtlpHttpExportClient(OtlpExporterOptions options, HttpClient httpClient, string signalPath)
         {
             Guard.ThrowIfNull(options);
             Guard.ThrowIfNull(httpClient);
+            Guard.ThrowIfNull(signalPath);
             Guard.ThrowIfInvalidTimeout(options.TimeoutMilliseconds);
 
-            this.Endpoint = new UriBuilder(options.Endpoint).Uri;
+            this.Endpoint = !options.ProgrammaticallyModifiedEndpoint
+                ? options.Endpoint.AppendPathIfNotPresent(signalPath)
+                : options.Endpoint;
             this.Headers = options.GetHeaders<Dictionary<string, string>>((d, k, v) => d.Add(k, v));
             this.HttpClient = httpClient;
         }
@@ -46,17 +49,17 @@ protected BaseOtlpHttpExportClient(OtlpExporterOptions options, HttpClient httpC
         /// <inheritdoc/>
         public bool SendExportRequest(TRequest request, CancellationToken cancellationToken = default)
         {
-            using var httpRequest = this.CreateHttpRequest(request);
-
             try
             {
+                using var httpRequest = this.CreateHttpRequest(request);
+
                 using var httpResponse = this.SendHttpRequest(httpRequest, cancellationToken);
 
                 httpResponse?.EnsureSuccessStatusCode();
             }
             catch (HttpRequestException ex)
             {
-                OpenTelemetryProtocolExporterEventSource.Log.FailedToReachCollector(httpRequest.RequestUri, ex);
+                OpenTelemetryProtocolExporterEventSource.Log.FailedToReachCollector(this.Endpoint, ex);
 
                 return false;
             }
@@ -71,7 +74,20 @@ public bool Shutdown(int timeoutMilliseconds)
             return true;
         }
 
-        protected abstract HttpRequestMessage CreateHttpRequest(TRequest request);
+        protected abstract HttpContent CreateHttpContent(TRequest exportRequest);
+
+        protected HttpRequestMessage CreateHttpRequest(TRequest exportRequest)
+        {
+            var request = new HttpRequestMessage(HttpMethod.Post, this.Endpoint);
+            foreach (var header in this.Headers)
+            {
+                request.Headers.Add(header.Key, header.Value);
+            }
+
+            request.Content = this.CreateHttpContent(exportRequest);
+
+            return request;
+        }
 
         protected HttpResponseMessage SendHttpRequest(HttpRequestMessage request, CancellationToken cancellationToken)
         {

src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/OtlpHttpMetricsExportClient.cs

@@ -14,7 +14,6 @@
 // limitations under the License.
 // </copyright>
 
-using System;
 using System.IO;
 using System.Net;
 using System.Net.Http;
@@ -33,27 +32,15 @@ namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClie
     internal sealed class OtlpHttpMetricsExportClient : BaseOtlpHttpExportClient<OtlpCollector.ExportMetricsServiceRequest>
     {
         internal const string MediaContentType = "application/x-protobuf";
-        private readonly Uri exportMetricsUri;
 
         public OtlpHttpMetricsExportClient(OtlpExporterOptions options, HttpClient httpClient)
-            : base(options, httpClient)
+            : base(options, httpClient, "v1/metrics")
         {
-            this.exportMetricsUri = options.Protocol == OtlpExportProtocol.HttpProtobuf
-                ? this.Endpoint.AppendPathIfNotPresent(OtlpExporterOptions.MetricsExportPath)
-                : this.Endpoint;
         }
 
-        protected override HttpRequestMessage CreateHttpRequest(OtlpCollector.ExportMetricsServiceRequest exportRequest)
+        protected override HttpContent CreateHttpContent(OtlpCollector.ExportMetricsServiceRequest exportRequest)
         {
-            var request = new HttpRequestMessage(HttpMethod.Post, this.exportMetricsUri);
-            foreach (var header in this.Headers)
-            {
-                request.Headers.Add(header.Key, header.Value);
-            }
-
-            request.Content = new ExportRequestContent(exportRequest);
-
-            return request;
+            return new ExportRequestContent(exportRequest);
         }
 
         internal sealed class ExportRequestContent : HttpContent

src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/OtlpHttpTraceExportClient.cs

@@ -14,7 +14,6 @@
 // limitations under the License.
 // </copyright>
 
-using System;
 using System.IO;
 using System.Net;
 using System.Net.Http;
@@ -33,27 +32,15 @@ namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClie
     internal sealed class OtlpHttpTraceExportClient : BaseOtlpHttpExportClient<OtlpCollector.ExportTraceServiceRequest>
     {
         internal const string MediaContentType = "application/x-protobuf";
-        private readonly Uri exportTracesUri;
 
         public OtlpHttpTraceExportClient(OtlpExporterOptions options, HttpClient httpClient)
-            : base(options, httpClient)
+            : base(options, httpClient, "v1/traces")
         {
-            this.exportTracesUri = options.Protocol == OtlpExportProtocol.HttpProtobuf
-                ? this.Endpoint.AppendPathIfNotPresent(OtlpExporterOptions.TracesExportPath)
-                : this.Endpoint;
         }
 
-        protected override HttpRequestMessage CreateHttpRequest(OtlpCollector.ExportTraceServiceRequest exportRequest)
+        protected override HttpContent CreateHttpContent(OtlpCollector.ExportTraceServiceRequest exportRequest)
         {
-            var request = new HttpRequestMessage(HttpMethod.Post, this.exportTracesUri);
-            foreach (var header in this.Headers)
-            {
-                request.Headers.Add(header.Key, header.Value);
-            }
-
-            request.Content = new ExportRequestContent(exportRequest);
-
-            return request;
+            return new ExportRequestContent(exportRequest);
         }
 
         internal sealed class ExportRequestContent : HttpContent

src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs

@@ -39,9 +39,6 @@ public class OtlpExporterOptions
         internal const string TimeoutEnvVarName = "OTEL_EXPORTER_OTLP_TIMEOUT";
         internal const string ProtocolEnvVarName = "OTEL_EXPORTER_OTLP_PROTOCOL";
 
-        internal const string TracesExportPath = "v1/traces";
-        internal const string MetricsExportPath = "v1/metrics";
-
         internal readonly Func<HttpClient> DefaultHttpClientFactory;
 
         private const string DefaultGrpcEndpoint = "http://localhost:4317";

src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptionsExtensions.cs

@@ -19,7 +19,6 @@
 using System.Reflection;
 using Grpc.Core;
 using OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClient;
-using OpenTelemetry.Internal;
 #if NETSTANDARD2_1 || NET5_0_OR_GREATER
 using Grpc.Net.Client;
 #endif
@@ -155,25 +154,6 @@ public static void TryEnableIHttpClientFactoryIntegration(this OtlpExporterOptio
             }
         }
 
-        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 (!options.ProgrammaticallyModifiedEndpoint)
-            {
-                if (options.Protocol == OtlpExportProtocol.HttpProtobuf)
-                {
-                    if (EnvironmentVariableHelper.LoadUri(OtlpExporterOptions.EndpointEnvVarName, out Uri endpoint))
-                    {
-                        // At this point we can conclude that endpoint was initialized from OTEL_EXPORTER_OTLP_ENDPOINT
-                        // and has to be appended by export relative path (traces/metrics).
-                        options.Endpoint = endpoint.AppendPathIfNotPresent(exportRelativePath);
-                    }
-                }
-            }
-        }
-
         internal static Uri AppendPathIfNotPresent(this Uri uri, string path)
         {
             var absoluteUri = uri.AbsoluteUri;

src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpMetricExporterExtensions.cs

@@ -102,8 +102,6 @@ internal static MeterProviderBuilder AddOtlpExporter(
 
             exporterOptions.TryEnableIHttpClientFactoryIntegration(serviceProvider, "OtlpMetricExporter");
 
-            exporterOptions.AppendExportPath(OtlpExporterOptions.MetricsExportPath);
-
             BaseExporter<Metric> metricExporter = new OtlpMetricExporter(exporterOptions);
 
             if (configureExporterInstance != null)

src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpTraceExporterHelperExtensions.cs

@@ -58,8 +58,6 @@ internal static TracerProviderBuilder AddOtlpExporter(
 
             exporterOptions.TryEnableIHttpClientFactoryIntegration(serviceProvider, "OtlpTraceExporter");
 
-            exporterOptions.AppendExportPath(OtlpExporterOptions.TracesExportPath);
-
             BaseExporter<Activity> otlpExporter = new OtlpTraceExporter(exporterOptions);
 
             if (configureExporterInstance != null)

test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/BaseOtlpHttpExportClientTests.cs

@@ -0,0 +1,67 @@
+// <copyright file="BaseOtlpHttpExportClientTests.cs" company="OpenTelemetry Authors">
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+// </copyright>
+
+using System;
+using System.Net.Http;
+using OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClient;
+using Xunit;
+
+namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests
+{
+    public class BaseOtlpHttpExportClientTests
+    {
+        [Theory]
+        [InlineData(null, null, "http://localhost:4318/signal/path")]
+        [InlineData(null, "http://from.otel.exporter.env.var", "http://from.otel.exporter.env.var/signal/path")]
+        [InlineData("https://custom.host", null, "https://custom.host")]
+        [InlineData("http://custom.host:44318/custom/path", null, "http://custom.host:44318/custom/path")]
+        [InlineData("https://custom.host", "http://from.otel.exporter.env.var", "https://custom.host")]
+        public void ValidateOtlpHttpExportClientEndpoint(string optionEndpoint, string endpointEnvVar, string expectedExporterEndpoint)
+        {
+            try
+            {
+                Environment.SetEnvironmentVariable(OtlpExporterOptions.EndpointEnvVarName, endpointEnvVar);
+
+                OtlpExporterOptions options = new() { Protocol = OtlpExportProtocol.HttpProtobuf };
+
+                if (optionEndpoint != null)
+                {
+                    options.Endpoint = new Uri(optionEndpoint);
+                }
+
+                var exporterClient = new TestOtlpHttpExportClient(options, new HttpClient());
+                Assert.Equal(new Uri(expectedExporterEndpoint), exporterClient.Endpoint);
+            }
+            finally
+            {
+                Environment.SetEnvironmentVariable(OtlpExporterOptions.EndpointEnvVarName, null);
+            }
+        }
+
+        internal class TestOtlpHttpExportClient : BaseOtlpHttpExportClient<string>
+        {
+            public TestOtlpHttpExportClient(OtlpExporterOptions options, HttpClient httpClient)
+                : base(options, httpClient, "signal/path")
+            {
+            }
+
+            protected override HttpContent CreateHttpContent(string exportRequest)
+            {
+                throw new NotImplementedException();
+            }
+        }
+    }
+}

test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterOptionsExtensionsTests.cs

@@ -165,71 +165,10 @@ public void AppendPathIfNotPresent_TracesPath_AppendsCorrectly(string inputUri,
         {
             var uri = new Uri(inputUri, UriKind.Absolute);
 
-            var resultUri = uri.AppendPathIfNotPresent(OtlpExporterOptions.TracesExportPath);
+            var resultUri = uri.AppendPathIfNotPresent("v1/traces");
 
             Assert.Equal(expectedUri, resultUri.AbsoluteUri);
         }
 
-        [Fact]
-        public void AppendExportPath_EndpointNotSet_EnvironmentVariableNotDefined_NotAppended()
-        {
-            ClearEndpointEnvVar();
-
-            var options = new OtlpExporterOptions { Protocol = OtlpExportProtocol.HttpProtobuf };
-
-            options.AppendExportPath("test/path");
-
-            Assert.Equal("http://localhost:4318/", options.Endpoint.AbsoluteUri);
-        }
-
-        [Fact]
-        public void AppendExportPath_EndpointNotSet_EnvironmentVariableDefined_Appended()
-        {
-            Environment.SetEnvironmentVariable(OtlpExporterOptions.EndpointEnvVarName, "http://test:8888");
-
-            var options = new OtlpExporterOptions { Protocol = OtlpExportProtocol.HttpProtobuf };
-
-            options.AppendExportPath("test/path");
-
-            Assert.Equal("http://test:8888/test/path", options.Endpoint.AbsoluteUri);
-
-            ClearEndpointEnvVar();
-        }
-
-        [Fact]
-        public void AppendExportPath_EndpointSetEqualToEnvironmentVariable_EnvironmentVariableDefined_NotAppended()
-        {
-            Environment.SetEnvironmentVariable(OtlpExporterOptions.EndpointEnvVarName, "http://test:8888");
-
-            var options = new OtlpExporterOptions { Protocol = OtlpExportProtocol.HttpProtobuf };
-            options.Endpoint = new Uri("http://test:8888");
-
-            options.AppendExportPath("test/path");
-
-            Assert.Equal("http://test:8888/", options.Endpoint.AbsoluteUri);
-
-            ClearEndpointEnvVar();
-        }
-
-        [Theory]
-        [InlineData("http://localhost:4317/")]
-        [InlineData("http://test:8888/")]
-        public void AppendExportPath_EndpointSet_EnvironmentVariableNotDefined_NotAppended(string endpoint)
-        {
-            ClearEndpointEnvVar();
-
-            var options = new OtlpExporterOptions { Protocol = OtlpExportProtocol.HttpProtobuf };
-            var originalEndpoint = options.Endpoint;
-            options.Endpoint = new Uri(endpoint);
-
-            options.AppendExportPath("test/path");
-
-            Assert.Equal(endpoint, options.Endpoint.AbsoluteUri);
-        }
-
-        private static void ClearEndpointEnvVar()
-        {
-            Environment.SetEnvironmentVariable(OtlpExporterOptions.EndpointEnvVarName, null);
-        }
     }
 }