From 79aa39fb7ee03dc57205c8c3a4eb5a98af2e34ab Mon Sep 17 00:00:00 2001 From: Nils Gruson Date: Wed, 29 Nov 2023 20:44:13 +0100 Subject: [PATCH] Removed usage of Mock (#5095) --- .../HttpClientTests.Basic.cs | 99 ++++++++++--------- .../HttpWebRequestTests.Basic.cs | 29 +++--- .../HttpWebRequestTests.cs | 9 +- 3 files changed, 67 insertions(+), 70 deletions(-) diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs index 97794e53d54..1fe6ccee839 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs @@ -120,14 +120,7 @@ public void AddHttpClientInstrumentation_BadArgs() [InlineData(false)] public async Task InjectsHeadersAsync(bool shouldEnrich) { - var processor = new Mock>(); - processor.Setup(x => x.OnStart(It.IsAny())).Callback(c => - { - c.SetTag("enrichedWithHttpWebRequest", "no"); - c.SetTag("enrichedWithHttpWebResponse", "no"); - c.SetTag("enrichedWithHttpRequestMessage", "no"); - c.SetTag("enrichedWithHttpResponseMessage", "no"); - }); + var exportedItems = new List(); using var request = new HttpRequestMessage { @@ -167,15 +160,15 @@ public async Task InjectsHeadersAsync(bool shouldEnrich) }; } }) - .AddProcessor(processor.Object) + .AddInMemoryExporter(exportedItems) .Build()) { using var c = new HttpClient(); await c.SendAsync(request); } - Assert.Equal(5, processor.Invocations.Count); // SetParentProvider/OnStart/OnEnd/OnShutdown/Dispose called. - var activity = (Activity)processor.Invocations[2].Arguments[0]; + Assert.Single(exportedItems); + var activity = exportedItems[0]; Assert.Equal(ActivityKind.Client, activity.Kind); Assert.Equal(parent.TraceId, activity.Context.TraceId); @@ -198,17 +191,33 @@ public async Task InjectsHeadersAsync(bool shouldEnrich) #endif #if NETFRAMEWORK - Assert.Equal(shouldEnrich ? "yes" : "no", activity.Tags.Where(tag => tag.Key == "enrichedWithHttpWebRequest").FirstOrDefault().Value); - Assert.Equal(shouldEnrich ? "yes" : "no", activity.Tags.Where(tag => tag.Key == "enrichedWithHttpWebResponse").FirstOrDefault().Value); + if (shouldEnrich) + { + Assert.Equal("yes", activity.Tags.Where(tag => tag.Key == "enrichedWithHttpWebRequest").FirstOrDefault().Value); + Assert.Equal("yes", activity.Tags.Where(tag => tag.Key == "enrichedWithHttpWebResponse").FirstOrDefault().Value); + } + else + { + Assert.DoesNotContain(activity.Tags, tag => tag.Key == "enrichedWithHttpWebRequest"); + Assert.DoesNotContain(activity.Tags, tag => tag.Key == "enrichedWithHttpWebResponse"); + } - Assert.Equal("no", activity.Tags.Where(tag => tag.Key == "enrichedWithHttpRequestMessage").FirstOrDefault().Value); - Assert.Equal("no", activity.Tags.Where(tag => tag.Key == "enrichedWithHttpResponseMessage").FirstOrDefault().Value); + Assert.DoesNotContain(activity.Tags, tag => tag.Key == "enrichedWithHttpRequestMessage"); + Assert.DoesNotContain(activity.Tags, tag => tag.Key == "enrichedWithHttpResponseMessage"); #else - Assert.Equal("no", activity.Tags.Where(tag => tag.Key == "enrichedWithHttpWebRequest").FirstOrDefault().Value); - Assert.Equal("no", activity.Tags.Where(tag => tag.Key == "enrichedWithHttpWebResponse").FirstOrDefault().Value); + Assert.DoesNotContain(activity.Tags, tag => tag.Key == "enrichedWithHttpWebRequest"); + Assert.DoesNotContain(activity.Tags, tag => tag.Key == "enrichedWithHttpWebResponse"); - Assert.Equal(shouldEnrich ? "yes" : "no", activity.Tags.Where(tag => tag.Key == "enrichedWithHttpRequestMessage").FirstOrDefault().Value); - Assert.Equal(shouldEnrich ? "yes" : "no", activity.Tags.Where(tag => tag.Key == "enrichedWithHttpResponseMessage").FirstOrDefault().Value); + if (shouldEnrich) + { + Assert.Equal("yes", activity.Tags.Where(tag => tag.Key == "enrichedWithHttpRequestMessage").FirstOrDefault().Value); + Assert.Equal("yes", activity.Tags.Where(tag => tag.Key == "enrichedWithHttpResponseMessage").FirstOrDefault().Value); + } + else + { + Assert.DoesNotContain(activity.Tags, tag => tag.Key == "enrichedWithHttpRequestMessage"); + Assert.DoesNotContain(activity.Tags, tag => tag.Key == "enrichedWithHttpResponseMessage"); + } #endif } @@ -223,7 +232,7 @@ public async Task InjectsHeadersAsync_CustomFormat() action(message, "custom_tracestate", Activity.Current.TraceStateString); }); - var processor = new Mock>(); + var exportedItems = new List(); using var request = new HttpRequestMessage { @@ -241,15 +250,15 @@ public async Task InjectsHeadersAsync_CustomFormat() using (Sdk.CreateTracerProviderBuilder() .AddHttpClientInstrumentation() - .AddProcessor(processor.Object) + .AddInMemoryExporter(exportedItems) .Build()) { using var c = new HttpClient(); await c.SendAsync(request); } - Assert.Equal(5, processor.Invocations.Count); // SetParentProvider/OnStart/OnEnd/OnShutdown/Dispose called. - var activity = (Activity)processor.Invocations[2].Arguments[0]; + Assert.Single(exportedItems); + var activity = exportedItems[0]; Assert.Equal(ActivityKind.Client, activity.Kind); Assert.Equal(parent.TraceId, activity.Context.TraceId); @@ -291,7 +300,7 @@ public async Task RespectsSuppress() action(message, "custom_tracestate", Activity.Current.TraceStateString); }); - var processor = new Mock>(); + var exportedItems = new List(); using var request = new HttpRequestMessage { @@ -309,7 +318,7 @@ public async Task RespectsSuppress() using (Sdk.CreateTracerProviderBuilder() .AddHttpClientInstrumentation() - .AddProcessor(processor.Object) + .AddInMemoryExporter(exportedItems) .Build()) { using var c = new HttpClient(); @@ -321,7 +330,7 @@ public async Task RespectsSuppress() // If suppressed, activity is not emitted and // propagation is also not performed. - Assert.Equal(3, processor.Invocations.Count); // SetParentProvider/OnShutdown/Dispose called. + Assert.Empty(exportedItems); Assert.False(request.Headers.Contains("custom_traceparent")); Assert.False(request.Headers.Contains("custom_tracestate")); } @@ -345,7 +354,7 @@ public async Task ExportsSpansCreatedForRetries() Method = new HttpMethod("GET"), }; - using var traceprovider = Sdk.CreateTracerProviderBuilder() + using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddHttpClientInstrumentation() .AddInMemoryExporter(exportedItems) .Build(); @@ -357,7 +366,7 @@ public async Task ExportsSpansCreatedForRetries() await httpClient.SendAsync(request); // number of exported spans should be 3(maxRetries) - Assert.Equal(maxRetries, exportedItems.Count()); + Assert.Equal(maxRetries, exportedItems.Count); var spanid1 = exportedItems[0].SpanId; var spanid2 = exportedItems[1].SpanId; @@ -390,7 +399,7 @@ public async Task HttpRequestMethodIsSetOnActivityAsPerSpec(string originalMetho Method = new HttpMethod(originalMethod), }; - using var traceprovider = Sdk.CreateTracerProviderBuilder() + using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddHttpClientInstrumentation() .AddInMemoryExporter(exportedItems) .Build(); @@ -447,7 +456,7 @@ public async Task HttpRequestMethodIsSetonRequestDurationMetricAsPerSpec(string Method = new HttpMethod(originalMethod), }; - using var meterprovider = Sdk.CreateMeterProviderBuilder() + using var meterProvider = Sdk.CreateMeterProviderBuilder() .AddHttpClientInstrumentation() .AddInMemoryExporter(metricItems) .Build(); @@ -463,7 +472,7 @@ public async Task HttpRequestMethodIsSetonRequestDurationMetricAsPerSpec(string // ignore error. } - meterprovider.Dispose(); + meterProvider.Dispose(); var metric = metricItems.FirstOrDefault(m => m.Name == "http.client.request.duration"); @@ -493,10 +502,10 @@ public async Task HttpRequestMethodIsSetonRequestDurationMetricAsPerSpec(string [Fact] public async Task RedirectTest() { - var processor = new Mock>(); + var exportedItems = new List(); using (Sdk.CreateTracerProviderBuilder() .AddHttpClientInstrumentation() - .AddProcessor(processor.Object) + .AddInMemoryExporter(exportedItems) .Build()) { using var c = new HttpClient(); @@ -509,18 +518,12 @@ public async Task RedirectTest() // good way to produce two spans when redirecting that we have // found. For now, this is not supported. - Assert.Equal(5, processor.Invocations.Count); // SetParentProvider/OnStart/OnEnd/OnShutdown/Dispose called. - - var firstActivity = (Activity)processor.Invocations[2].Arguments[0]; // First OnEnd - Assert.Contains(firstActivity.TagObjects, t => t.Key == "http.response.status_code" && (int)t.Value == 200); + Assert.Single(exportedItems); + Assert.Contains(exportedItems[0].TagObjects, t => t.Key == "http.response.status_code" && (int)t.Value == 200); #else - Assert.Equal(7, processor.Invocations.Count); // SetParentProvider/OnStart/OnEnd/OnStart/OnEnd/OnShutdown/Dispose called. - - var firstActivity = (Activity)processor.Invocations[2].Arguments[0]; // First OnEnd - Assert.Contains(firstActivity.TagObjects, t => t.Key == "http.response.status_code" && (int)t.Value == 302); - - var secondActivity = (Activity)processor.Invocations[4].Arguments[0]; // Second OnEnd - Assert.Contains(secondActivity.TagObjects, t => t.Key == "http.response.status_code" && (int)t.Value == 200); + Assert.Equal(2, exportedItems.Count); + Assert.Contains(exportedItems[0].TagObjects, t => t.Key == "http.response.status_code" && (int)t.Value == 302); + Assert.Contains(exportedItems[1].TagObjects, t => t.Key == "http.response.status_code" && (int)t.Value == 200); #endif } @@ -595,7 +598,7 @@ public async Task ReportsExceptionEventForNetworkFailuresWithGetAsync() var exportedItems = new List(); bool exceptionThrown = false; - using var traceprovider = Sdk.CreateTracerProviderBuilder() + using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddHttpClientInstrumentation(o => o.RecordException = true) .AddInMemoryExporter(exportedItems) .Build(); @@ -621,7 +624,7 @@ public async Task DoesNotReportExceptionEventOnErrorResponseWithGetAsync() var exportedItems = new List(); bool exceptionThrown = false; - using var traceprovider = Sdk.CreateTracerProviderBuilder() + using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddHttpClientInstrumentation(o => o.RecordException = true) .AddInMemoryExporter(exportedItems) .Build(); @@ -652,7 +655,7 @@ public async Task DoesNotReportExceptionEventOnErrorResponseWithGetStringAsync() Method = new HttpMethod("GET"), }; - using var traceprovider = Sdk.CreateTracerProviderBuilder() + using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddHttpClientInstrumentation(o => o.RecordException = true) .AddInMemoryExporter(exportedItems) .Build(); @@ -706,7 +709,7 @@ public async Task CustomPropagatorCalled(bool sample, bool createParentActivity) var exportedItems = new List(); - using (var traceprovider = Sdk.CreateTracerProviderBuilder() + using (var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddHttpClientInstrumentation() .AddInMemoryExporter(exportedItems) .SetSampler(sample ? new ParentBasedSampler(new AlwaysOnSampler()) : new AlwaysOffSampler()) diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.cs index 40f0cf9a873..be35cedaee4 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.cs @@ -75,9 +75,9 @@ public void Dispose() [Fact] public async Task BacksOffIfAlreadyInstrumented() { - var activityProcessor = new Mock>(); + var exportedItems = new List(); using var tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddProcessor(activityProcessor.Object) + .AddInMemoryExporter(exportedItems) .AddHttpClientInstrumentation() .Build(); @@ -90,12 +90,9 @@ public async Task BacksOffIfAlreadyInstrumented() using var response = await request.GetResponseAsync(); #if NETFRAMEWORK - // Note: Back-off is part of the .NET Framework reflection only and - // is needed to prevent issues when the same request is re-used for - // things like redirects or SSL negotiation. - Assert.Single(activityProcessor.Invocations); // SetParentProvider called + Assert.Empty(exportedItems); #else - Assert.Equal(3, activityProcessor.Invocations.Count); // SetParentProvider/Begin/End called + Assert.Single(exportedItems); #endif } @@ -105,7 +102,7 @@ public async Task RequestNotCollectedWhenInstrumentationFilterApplied() bool httpWebRequestFilterApplied = false; bool httpRequestMessageFilterApplied = false; - List exportedItems = new(); + var exportedItems = new List(); using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddInMemoryExporter(exportedItems) @@ -145,7 +142,7 @@ public async Task RequestNotCollectedWhenInstrumentationFilterApplied() [Fact] public async Task RequestNotCollectedWhenInstrumentationFilterThrowsException() { - List exportedItems = new(); + var exportedItems = new List(); using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddInMemoryExporter(exportedItems) @@ -174,9 +171,9 @@ public async Task RequestNotCollectedWhenInstrumentationFilterThrowsException() [Fact] public async Task InjectsHeadersAsync() { - var activityProcessor = new Mock>(); + var exportedItems = new List(); using var tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddProcessor(activityProcessor.Object) + .AddInMemoryExporter(exportedItems) .AddHttpClientInstrumentation() .Build(); @@ -192,8 +189,8 @@ public async Task InjectsHeadersAsync() using var response = await request.GetResponseAsync(); - Assert.Equal(3, activityProcessor.Invocations.Count); // SetParentProvider/Begin/End called - var activity = (Activity)activityProcessor.Invocations[2].Arguments[0]; + Assert.Single(exportedItems); + var activity = exportedItems[0]; Assert.Equal(parent.TraceId, activity.Context.TraceId); Assert.Equal(parent.SpanId, activity.ParentSpanId); @@ -312,13 +309,11 @@ public void AddHttpClientInstrumentationUsesOptionsApi(string name) int configurationDelegateInvocations = 0; - var activityProcessor = new Mock>(); using var tracerProvider = Sdk.CreateTracerProviderBuilder() .ConfigureServices(services => { services.Configure(name, o => configurationDelegateInvocations++); }) - .AddProcessor(activityProcessor.Object) .AddHttpClientInstrumentation(name, options => { Assert.IsType(options); @@ -334,7 +329,7 @@ public async Task ReportsExceptionEventForNetworkFailures() var exportedItems = new List(); bool exceptionThrown = false; - using var traceprovider = Sdk.CreateTracerProviderBuilder() + using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddHttpClientInstrumentation(o => o.RecordException = true) .AddInMemoryExporter(exportedItems) .Build(); @@ -363,7 +358,7 @@ public async Task ReportsExceptionEventOnErrorResponse() var exportedItems = new List(); bool exceptionThrown = false; - using var traceprovider = Sdk.CreateTracerProviderBuilder() + using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddHttpClientInstrumentation(o => o.RecordException = true) .AddInMemoryExporter(exportedItems) .Build(); diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.cs index 1a95c0cdbf6..a9ed60c07e6 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.cs @@ -17,7 +17,6 @@ using System.Diagnostics; using System.Net; using System.Text.Json; -using Moq; using OpenTelemetry.Tests; using OpenTelemetry.Trace; using Xunit; @@ -49,9 +48,9 @@ public void HttpOutCallsAreCollectedSuccessfully(HttpTestData.HttpOutTestCase tc bool enrichWithHttpResponseMessageCalled = false; bool enrichWithExceptionCalled = false; - var activityProcessor = new Mock>(); + var exportedItems = new List(); using var tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddProcessor(activityProcessor.Object) + .AddInMemoryExporter(exportedItems) .AddHttpClientInstrumentation(options => { options.EnrichWithHttpWebRequest = (activity, httpWebRequest) => { enrichWithHttpWebRequestCalled = true; }; @@ -92,8 +91,8 @@ public void HttpOutCallsAreCollectedSuccessfully(HttpTestData.HttpOutTestCase tc tc.ResponseExpected = false; } - Assert.Equal(3, activityProcessor.Invocations.Count); // SetParentProvider/Begin/End called - var activity = (Activity)activityProcessor.Invocations[2].Arguments[0]; + Assert.Single(exportedItems); + var activity = exportedItems[0]; ValidateHttpWebRequestActivity(activity); Assert.Equal(tc.SpanName, activity.DisplayName);