From b5dd5c286602896392feb7dbff1cccbed9254cf7 Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Tue, 21 Feb 2023 16:40:32 -0800 Subject: [PATCH 1/4] wip --- .../AzureMonitorTraceExporterLiveTests.cs | 15 ++++++--------- .../Controllers/HomeController.cs | 4 +--- .../tests/Integration.Tests/BasicTests.cs | 5 +---- .../Integration.Tests/RequestTelemetryTests.cs | 8 +++----- 4 files changed, 11 insertions(+), 21 deletions(-) diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/E2E.Tests/AzureMonitorTraceExporterLiveTests.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/E2E.Tests/AzureMonitorTraceExporterLiveTests.cs index 56be815019ee8..06d7743c9901e 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/E2E.Tests/AzureMonitorTraceExporterLiveTests.cs +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/E2E.Tests/AzureMonitorTraceExporterLiveTests.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -#nullable disable // TODO: remove and fix errors - #if !NETCOREAPP3_1 namespace Azure.Monitor.OpenTelemetry.Exporter.E2E.Tests { @@ -56,16 +54,15 @@ public async Task VerifyTraceExporter() } // Export - tracerProvider.ForceFlush(); + tracerProvider?.ForceFlush(); // Query var client = CreateClient(); string query = $"AppDependencies | where AppRoleName == '{nameof(VerifyTraceExporter)}' | top 1 by TimeGenerated"; - LogsTable table = await CheckForRecord(client, query); - - var rowCount = table.Rows.Count(); + LogsTable? table = await CheckForRecord(client, query); + var rowCount = table?.Rows.Count(); // Assert @@ -76,13 +73,13 @@ public async Task VerifyTraceExporter() } else { - Assert.True(table.Rows.Count() == 1); + Assert.True(table?.Rows.Count() == 1); } } - private async Task CheckForRecord(LogsQueryClient client, string query) + private async Task CheckForRecord(LogsQueryClient client, string query) { - LogsTable table = null; + LogsTable? table = null; int count = 0; // Try every 30 secs for total of 5 minutes. diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Integration.Tests.AspNetCoreWebApp/Controllers/HomeController.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Integration.Tests.AspNetCoreWebApp/Controllers/HomeController.cs index de395774b82d4..c9e6b00f9fe4f 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Integration.Tests.AspNetCoreWebApp/Controllers/HomeController.cs +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Integration.Tests.AspNetCoreWebApp/Controllers/HomeController.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -#nullable disable // TODO: remove and fix errors - using System.Net; using Microsoft.AspNetCore.Mvc; @@ -24,7 +22,7 @@ public class HomeController : ControllerBase /// Set this value to a random value and use this value to distinguish requests in any unit tests. /// [HttpGet("{id?}")] - public ActionResult Get(string id = null) => StatusCode((int)HttpStatusCode.OK); + public ActionResult Get(string? id = null) => StatusCode((int)HttpStatusCode.OK); /// /// This URI will return the matching the provided value. diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Integration.Tests/BasicTests.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Integration.Tests/BasicTests.cs index 787b0f4e0451e..ed31d24f380ae 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Integration.Tests/BasicTests.cs +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Integration.Tests/BasicTests.cs @@ -1,12 +1,9 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -#nullable disable // TODO: remove and fix errors - using System; using System.Net; using System.Threading.Tasks; -using Azure.Core.TestFramework; using Microsoft.AspNetCore.Mvc.Testing; using Xunit; @@ -32,7 +29,7 @@ public async Task VerifyRequest(HttpStatusCode httpStatusCode) { // Arrange var client = this.factory.CreateClient(); - var request = new Uri(client.BaseAddress, $"api/home/statuscode/{(int)httpStatusCode}"); + var request = new Uri(client.BaseAddress!, $"api/home/statuscode/{(int)httpStatusCode}"); // Act var response = await client.GetAsync(request); diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Integration.Tests/RequestTelemetryTests.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Integration.Tests/RequestTelemetryTests.cs index 0e17072cc2d55..aed0284fb84ab 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Integration.Tests/RequestTelemetryTests.cs +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Integration.Tests/RequestTelemetryTests.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -#nullable disable // TODO: remove and fix errors - using System; using System.Collections.Concurrent; using System.Linq; @@ -15,7 +13,6 @@ using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.AspNetCore.TestHost; using Microsoft.Extensions.DependencyInjection; -using OpenTelemetry; using OpenTelemetry.Trace; using Xunit; @@ -39,7 +36,7 @@ public async Task VerifyRequestTelemetry() { string testValue = Guid.NewGuid().ToString(); - ConcurrentBag telemetryItems = null; + ConcurrentBag? telemetryItems = null; // Arrange var client = this.factory @@ -54,11 +51,12 @@ public async Task VerifyRequestTelemetry() .CreateClient(); // Act - var request = new Uri(client.BaseAddress, $"api/home/{testValue}"); + var request = new Uri(client.BaseAddress!, $"api/home/{testValue}"); var response = await client.GetAsync(request); // Shutdown response.EnsureSuccessStatusCode(); + Assert.NotNull(telemetryItems); this.WaitForActivityExport(telemetryItems); // Assert From 3cf23721e8f25298f27314777d316d303bad59d9 Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Tue, 21 Feb 2023 17:52:50 -0800 Subject: [PATCH 2/4] wip --- .../AzureMonitorTraceExporterTests.cs | 24 ++++++++++--------- .../OfflineStorageTests.cs | 15 +++++++----- .../SampleRateTests.cs | 14 +++++------ 3 files changed, 29 insertions(+), 24 deletions(-) diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/AzureMonitorTraceExporterTests.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/AzureMonitorTraceExporterTests.cs index a54fe4e968aaa..da7b043d5c496 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/AzureMonitorTraceExporterTests.cs +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/AzureMonitorTraceExporterTests.cs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -#nullable disable // TODO: remove and fix errors +//#nullable disable // TODO: remove and fix errors using OpenTelemetry.Trace; using System; @@ -24,7 +24,7 @@ public void VerifyConnectionString_CorrectlySetsEndpoint() var exporter = new AzureMonitorTraceExporter(new AzureMonitorExporterOptions { ConnectionString = $"InstrumentationKey={testIkey};IngestionEndpoint={testEndpoint}" }); - GetInternalFields(exporter, out string ikey, out string endpoint); + GetInternalFields(exporter, out string? ikey, out string? endpoint); Assert.Equal(testIkey, ikey); Assert.Equal(testEndpoint, endpoint); } @@ -36,7 +36,7 @@ public void VerifyConnectionString_CorrectlySetsDefaultEndpoint() var exporter = new AzureMonitorTraceExporter(new AzureMonitorExporterOptions { ConnectionString = $"InstrumentationKey={testIkey};" }); - GetInternalFields(exporter, out string ikey, out string endpoint); + GetInternalFields(exporter, out string? ikey, out string? endpoint); Assert.Equal(testIkey, ikey); Assert.Equal(Constants.DefaultIngestionEndpoint, endpoint); } @@ -58,11 +58,13 @@ public void VerifyConnectionString_ThrowsExceptionWhenMissingInstrumentationKey( [Fact] public void AzureMonitorExporter_BadArgs() { +#pragma warning disable CS8600 // Converting null literal or possible null value to non-nullable type. TracerProviderBuilder builder = null; - Assert.Throws(() => builder.AddAzureMonitorTraceExporter()); + Assert.Throws(() => builder!.AddAzureMonitorTraceExporter()); +#pragma warning restore CS8600 // Converting null literal or possible null value to non-nullable type. } - private void GetInternalFields(AzureMonitorTraceExporter exporter, out string ikey, out string endpoint) + private void GetInternalFields(AzureMonitorTraceExporter exporter, out string? ikey, out string? endpoint) { // TODO: NEED A BETTER APPROACH FOR TESTING. WE DECIDED AGAINST MAKING FIELDS "internal". // instrumentationKey: AzureMonitorTraceExporter.AzureMonitorTransmitter.instrumentationKey @@ -70,21 +72,21 @@ private void GetInternalFields(AzureMonitorTraceExporter exporter, out string ik ikey = typeof(AzureMonitorTraceExporter) .GetField("_instrumentationKey", BindingFlags.Instance | BindingFlags.NonPublic) - .GetValue(exporter) - .ToString(); + ?.GetValue(exporter) + ?.ToString(); var transmitter = typeof(AzureMonitorTraceExporter) .GetField("_transmitter", BindingFlags.Instance | BindingFlags.NonPublic) - .GetValue(exporter); + ?.GetValue(exporter); var serviceRestClient = typeof(AzureMonitorTransmitter) .GetField("_applicationInsightsRestClient", BindingFlags.Instance | BindingFlags.NonPublic) - .GetValue(transmitter); + ?.GetValue(transmitter); endpoint = typeof(ApplicationInsightsRestClient) .GetField("_host", BindingFlags.Instance | BindingFlags.NonPublic) - .GetValue(serviceRestClient) - .ToString(); + ?.GetValue(serviceRestClient) + ?.ToString(); } } } diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/OfflineStorageTests.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/OfflineStorageTests.cs index d4830e7d12648..74c84df92a5f5 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/OfflineStorageTests.cs +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/OfflineStorageTests.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -#nullable disable // TODO: remove and fix errors - using System; using System.Collections.Generic; using System.Diagnostics; @@ -59,6 +57,7 @@ public void Success200() transmitter.TrackAsync(telemetryItems, false, CancellationToken.None).EnsureCompleted(); //Assert + Assert.NotNull(transmitter._fileBlobProvider); Assert.Empty(transmitter._fileBlobProvider.GetBlobs()); } @@ -76,6 +75,7 @@ public void FailureResponseCode500() transmitter.TrackAsync(telemetryItems, false, CancellationToken.None).EnsureCompleted(); //Assert + Assert.NotNull(transmitter._fileBlobProvider); Assert.Single(transmitter._fileBlobProvider.GetBlobs()); } @@ -95,6 +95,7 @@ public void FailureResponseCode429() transmitter.TrackAsync(telemetryItems, false, CancellationToken.None).EnsureCompleted(); //Assert + Assert.NotNull(transmitter._fileBlobProvider); Assert.Single(transmitter._fileBlobProvider.GetBlobs()); } @@ -120,8 +121,9 @@ public void FailureResponseCode206() transmitter.TrackAsync(telemetryItems, false, CancellationToken.None).EnsureCompleted(); //Assert + Assert.NotNull(transmitter._fileBlobProvider); Assert.Single(transmitter._fileBlobProvider.GetBlobs()); - transmitter._fileBlobProvider.TryGetBlob(out var blob); + Assert.True(transmitter._fileBlobProvider.TryGetBlob(out var blob)); blob.TryRead(out var content); Assert.NotNull(content); @@ -190,13 +192,14 @@ private static Activity CreateActivity(string activityName) parentContext: default, startTime: DateTime.UtcNow); + Assert.NotNull(activity); return activity; } private static TelemetryItem CreateTelemetryItem(Activity activity) { var monitorTags = TraceHelper.EnumerateActivityTags(activity); - return new TelemetryItem(activity, ref monitorTags, null, null); + return new TelemetryItem(activity, ref monitorTags, null, string.Empty); } private class MockFileProvider : PersistentBlobProvider @@ -224,7 +227,7 @@ protected override bool OnTryCreateBlob(byte[] buffer, out PersistentBlob blob) protected override bool OnTryGetBlob(out PersistentBlob blob) { - blob = this.GetBlobs().FirstOrDefault(); + blob = this.GetBlobs().First(); return true; } @@ -232,7 +235,7 @@ protected override bool OnTryGetBlob(out PersistentBlob blob) private class MockFileBlob : PersistentBlob { - private byte[] _buffer; + private byte[] _buffer = Array.Empty(); private readonly List _mockStorage; diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/SampleRateTests.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/SampleRateTests.cs index e41fbd9c18740..65f64e0434f31 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/SampleRateTests.cs +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/SampleRateTests.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -#nullable disable // TODO: remove and fix errors - using System; using System.Collections.Concurrent; using System.Collections.Generic; @@ -52,8 +50,9 @@ public void ValidateSampleRateForEventException(object SampleRate) ActivityKind.Client, parentContext: default, startTime: DateTime.UtcNow, - tags: new Dictionary() { ["sampleRate"] = SampleRate }); + tags: new Dictionary() { ["sampleRate"] = SampleRate }); + Assert.NotNull(activity); var monitorTags = TraceHelper.EnumerateActivityTags(activity); var telemetryItem = new TelemetryItem(activity, ref monitorTags, null, "00000000-0000-0000-0000-000000000000"); var expTelemetryItem = new TelemetryItem("Exception", telemetryItem, default, default, default); @@ -83,8 +82,9 @@ public void ValidateSampleRateInTelemetry(object SampleRate) ActivityKind.Client, parentContext: default, startTime: DateTime.UtcNow, - tags: new Dictionary() { ["sampleRate"] = SampleRate }); + tags: new Dictionary() { ["sampleRate"] = SampleRate }); + Assert.NotNull(activity); var monitorTags = TraceHelper.EnumerateActivityTags(activity); var telemetryItem = new TelemetryItem(activity, ref monitorTags, null, "00000000-0000-0000-0000-000000000000"); @@ -112,10 +112,10 @@ public void SampleRateE2ETest() { } - tracerProvider.ForceFlush(); + tracerProvider?.ForceFlush(); Assert.NotEmpty(telemetryItems); - Assert.Equal(100F, telemetryItems.FirstOrDefault().SampleRate); + Assert.Equal(100F, telemetryItems.First().SampleRate); } [Fact] @@ -132,7 +132,7 @@ public void NoTelemetryCreatedOnZeroSampleRate() { } - tracerProvider.ForceFlush(); + tracerProvider?.ForceFlush(); Assert.Empty(telemetryItems); } From 5f1038c97e3fe8ec7c42c1bed58698d6d345e82f Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Tue, 21 Feb 2023 19:06:43 -0800 Subject: [PATCH 3/4] wip --- .../AzureMonitorTraceExporterTests.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/AzureMonitorTraceExporterTests.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/AzureMonitorTraceExporterTests.cs index da7b043d5c496..84e25a3c5cab7 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/AzureMonitorTraceExporterTests.cs +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/AzureMonitorTraceExporterTests.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -//#nullable disable // TODO: remove and fix errors - using OpenTelemetry.Trace; using System; using System.Reflection; From dd9be36c82b4b576281190bac35d963a886e1326 Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Tue, 21 Feb 2023 19:31:42 -0800 Subject: [PATCH 4/4] wip --- .../Models/TelemetryExceptionDetails.cs | 2 +- .../TelemetryExceptionDataTests.cs | 16 ++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Customizations/Models/TelemetryExceptionDetails.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Customizations/Models/TelemetryExceptionDetails.cs index 38989c48ae517..8dcdc18bb6d48 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Customizations/Models/TelemetryExceptionDetails.cs +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Customizations/Models/TelemetryExceptionDetails.cs @@ -14,7 +14,7 @@ internal partial class TelemetryExceptionDetails /// /// Creates a new instance of ExceptionDetails from a System.Exception and a parent ExceptionDetails. /// - internal TelemetryExceptionDetails(Exception exception, string message, TelemetryExceptionDetails parentExceptionDetails) + internal TelemetryExceptionDetails(Exception exception, string message, TelemetryExceptionDetails? parentExceptionDetails) { if (exception == null) { diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/TelemetryExceptionDataTests.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/TelemetryExceptionDataTests.cs index 706315abfe90f..be8afa05ae8f1 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/TelemetryExceptionDataTests.cs +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/TelemetryExceptionDataTests.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -#nullable disable // TODO: remove and fix errors - using System; using System.Collections.Generic; using System.Diagnostics; @@ -47,7 +45,9 @@ public void ValidateProblemIdForExceptionWithStackTrace() [Fact] public void CallingConvertToExceptionDetailsWithNullExceptionThrowsArgumentNullException() { +#pragma warning disable CS8625 // Cannot convert null literal to non-nullable reference type. Assert.Throws(() => new TelemetryExceptionDetails(null, "Exception Message", null)); +#pragma warning restore CS8625 // Cannot convert null literal to non-nullable reference type. } [Fact] @@ -92,7 +92,11 @@ public void FirstAndLastStackPointsAreCollectedForLongStack() public void TestNullMethodInfoInStack() { var frameMock = new Mock(null, 0, 0); +#pragma warning disable CS8600 // Converting null literal or possible null value to non-nullable type. +#pragma warning disable CS8625 // Cannot convert null literal to non-nullable reference type. frameMock.Setup(x => x.GetMethod()).Returns((MethodBase)null); +#pragma warning restore CS8600 // Converting null literal or possible null value to non-nullable type. +#pragma warning restore CS8625 // Cannot convert null literal to non-nullable reference type. Models.StackFrame stackFrame = new Models.StackFrame(frameMock.Object, 0); @@ -109,8 +113,8 @@ public void CheckThatFileNameAndLineAreCorrectIfAvailable() StackTrace st = new StackTrace(exception, true); var frame = st.GetFrame(0); - var line = frame.GetFileLineNumber(); - var fileName = frame.GetFileName(); + var line = frame?.GetFileLineNumber(); + var fileName = frame?.GetFileName(); var exceptionDetails = new TelemetryExceptionDetails(exception, exception.Message, null); var stack = exceptionDetails.ParsedStack; @@ -357,7 +361,7 @@ public void ValidateTelemetryExceptionData(LogLevel logLevel) [MethodImpl(MethodImplOptions.NoInlining)] private Exception CreateException(int stackDepth) { - Exception exception = null; + Exception? exception = null; try { @@ -368,7 +372,7 @@ private Exception CreateException(int stackDepth) exception = exp; } - return exception; + return exception!; } [MethodImpl(MethodImplOptions.NoInlining)]