Skip to content

Commit

Permalink
open-telemetry#2292: got rod of HttpClient wrapper and adapted unit t…
Browse files Browse the repository at this point in the history 
…ests
  • Loading branch information
@rypdal
rypdal committed Sep 27, 2021
1 parent 1285cca commit e850df0
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 165 deletions.
42 changes: 36 additions & 6 deletions ...ry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/BaseOtlpHttpExportClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

using System;
using System.Collections.Generic;
using System.Net.Http;
using System.Threading;

namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClient
Expand All @@ -24,7 +25,7 @@ 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, IHttpHandler httpHandler = null)
protected BaseOtlpHttpExportClient(OtlpExporterOptions options, HttpClient httpClient = null)
{
this.Options = options ?? throw new ArgumentNullException(nameof(options));

Expand All @@ -35,24 +36,42 @@ protected BaseOtlpHttpExportClient(OtlpExporterOptions options, IHttpHandler htt

this.Headers = options.GetHeaders<Dictionary<string, string>>((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<string, string> Headers { get; }

/// <inheritdoc/>
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;
}

/// <inheritdoc/>
public virtual bool CancelExportRequest(int timeoutMilliseconds)
public bool CancelExportRequest(int timeoutMilliseconds)
{
try
{
this.HttpHandler.CancelPendingRequests();
this.HttpClient.CancelPendingRequests();
return true;
}
catch (Exception ex)
Expand All @@ -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
}
}
}
28 changes: 3 additions & 25 deletions ...y.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/OtlpHttpTraceExportClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -30,34 +29,13 @@ internal sealed class OtlpHttpTraceExportClient : BaseOtlpHttpExportClient<OtlpC
internal const string MediaContentType = "application/x-protobuf";
private readonly Uri exportTracesUri;

public OtlpHttpTraceExportClient(OtlpExporterOptions options, IHttpHandler httpHandler = null)
: base(options, httpHandler)
public OtlpHttpTraceExportClient(OtlpExporterOptions options, HttpClient httpClient = null)
: base(options, httpClient)
{
this.exportTracesUri = this.Options.Endpoint.AppendPathIfNotPresent(OtlpExporterOptions.TracesExportPath);
}

/// <inheritdoc/>
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)
Expand Down
52 changes: 0 additions & 52 deletions src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/HttpHandler.cs
View file

This file was deleted.

40 changes: 0 additions & 40 deletions src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/IHttpHandler.cs
View file

This file was deleted.

70 changes: 28 additions & 42 deletions ...OpenTelemetryProtocol.Tests/Implementation/ExportClient/OtlpHttpTraceExportClientTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<HttpHandler>(client.HttpHandler);
}

[Fact]
public void NewOtlpHttpTraceExportClient_OtlpExporterOptions_ExporterHasCorrectProperties()
{
Expand All @@ -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<NoopHttpHandler>(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);
Expand Down Expand Up @@ -98,21 +89,42 @@ public void SendExportRequest_ExportTraceServiceRequest_SendsCorrectHttpRequest(
Headers = $"{header1.Name}={header1.Value}, {header2.Name} = {header2.Value}",
};

var httpHandlerMock = new Mock<IHttpHandler>();
var httpHandlerMock = new Mock<HttpMessageHandler>();

HttpRequestMessage httpRequest = null;
var httpRequestContent = Array.Empty<byte>();

httpHandlerMock.Setup(h => h.Send(It.IsAny<HttpRequestMessage>(), It.IsAny<CancellationToken>()))
httpHandlerMock.Protected()
#if NET5_0_OR_GREATER
.Setup<HttpResponseMessage>("Send", ItExpr.IsAny<HttpRequestMessage>(), ItExpr.IsAny<CancellationToken>())
.Returns((HttpRequestMessage request, CancellationToken token) =>
{
return new HttpResponseMessage();
})
.Callback<HttpRequestMessage, CancellationToken>((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<Task<HttpResponseMessage>>("SendAsync", ItExpr.IsAny<HttpRequestMessage>(), ItExpr.IsAny<CancellationToken>())
.ReturnsAsync((HttpRequestMessage request, CancellationToken token) =>
{
return new HttpResponseMessage();
})
.Callback<HttpRequestMessage, CancellationToken>(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);

Expand Down Expand Up @@ -141,8 +153,6 @@ public void SendExportRequest_ExportTraceServiceRequest_SendsCorrectHttpRequest(
var result = exportClient.SendExportRequest(request);

// Assert
httpHandlerMock.Verify(m => m.Send(It.IsAny<HttpRequestMessage>(), It.IsAny<CancellationToken>()), Times.Once());

Assert.True(result);
Assert.NotNull(httpRequest);
Assert.Equal(HttpMethod.Post, httpRequest.Method);
Expand Down Expand Up @@ -173,29 +183,5 @@ public void SendExportRequest_ExportTraceServiceRequest_SendsCorrectHttpRequest(

Assert.Single(resourceSpan.InstrumentationLibrarySpans);
}

[Fact]
public void CancelExportRequest_PendingHttpRequestsCancelled()
{
var httpHandlerMock = new Mock<IHttpHandler>();

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;
}
}
}
}

0 comments on commit e850df0

Please sign in to comment.