From 5cc9d5b55e055539bf33612547bcee86af7a3e7a Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 11 Mar 2024 12:14:21 -0700 Subject: [PATCH 1/8] Nullable annotations for the OtlpExporterOptions class and some xml doc improvements. --- .../.publicApi/Stable/PublicAPI.Shipped.txt | 16 ++-- .../OtlpExporterOptions.cs | 82 ++++++++++++------- 2 files changed, 62 insertions(+), 36 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Shipped.txt b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Shipped.txt index 4b7af046438..30b70382df5 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Shipped.txt +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Shipped.txt @@ -1,12 +1,12 @@ #nullable enable -~OpenTelemetry.Exporter.OtlpExporterOptions.BatchExportProcessorOptions.get -> OpenTelemetry.BatchExportProcessorOptions -~OpenTelemetry.Exporter.OtlpExporterOptions.BatchExportProcessorOptions.set -> void -~OpenTelemetry.Exporter.OtlpExporterOptions.Endpoint.get -> System.Uri -~OpenTelemetry.Exporter.OtlpExporterOptions.Endpoint.set -> void -~OpenTelemetry.Exporter.OtlpExporterOptions.Headers.get -> string -~OpenTelemetry.Exporter.OtlpExporterOptions.Headers.set -> void -~OpenTelemetry.Exporter.OtlpExporterOptions.HttpClientFactory.get -> System.Func -~OpenTelemetry.Exporter.OtlpExporterOptions.HttpClientFactory.set -> void +OpenTelemetry.Exporter.OtlpExporterOptions.BatchExportProcessorOptions.get -> OpenTelemetry.BatchExportProcessorOptions! +OpenTelemetry.Exporter.OtlpExporterOptions.BatchExportProcessorOptions.set -> void +OpenTelemetry.Exporter.OtlpExporterOptions.Endpoint.get -> System.Uri! +OpenTelemetry.Exporter.OtlpExporterOptions.Endpoint.set -> void +OpenTelemetry.Exporter.OtlpExporterOptions.Headers.get -> string? +OpenTelemetry.Exporter.OtlpExporterOptions.Headers.set -> void +OpenTelemetry.Exporter.OtlpExporterOptions.HttpClientFactory.get -> System.Func! +OpenTelemetry.Exporter.OtlpExporterOptions.HttpClientFactory.set -> void ~OpenTelemetry.Exporter.OtlpMetricExporter.OtlpMetricExporter(OpenTelemetry.Exporter.OtlpExporterOptions options) -> void ~OpenTelemetry.Exporter.OtlpTraceExporter.OtlpTraceExporter(OpenTelemetry.Exporter.OtlpExporterOptions options) -> void ~override OpenTelemetry.Exporter.OtlpMetricExporter.Export(in OpenTelemetry.Batch metrics) -> OpenTelemetry.ExportResult diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs index 0cffd50806c..94ba800f2c6 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs @@ -1,6 +1,8 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#nullable enable + using System.Diagnostics; using System.Reflection; #if NETFRAMEWORK @@ -10,7 +12,6 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using OpenTelemetry.Internal; -using OpenTelemetry.Metrics; using OpenTelemetry.Trace; namespace OpenTelemetry.Exporter; @@ -38,7 +39,8 @@ public class OtlpExporterOptions private const string UserAgentProduct = "OTel-OTLP-Exporter-Dotnet"; - private Uri endpoint; + private Uri? endpoint; + private Func? httpClientFactory; /// /// Initializes a new instance of the class. @@ -74,16 +76,28 @@ internal OtlpExporterOptions( }; }; - this.BatchExportProcessorOptions = defaultBatchOptions; + this.BatchExportProcessorOptions = defaultBatchOptions!; } /// - /// 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 for - /// * http://localhost:4318 for . + /// Gets or sets the target to which the exporter is going to send + /// telemetry. /// + /// + /// Notes: + /// + /// must be a valid with + /// scheme (http or https) and host, and may contain a port and path. + /// The default value is based on the property: + /// + /// http://localhost:4317 for . + /// http://localhost:4318 for . + /// + /// + /// + /// public Uri Endpoint { get @@ -101,23 +115,29 @@ public Uri Endpoint set { this.endpoint = value; - this.AppendSignalPathToEndpoint = false; + this.AppendSignalPathToEndpoint = value == null; } } /// - /// Gets or sets optional headers for the connection. Refer to the - /// specification for information on the expected format for Headers. + /// Gets or sets optional headers for the connection. /// - public string Headers { get; set; } + /// + /// Note: Refer to the + /// OpenTelemetry Specification for details on the format of . + /// + public string? Headers { get; set; } /// - /// Gets or sets the max waiting time (in milliseconds) for the backend to process each batch. The default value is 10000. + /// Gets or sets the max waiting time (in milliseconds) for the backend to + /// process each batch. Default value: 10000. /// public int TimeoutMilliseconds { get; set; } = 10000; /// - /// Gets or sets the the OTLP transport protocol. Supported values: Grpc and HttpProtobuf. + /// Gets or sets the the OTLP transport protocol. /// public OtlpExportProtocol Protocol { get; set; } = DefaultOtlpExportProtocol; @@ -144,27 +164,33 @@ public Uri Endpoint /// /// This is only invoked for the protocol. - /// The default behavior when using the extension is if an The default behavior when using tracing registration extensions is + /// if an IHttpClientFactory /// instance can be resolved through the application then an will be - /// created through the factory with the name "OtlpTraceExporter" - /// otherwise an will be instantiated - /// directly. - /// The default behavior when using the extension is if an will be instantiated directly. + /// The default behavior when using metrics registration extensions is + /// if an IHttpClientFactory /// instance can be resolved through the application then an will be - /// created through the factory with the name "OtlpMetricExporter" - /// otherwise an will be instantiated - /// directly. + /// created through the factory with the name "OtlpMetricExporter" otherwise + /// an will be instantiated directly. + /// + /// The default behavior when using logging registration extensions is an + /// will be instantiated directly. IHttpClientFactory + /// is not currently supported for logging. + /// /// /// - public Func HttpClientFactory { get; set; } + public Func HttpClientFactory + { + get => this.httpClientFactory ?? this.DefaultHttpClientFactory; + set => this.httpClientFactory = value; + } /// /// Gets a value indicating whether or not the signal-specific path should @@ -228,7 +254,7 @@ private static string GetUserAgentString() try { var assemblyVersion = typeof(OtlpExporterOptions).Assembly.GetCustomAttribute(); - var informationalVersion = assemblyVersion.InformationalVersion; + var informationalVersion = assemblyVersion?.InformationalVersion; return string.IsNullOrEmpty(informationalVersion) ? UserAgentProduct : $"{UserAgentProduct}/{informationalVersion}"; } catch (Exception) From 745d6272b74992c8668d2bb0d358427f66476585 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 11 Mar 2024 12:17:32 -0700 Subject: [PATCH 2/8] Tweak. --- .../OtlpExporterOptions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs index 94ba800f2c6..57dfd05b11e 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs @@ -68,7 +68,7 @@ internal OtlpExporterOptions( this.ApplyConfiguration(configuration, configurationType); - this.HttpClientFactory = this.DefaultHttpClientFactory = () => + this.DefaultHttpClientFactory = () => { return new HttpClient { From 9f05da27b6a392509fed4da0c7f08ec4061b20e0 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 11 Mar 2024 12:34:28 -0700 Subject: [PATCH 3/8] Lint. --- .../OtlpLogExporterHelperExtensions.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs index 85298517f95..1f99374c324 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs @@ -60,7 +60,7 @@ public static OpenTelemetryLoggerOptions AddOtlpExporter( return loggerOptions.AddProcessor(sp => { - var exporterOptions = GetOptions(sp, name, finalOptionsName, OtlpExporterOptions.CreateOtlpExporterOptions); + var exporterOptions = GetOptions(sp, name, finalOptionsName, OtlpExporterOptions.CreateOtlpExporterOptions); var processorOptions = sp.GetRequiredService>().Get(finalOptionsName); @@ -104,7 +104,7 @@ public static OpenTelemetryLoggerOptions AddOtlpExporter( return loggerOptions.AddProcessor(sp => { - var exporterOptions = GetOptions(sp, name, finalOptionsName, OtlpExporterOptions.CreateOtlpExporterOptions); + var exporterOptions = GetOptions(sp, name, finalOptionsName, OtlpExporterOptions.CreateOtlpExporterOptions); var processorOptions = sp.GetRequiredService>().Get(finalOptionsName); From 64fc64860d5022104d9d73f3d3a71ac4c2d36f13 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 11 Mar 2024 12:40:57 -0700 Subject: [PATCH 4/8] Test improvements. --- .../OtlpExporterOptionsTests.cs | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterOptionsTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterOptionsTests.cs index 4ba49ec760a..12b4b6535ad 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterOptionsTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterOptionsTests.cs @@ -243,6 +243,38 @@ public void OtlpExporterOptions_EnvironmentVariableNames() Assert.Equal("OTEL_EXPORTER_OTLP_PROTOCOL", OtlpSpecConfigDefinitions.DefaultProtocolEnvVarName); } + [Fact] + public void OtlpExporterOptions_SettingEndpointToNullResetsAppendSignalPathToEndpoint() + { + var options = new OtlpExporterOptions(OtlpExporterOptionsConfigurationType.Default); + + Assert.True(options.AppendSignalPathToEndpoint); + + options.Endpoint = new Uri("http://some_endpoint"); + + Assert.False(options.AppendSignalPathToEndpoint); + + options.Endpoint = null; + + Assert.True(options.AppendSignalPathToEndpoint); + } + + [Fact] + public void OtlpExporterOptions_SettingHttpClientFactoryToNullResetsToDefaultFactory() + { + var options = new OtlpExporterOptions(OtlpExporterOptionsConfigurationType.Default); + + Assert.Equal(options.DefaultHttpClientFactory, options.HttpClientFactory); + + options.HttpClientFactory = () => null!; + + Assert.NotEqual(options.DefaultHttpClientFactory, options.HttpClientFactory); + + options.HttpClientFactory = null; + + Assert.Equal(options.DefaultHttpClientFactory, options.HttpClientFactory); + } + private static void ClearEnvVars() { foreach (var item in GetOtlpExporterOptionsTestCases()) From 57096cf11a0dba22ba0fd86cc6b75e6d3c1fae67 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 11 Mar 2024 13:04:03 -0700 Subject: [PATCH 5/8] Code review. --- .../OtlpExporterOptions.cs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs index 57dfd05b11e..322c269971a 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs @@ -86,15 +86,28 @@ internal OtlpExporterOptions( /// /// Notes: /// - /// must be a valid with - /// scheme (http or https) and host, and may contain a port and path. - /// The default value is based on the property: + /// When setting the value must be a valid with scheme (http or https) and host, and may contain a + /// port and path. + /// The default value when not set is based on the property: /// /// http://localhost:4317 for . /// http://localhost:4318 for . /// + /// When is set to , if has + /// not been set, the default value http://localhost:4318 will have a + /// signal-specific path appended. The final default endpoint values will be + /// constructed as: + /// + /// Logging: http://localhost:4318/v1/logs + /// Metrics: http://localhost:4318/v1/metrics + /// Tracing: http://localhost:4318/v1/traces + /// + /// /// /// /// From c61409000468b1228dfaa17c2a35d56060feb967 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 11 Mar 2024 13:17:15 -0700 Subject: [PATCH 6/8] Tweaks. --- .../OtlpExporterOptions.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs index 322c269971a..2671a0e3a60 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs @@ -98,10 +98,10 @@ internal OtlpExporterOptions( /// cref="OtlpExportProtocol.HttpProtobuf"/>. /// /// When is set to , if has - /// not been set, the default value http://localhost:4318 will have a - /// signal-specific path appended. The final default endpoint values will be - /// constructed as: + /// cref="OtlpExportProtocol.HttpProtobuf"/> and has + /// not been set the default value (http://localhost:4318) will have + /// a signal-specific path appended. The final default endpoint values will + /// be constructed as: /// /// Logging: http://localhost:4318/v1/logs /// Metrics: http://localhost:4318/v1/metrics From 80871d3b4135d2413ee07fe7474274585a8dee15 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 11 Mar 2024 13:45:18 -0700 Subject: [PATCH 7/8] Code review. --- .../OtlpExporterOptions.cs | 11 +++++++-- .../OtlpExporterOptionsTests.cs | 24 ++++--------------- .../OtlpLogExporterTests.cs | 2 +- .../OtlpMetricsExporterTests.cs | 6 ----- .../OtlpTraceExporterTests.cs | 6 ----- 5 files changed, 14 insertions(+), 35 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs index 2671a0e3a60..0523b12d99a 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs @@ -127,8 +127,10 @@ public Uri Endpoint set { + Guard.ThrowIfNull(value); + this.endpoint = value; - this.AppendSignalPathToEndpoint = value == null; + this.AppendSignalPathToEndpoint = false; } } @@ -202,7 +204,12 @@ public Uri Endpoint public Func HttpClientFactory { get => this.httpClientFactory ?? this.DefaultHttpClientFactory; - set => this.httpClientFactory = value; + set + { + Guard.ThrowIfNull(value); + + this.httpClientFactory = value; + } } /// diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterOptionsTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterOptionsTests.cs index 12b4b6535ad..bf55abb4795 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterOptionsTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterOptionsTests.cs @@ -226,7 +226,7 @@ public void OtlpExporterOptions_EndpointGetterUsesProtocolWhenNull() } [Fact] - public void OtlpExporterOptions_EndpointGetterIgnoresProtocolWhenNotNull() + public void OtlpExporterOptions_EndpointThrowsWhenSetToNull() { var options = new OtlpExporterOptions { Endpoint = new Uri("http://test:8888"), Protocol = OtlpExportProtocol.Grpc }; @@ -248,31 +248,15 @@ public void OtlpExporterOptions_SettingEndpointToNullResetsAppendSignalPathToEnd { var options = new OtlpExporterOptions(OtlpExporterOptionsConfigurationType.Default); - Assert.True(options.AppendSignalPathToEndpoint); - - options.Endpoint = new Uri("http://some_endpoint"); - - Assert.False(options.AppendSignalPathToEndpoint); - - options.Endpoint = null; - - Assert.True(options.AppendSignalPathToEndpoint); + Assert.Throws(() => options.Endpoint = null); } [Fact] - public void OtlpExporterOptions_SettingHttpClientFactoryToNullResetsToDefaultFactory() + public void OtlpExporterOptions_HttpClientFactoryThrowsWhenSetToNull() { var options = new OtlpExporterOptions(OtlpExporterOptionsConfigurationType.Default); - Assert.Equal(options.DefaultHttpClientFactory, options.HttpClientFactory); - - options.HttpClientFactory = () => null!; - - Assert.NotEqual(options.DefaultHttpClientFactory, options.HttpClientFactory); - - options.HttpClientFactory = null; - - Assert.Equal(options.DefaultHttpClientFactory, options.HttpClientFactory); + Assert.Throws(() => options.HttpClientFactory = null); } private static void ClearEnvVars() diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs index 511e2f55876..92720ad0302 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs @@ -101,7 +101,7 @@ public void UserHttpFactoryCalledWhenUsingHttpProtobuf() Assert.Equal(2, invocations); } - options.HttpClientFactory = null; + options.HttpClientFactory = () => null; Assert.Throws(() => { using var exporter = new OtlpLogExporter(options); diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpMetricsExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpMetricsExporterTests.cs index 7322c4d90e5..451362ed927 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpMetricsExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpMetricsExporterTests.cs @@ -131,12 +131,6 @@ public void UserHttpFactoryCalled() Assert.Equal(2, invocations); } - options.HttpClientFactory = null; - Assert.Throws(() => - { - using var exporter = new OtlpMetricExporter(options); - }); - options.HttpClientFactory = () => null; Assert.Throws(() => { diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs index e3a5ad52b86..527fb5de72b 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs @@ -96,12 +96,6 @@ public void UserHttpFactoryCalled() Assert.Equal(2, invocations); } - options.HttpClientFactory = null; - Assert.Throws(() => - { - using var exporter = new OtlpTraceExporter(options); - }); - options.HttpClientFactory = () => null; Assert.Throws(() => { From 82951f87ad69292d96f2a45124e244698c431e17 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 11 Mar 2024 13:55:31 -0700 Subject: [PATCH 8/8] Update CHANGELOG. --- src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md index d5d71920bd0..e88f15d58dd 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md @@ -35,6 +35,10 @@ ExponentialHistograms. ([#5397](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5397)) +* Setting `Endpoint` or `HttpClientFactory` properties on `OtlpExporterOptions` + to `null` will now result in an `ArgumentNullException` being thrown. + ([#5434](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5434)) + ## 1.7.0 Released 2023-Dec-08