From 57057235e4b314a8f5bbc67ed039bd4a70a80ecc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= Date: Mon, 7 Feb 2022 11:52:13 +0100 Subject: [PATCH] default endpoint for otel exporter - http/protobuf fixes #2857 --- .../CHANGELOG.md | 3 + .../OtlpExporterOptions.cs | 85 ++++++++++++++++--- .../OtlpExporterOptionsExtensions.cs | 4 +- .../OtlpMetricExporterExtensions.cs | 4 +- .../OtlpTraceExporterHelperExtensions.cs | 4 +- .../OtlpExporterOptionsExtensionsTests.cs | 11 ++- .../OtlpExporterOptionsTests.cs | 33 +++++++ 7 files changed, 117 insertions(+), 27 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md index 41b14d77289..d77e9c77635 100644 --- 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 index a5725b9ba6c..93e093e1c29 100644 --- 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 DefaultHttpClientFactory; + private OtlpExportProtocol protocol; + private Uri endpoint; + private bool isCustomEndpointSet; /// /// Initializes a new instance of the class. /// 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() /// /// 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 => this.endpoint; + set + { + this.SetEndpoint(value, custom: true); + this.ProgrammaticallyModifiedEndpoint = true; + } + } /// /// Gets or sets optional headers for the connection. Refer to the @@ -107,10 +128,27 @@ 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 => 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; + } + } + } /// - /// Gets or sets the export processor type to be used with the OpenTelemetry Protocol Exporter. The default value is . + /// Gets or sets the export processor type to be used with the OpenTelemetry Protocol Exporter. The default value is . /// public ExportProcessorType ExportProcessorType { get; set; } = ExportProcessorType.Batch; @@ -167,5 +205,26 @@ public OtlpExporterOptions() /// /// public Func HttpClientFactory { get; set; } + + /// + /// Gets a value indicating whether was programmatically modified. + /// + 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 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..e4f69e34a59 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() {