diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/BaseOtlpHttpExportClient.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/BaseOtlpHttpExportClient.cs index 3fc2ec702fa..787e034dc6e 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/BaseOtlpHttpExportClient.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/BaseOtlpHttpExportClient.cs @@ -16,6 +16,7 @@ using System; using System.Collections.Generic; +using System.Net.Http; using System.Threading; namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClient @@ -24,7 +25,7 @@ namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClie /// Type of export request. internal abstract class BaseOtlpHttpExportClient : IExportClient { - protected BaseOtlpHttpExportClient(OtlpExporterOptions options, IHttpHandler httpHandler = null) + protected BaseOtlpHttpExportClient(OtlpExporterOptions options, HttpClient httpClient = null) { this.Options = options ?? throw new ArgumentNullException(nameof(options)); @@ -35,24 +36,42 @@ protected BaseOtlpHttpExportClient(OtlpExporterOptions options, IHttpHandler htt this.Headers = options.GetHeaders>((d, k, v) => d.Add(k, v)); - this.HttpHandler = httpHandler ?? new HttpHandler(TimeSpan.FromMilliseconds(this.Options.TimeoutMilliseconds)); + this.HttpClient = httpClient ?? new HttpClient { Timeout = TimeSpan.FromMilliseconds(this.Options.TimeoutMilliseconds) }; } internal OtlpExporterOptions Options { get; } - internal IHttpHandler HttpHandler { get; } + internal HttpClient HttpClient { get; } internal IReadOnlyDictionary Headers { get; } /// - public abstract bool SendExportRequest(TRequest request, CancellationToken cancellationToken = default); + public bool SendExportRequest(TRequest request, CancellationToken cancellationToken = default) + { + try + { + using var httpRequest = this.CreateHttpRequest(request); + + using var httpResponse = this.SendHttpRequest(httpRequest, cancellationToken); + + httpResponse?.EnsureSuccessStatusCode(); + } + catch (HttpRequestException ex) + { + OpenTelemetryProtocolExporterEventSource.Log.FailedToReachCollector(ex); + + return false; + } + + return true; + } /// - public virtual bool CancelExportRequest(int timeoutMilliseconds) + public bool CancelExportRequest(int timeoutMilliseconds) { try { - this.HttpHandler.CancelPendingRequests(); + this.HttpClient.CancelPendingRequests(); return true; } catch (Exception ex) @@ -61,5 +80,16 @@ public virtual bool CancelExportRequest(int timeoutMilliseconds) return false; } } + + protected abstract HttpRequestMessage CreateHttpRequest(TRequest request); + + protected HttpResponseMessage SendHttpRequest(HttpRequestMessage request, CancellationToken cancellationToken) + { +#if NET5_0_OR_GREATER + return this.HttpClient.Send(request, cancellationToken); +#else + return this.HttpClient.SendAsync(request, cancellationToken).GetAwaiter().GetResult(); +#endif + } } } diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/OtlpHttpTraceExportClient.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/OtlpHttpTraceExportClient.cs index 8ea26f24835..9bb7e04fb27 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/OtlpHttpTraceExportClient.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/OtlpHttpTraceExportClient.cs @@ -18,7 +18,6 @@ using System.IO; using System.Net.Http; using System.Net.Http.Headers; -using System.Threading; using Google.Protobuf; using OtlpCollector = Opentelemetry.Proto.Collector.Trace.V1; @@ -30,34 +29,13 @@ internal sealed class OtlpHttpTraceExportClient : BaseOtlpHttpExportClient - public override bool SendExportRequest(OtlpCollector.ExportTraceServiceRequest request, CancellationToken cancellationToken = default) - { - try - { - using var httpRequest = this.CreateHttpRequest(request); - - using var httpResponse = this.HttpHandler.Send(httpRequest); - - httpResponse?.EnsureSuccessStatusCode(); - } - catch (HttpRequestException ex) - { - OpenTelemetryProtocolExporterEventSource.Log.FailedToReachCollector(ex); - - return false; - } - - return true; - } - - private HttpRequestMessage CreateHttpRequest(OtlpCollector.ExportTraceServiceRequest exportRequest) + protected override HttpRequestMessage CreateHttpRequest(OtlpCollector.ExportTraceServiceRequest exportRequest) { var request = new HttpRequestMessage(HttpMethod.Post, this.exportTracesUri); foreach (var header in this.Headers) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/HttpHandler.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/HttpHandler.cs deleted file mode 100644 index 7cfc1f937b2..00000000000 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/HttpHandler.cs +++ /dev/null @@ -1,52 +0,0 @@ -// -// 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. -// - -using System; -using System.Net.Http; -using System.Threading; - -namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation -{ - /// - /// Class decorating . - /// - internal class HttpHandler : IHttpHandler - { - internal readonly HttpClient HttpClient; - - public HttpHandler(TimeSpan timeout) - { - this.HttpClient = new HttpClient - { - Timeout = timeout, - }; - } - - public void CancelPendingRequests() - { - this.HttpClient.CancelPendingRequests(); - } - - public HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken) - { -#if NET5_0_OR_GREATER - return this.HttpClient.Send(request, cancellationToken); -#else - return this.HttpClient.SendAsync(request, cancellationToken).GetAwaiter().GetResult(); -#endif - } - } -} diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/IHttpHandler.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/IHttpHandler.cs deleted file mode 100644 index f2b8bd5df9b..00000000000 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/IHttpHandler.cs +++ /dev/null @@ -1,40 +0,0 @@ -// -// 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. -// - -using System.Net.Http; -using System.Threading; - -namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation -{ - /// - /// Interface partially exposing methods. - /// - internal interface IHttpHandler - { - /// - /// Cancel all pending requests on this instance. - /// - void CancelPendingRequests(); - - /// - /// Send an HTTP request as an asynchronous operation. - /// - /// The HTTP request message to send. - /// The cancellation token to cancel operation. - /// Result of the export operation. - HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken = default); - } -} diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/Implementation/ExportClient/OtlpHttpTraceExportClientTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/Implementation/ExportClient/OtlpHttpTraceExportClientTests.cs index 36930724c22..e3e6326d49e 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/Implementation/ExportClient/OtlpHttpTraceExportClientTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/Implementation/ExportClient/OtlpHttpTraceExportClientTests.cs @@ -22,6 +22,7 @@ using System.Threading; using System.Threading.Tasks; using Moq; +using Moq.Protected; using OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation; using OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClient; using OpenTelemetry.Resources; @@ -47,15 +48,6 @@ static OtlpHttpTraceExportClientTests() ActivitySource.AddActivityListener(listener); } - [Fact] - public void NewOtlpHttpTraceExportClient_DefaultOtlpExporterOptions_ExportClientHasDefaulProperties() - { - var client = new OtlpHttpTraceExportClient(new OtlpExporterOptions()); - - Assert.NotNull(client.HttpHandler); - Assert.IsType(client.HttpHandler); - } - [Fact] public void NewOtlpHttpTraceExportClient_OtlpExporterOptions_ExporterHasCorrectProperties() { @@ -67,10 +59,9 @@ public void NewOtlpHttpTraceExportClient_OtlpExporterOptions_ExporterHasCorrectP Headers = $"{header1.Name}={header1.Value}, {header2.Name} = {header2.Value}", }; - var client = new OtlpHttpTraceExportClient(options, new NoopHttpHandler()); + var client = new OtlpHttpTraceExportClient(options); - Assert.NotNull(client.HttpHandler); - Assert.IsType(client.HttpHandler); + Assert.NotNull(client.HttpClient); Assert.Equal(2, client.Headers.Count); Assert.Contains(client.Headers, kvp => kvp.Key == header1.Name && kvp.Value == header1.Value); @@ -98,21 +89,42 @@ public void SendExportRequest_ExportTraceServiceRequest_SendsCorrectHttpRequest( Headers = $"{header1.Name}={header1.Value}, {header2.Name} = {header2.Value}", }; - var httpHandlerMock = new Mock(); + var httpHandlerMock = new Mock(); HttpRequestMessage httpRequest = null; var httpRequestContent = Array.Empty(); - httpHandlerMock.Setup(h => h.Send(It.IsAny(), It.IsAny())) + httpHandlerMock.Protected() +#if NET5_0_OR_GREATER + .Setup("Send", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns((HttpRequestMessage request, CancellationToken token) => + { + return new HttpResponseMessage(); + }) + .Callback((r, ct) => + { + httpRequest = r; + + // We have to capture content as it can't be accessed after request is disposed inside of SendExportRequest method + httpRequestContent = r.Content.ReadAsByteArrayAsync()?.Result; + }) +#else + .Setup>("SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .ReturnsAsync((HttpRequestMessage request, CancellationToken token) => + { + return new HttpResponseMessage(); + }) .Callback(async (r, ct) => { httpRequest = r; // We have to capture content as it can't be accessed after request is disposed inside of SendExportRequest method httpRequestContent = await r.Content.ReadAsByteArrayAsync(); - }); + }) +#endif + .Verifiable(); - var exportClient = new OtlpHttpTraceExportClient(options, httpHandlerMock.Object); + var exportClient = new OtlpHttpTraceExportClient(options, new HttpClient(httpHandlerMock.Object)); var exporter = new OtlpTraceExporter(options, exportClient); @@ -141,8 +153,6 @@ public void SendExportRequest_ExportTraceServiceRequest_SendsCorrectHttpRequest( var result = exportClient.SendExportRequest(request); // Assert - httpHandlerMock.Verify(m => m.Send(It.IsAny(), It.IsAny()), Times.Once()); - Assert.True(result); Assert.NotNull(httpRequest); Assert.Equal(HttpMethod.Post, httpRequest.Method); @@ -173,29 +183,5 @@ public void SendExportRequest_ExportTraceServiceRequest_SendsCorrectHttpRequest( Assert.Single(resourceSpan.InstrumentationLibrarySpans); } - - [Fact] - public void CancelExportRequest_PendingHttpRequestsCancelled() - { - var httpHandlerMock = new Mock(); - - var client = new OtlpHttpTraceExportClient(new OtlpExporterOptions(), httpHandlerMock.Object); - - var result = client.CancelExportRequest(10); - - httpHandlerMock.Verify(m => m.CancelPendingRequests(), Times.Once()); - } - - private class NoopHttpHandler : IHttpHandler - { - public void CancelPendingRequests() - { - } - - public HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken = default) - { - return null; - } - } } }