From 73b99b64b043371a280dbc6739ce2be2f9429aef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= Date: Wed, 16 Feb 2022 00:13:56 +0100 Subject: [PATCH] Fix the default endpoint for OTLP exporter when using http/protobuf protocol (#2868) --- .../ApiCompatBaseline.txt | 4 ++ .../CHANGELOG.md | 6 ++- .../OtlpExporterOptions.cs | 44 ++++++++++++++++--- .../OtlpExporterOptionsExtensions.cs | 4 +- .../OtlpMetricExporterExtensions.cs | 4 +- .../OtlpTraceExporterHelperExtensions.cs | 4 +- .../OtlpExporterOptionsExtensionsTests.cs | 11 +++-- .../OtlpExporterOptionsTests.cs | 33 ++++++++++++++ 8 files changed, 89 insertions(+), 21 deletions(-) create mode 100644 src/OpenTelemetry.Exporter.OpenTelemetryProtocol/ApiCompatBaseline.txt diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/ApiCompatBaseline.txt b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/ApiCompatBaseline.txt new file mode 100644 index 00000000000..e7158253a98 --- /dev/null +++ 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 index 1a046e0b9d4..da8dedcd2ad 100644 --- 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 index a5725b9ba6c..e8b06e43914 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs @@ -44,14 +44,20 @@ public class OtlpExporterOptions internal readonly Func DefaultHttpClientFactory; + private const string DefaultGrpcEndpoint = "http://localhost:4317"; + private const string DefaultHttpEndpoint = "http://localhost:4318"; + private const OtlpExportProtocol DefaultOtlpExportProtocol = OtlpExportProtocol.Grpc; + + private Uri endpoint; + /// /// Initializes a new instance of the class. /// 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() /// /// 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 + /// * http://localhost:4318 for . /// - 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; + } + } /// /// Gets or sets optional headers for the connection. Refer to the @@ -107,7 +134,7 @@ public OtlpExporterOptions() /// /// Gets or sets the the OTLP transport protocol. Supported values: Grpc and HttpProtobuf. /// - public OtlpExportProtocol Protocol { get; set; } = OtlpExportProtocol.Grpc; + public OtlpExportProtocol Protocol { get; set; } = DefaultOtlpExportProtocol; /// /// Gets or sets the export processor type to be used with the OpenTelemetry Protocol Exporter. The default value is . @@ -167,5 +194,10 @@ public OtlpExporterOptions() /// /// public Func HttpClientFactory { get; set; } + + /// + /// Gets a value indicating whether was modified via its setter. + /// + internal bool ProgrammaticallyModifiedEndpoint { get; private set; } } } diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptionsExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptionsExtensions.cs index 3760a1830de..407e9b30b40 100644 --- 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 index fc0956d0ad5..12f2333b8ad 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpMetricExporterExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpMetricExporterExtensions.cs @@ -52,13 +52,11 @@ private static MeterProviderBuilder AddOtlpExporter( Action 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 index d9e66316919..7c4cc479f36 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpTraceExporterHelperExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpTraceExporterHelperExtensions.cs @@ -52,13 +52,11 @@ private static TracerProviderBuilder AddOtlpExporter( Action 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/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterOptionsExtensionsTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterOptionsExtensionsTests.cs index 970f96daaea..3a9b32b179e 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterOptionsExtensionsTests.cs +++ b/test/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 index 2b7a612fa06..f92b7996885 100644 --- 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() {