From 58790c60e5daed0dccbe56f165d0e79a748996fb Mon Sep 17 00:00:00 2001 From: Rajkumar Rangaraj Date: Wed, 26 Jun 2024 15:49:36 -0700 Subject: [PATCH 1/3] Remove logs sampling. --- .../CHANGELOG.md | 8 ++-- .../src/AzureMonitorAspNetCoreEventSource.cs | 20 ++------- .../src/LogFilteringProcessor.cs | 24 ----------- .../src/OpenTelemetryBuilderExtensions.cs | 25 ++--------- .../E2EAzureMonitorDistroTests.cs | 42 ------------------- 5 files changed, 11 insertions(+), 108 deletions(-) delete mode 100644 sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/src/LogFilteringProcessor.cs diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/CHANGELOG.md b/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/CHANGELOG.md index 58a470ca130ee..e3b6c6bc93f7c 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/CHANGELOG.md +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/CHANGELOG.md @@ -15,11 +15,6 @@ (This feature was originally introduced in 1.2.0-beta.2) ([#44511](https://github.com/Azure/azure-sdk-for-net/pull/44511)) -* Added an experimental feature for logs emitted within an active tracing context to follow the Activity's sampling decision. - The feature can be enabled by setting `OTEL_DOTNET_AZURE_MONITOR_EXPERIMENTAL_ENABLE_LOG_SAMPLING` environment variable to `true`. - (This feature was originally introduced in 1.2.0-beta.1) - ([#44511](https://github.com/Azure/azure-sdk-for-net/pull/44511)) - * Update OpenTelemetry dependencies. ([#44650](https://github.com/Azure/azure-sdk-for-net/pull/44650)) - OpenTelemetry 1.9.0 @@ -35,6 +30,9 @@ Code has been updated to [1.0.0-beta.8](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/tree/Resources.Azure-1.0.0-beta.8/src/OpenTelemetry.Resources.Azure). ([#44682](https://github.com/Azure/azure-sdk-for-net/pull/44682)) +* Removed an experimental feature for logs emitted within an active tracing context to follow the Activity's sampling decision. + ([#](https://github.com/Azure/azure-sdk-for-net/pull/)) + ## 1.2.0 (2024-06-11) ### Other Changes diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/src/AzureMonitorAspNetCoreEventSource.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/src/AzureMonitorAspNetCoreEventSource.cs index 9e5c48e8d6c98..b170ac508c4ed 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/src/AzureMonitorAspNetCoreEventSource.cs +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/src/AzureMonitorAspNetCoreEventSource.cs @@ -72,23 +72,11 @@ public void ConfigureFailed(System.Exception ex) [Event(4, Message = "Vendor instrumentation added for: {0}.", Level = EventLevel.Verbose)] public void VendorInstrumentationAdded(string packageName) => WriteEvent(4, packageName); - [NonEvent] - public void GetEnvironmentVariableFailed(string envVarName, System.Exception ex) - { - if (IsEnabled(EventLevel.Error)) - { - GetEnvironmentVariableFailed(envVarName, ex.FlattenException().ToInvariantString()); - } - } - - [Event(5, Message = "Failed to Read environment variable {0}, exception: {1}", Level = EventLevel.Error)] - public void GetEnvironmentVariableFailed(string envVarName, string exceptionMessage) => WriteEvent(5, envVarName, exceptionMessage); - - [Event(6, Message = "Failed to map unknown EventSource log level in AzureEventSourceLogForwarder {0}", Level = EventLevel.Warning)] - public void MapLogLevelFailed(string level) => WriteEvent(6, level); + [Event(5, Message = "Failed to map unknown EventSource log level in AzureEventSourceLogForwarder {0}", Level = EventLevel.Warning)] + public void MapLogLevelFailed(string level) => WriteEvent(5, level); - [Event(7, Message = "Found existing Microsoft.Extensions.Azure.AzureEventSourceLogForwarder registration.", Level = EventLevel.Informational)] - public void LogForwarderIsAlreadyRegistered() => WriteEvent(7); + [Event(6, Message = "Found existing Microsoft.Extensions.Azure.AzureEventSourceLogForwarder registration.", Level = EventLevel.Informational)] + public void LogForwarderIsAlreadyRegistered() => WriteEvent(6); [NonEvent] public void FailedToParseConnectionString(System.Exception ex) diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/src/LogFilteringProcessor.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/src/LogFilteringProcessor.cs deleted file mode 100644 index ede796dcb060d..0000000000000 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/src/LogFilteringProcessor.cs +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -using OpenTelemetry.Logs; -using OpenTelemetry; - -namespace Azure.Monitor.OpenTelemetry.AspNetCore -{ - internal class LogFilteringProcessor : BatchLogRecordExportProcessor - { - public LogFilteringProcessor(BaseExporter exporter) - : base(exporter) - { - } - - public override void OnEnd(LogRecord logRecord) - { - if (logRecord.SpanId == default || logRecord.TraceFlags == ActivityTraceFlags.Recorded) - { - base.OnEnd(logRecord); - } - } - } -} diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/src/OpenTelemetryBuilderExtensions.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/src/OpenTelemetryBuilderExtensions.cs index 68cdf8237b47b..dffea770ed0aa 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/src/OpenTelemetryBuilderExtensions.cs +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/src/OpenTelemetryBuilderExtensions.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. + // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. using System.Reflection; @@ -29,8 +29,6 @@ public static class OpenTelemetryBuilderExtensions { private const string SqlClientInstrumentationPackageName = "OpenTelemetry.Instrumentation.SqlClient"; - private const string EnableLogSamplingEnvVar = "OTEL_DOTNET_AZURE_MONITOR_EXPERIMENTAL_ENABLE_LOG_SAMPLING"; - /// /// Configures Azure Monitor for logging, distributed tracing, and metrics. /// @@ -135,39 +133,24 @@ public static OpenTelemetryBuilder UseAzureMonitor(this OpenTelemetryBuilder bui builder.WithLogging( logging => logging.AddProcessor(sp => { - bool enableLogSampling = false; - try - { - var enableLogSamplingEnvVar = Environment.GetEnvironmentVariable(EnableLogSamplingEnvVar); - bool.TryParse(enableLogSamplingEnvVar, out enableLogSampling); - } - catch (Exception ex) - { - AzureMonitorAspNetCoreEventSource.Log.GetEnvironmentVariableFailed(EnableLogSamplingEnvVar, ex); - } - var azureMonitorOptions = sp.GetRequiredService>().CurrentValue; var azureMonitorExporterOptions = new AzureMonitorExporterOptions(); azureMonitorOptions.SetValueToExporterOptions(azureMonitorExporterOptions); var exporter = new AzureMonitorLogExporter(azureMonitorExporterOptions); - var logProcessor = enableLogSampling - ? new LogFilteringProcessor(exporter) - : new BatchLogRecordExportProcessor(exporter); if (azureMonitorOptions.EnableLiveMetrics) { var manager = sp.GetRequiredService(); - var liveMetricsProcessor = new LiveMetricsLogProcessor(manager); return new CompositeProcessor(new BaseProcessor[] { - liveMetricsProcessor, - logProcessor + new LiveMetricsLogProcessor(manager), + new BatchLogRecordExportProcessor(exporter) }); } - return logProcessor; + return new BatchLogRecordExportProcessor(exporter); }), options => options.IncludeFormattedMessage = true); diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/tests/Azure.Monitor.OpenTelemetry.AspNetCore.Tests/E2EAzureMonitorDistroTests.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/tests/Azure.Monitor.OpenTelemetry.AspNetCore.Tests/E2EAzureMonitorDistroTests.cs index 4b3f7cddb2015..84b311b5a8a71 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/tests/Azure.Monitor.OpenTelemetry.AspNetCore.Tests/E2EAzureMonitorDistroTests.cs +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/tests/Azure.Monitor.OpenTelemetry.AspNetCore.Tests/E2EAzureMonitorDistroTests.cs @@ -16,9 +16,6 @@ using System.Collections.Generic; using System.Linq; using Xunit.Abstractions; -using OpenTelemetry.Logs; -using OpenTelemetry; -using System.Reflection; namespace Azure.Monitor.OpenTelemetry.AspNetCore.Tests { @@ -80,45 +77,6 @@ public async Task ValidateTelemetryExport() // TODO: This test needs to assert telemetry content. (ie: sample rate) } - [Theory(Skip = "TODO: Ordering needs to be fixed for processor in this test.")] - [InlineData(null)] - [InlineData("true")] - [InlineData("True")] - [InlineData("False")] - [InlineData("false")] - public void ValidateLogFilteringProcessorIsAddedToLoggerProvider(string enableLogSampling) - { - try - { - Environment.SetEnvironmentVariable("OTEL_DOTNET_AZURE_MONITOR_EXPERIMENTAL_ENABLE_LOG_SAMPLING", enableLogSampling); - - var sv = new ServiceCollection(); - sv.AddOpenTelemetry().UseAzureMonitor(o => o.ConnectionString = "InstrumentationKey=00000000-0000-0000-0000-000000000000"); - - var sp = sv.BuildServiceProvider(); - var loggerProvider = sp.GetRequiredService(); - var sdkProvider = typeof(OpenTelemetryLoggerProvider).GetField("Provider", BindingFlags.Instance | BindingFlags.NonPublic)?.GetValue(loggerProvider); - var processor = sdkProvider?.GetType().GetProperty("Processor", BindingFlags.Instance | BindingFlags.Public)?.GetMethod?.Invoke(sdkProvider, null); - - Assert.NotNull(processor); - - if (enableLogSampling != null && enableLogSampling.Equals("true" , StringComparison.OrdinalIgnoreCase)) - { - Assert.True(processor is LogFilteringProcessor); - Assert.True(processor is BatchLogRecordExportProcessor); - } - else - { - Assert.True(processor is not LogFilteringProcessor); - Assert.True(processor is BatchLogRecordExportProcessor); - } - } - finally - { - Environment.SetEnvironmentVariable("OTEL_DOTNET_AZURE_MONITOR_EXPERIMENTAL_ENABLE_LOG_SAMPLING", null); - } - } - private void WaitForRequest(MockTransport transport) { SpinWait.SpinUntil( From 4248c6eb8301b781bbd0789ff9062c72076bde69 Mon Sep 17 00:00:00 2001 From: Rajkumar Rangaraj Date: Wed, 26 Jun 2024 15:51:52 -0700 Subject: [PATCH 2/3] changelog update --- sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/CHANGELOG.md | 2 +- .../src/OpenTelemetryBuilderExtensions.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/CHANGELOG.md b/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/CHANGELOG.md index e3b6c6bc93f7c..566f890711bbb 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/CHANGELOG.md +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/CHANGELOG.md @@ -31,7 +31,7 @@ ([#44682](https://github.com/Azure/azure-sdk-for-net/pull/44682)) * Removed an experimental feature for logs emitted within an active tracing context to follow the Activity's sampling decision. - ([#](https://github.com/Azure/azure-sdk-for-net/pull/)) + ([#44745](https://github.com/Azure/azure-sdk-for-net/pull/44745)) ## 1.2.0 (2024-06-11) diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/src/OpenTelemetryBuilderExtensions.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/src/OpenTelemetryBuilderExtensions.cs index dffea770ed0aa..071ab6f92b33e 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/src/OpenTelemetryBuilderExtensions.cs +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/src/OpenTelemetryBuilderExtensions.cs @@ -1,4 +1,4 @@ - // Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. using System.Reflection; From 08f91114f44541dfc0978fc549742b92cfdfae7e Mon Sep 17 00:00:00 2001 From: Rajkumar Rangaraj Date: Wed, 26 Jun 2024 16:25:57 -0700 Subject: [PATCH 3/3] pr feedback - changelog --- sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/CHANGELOG.md b/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/CHANGELOG.md index 566f890711bbb..a265b8879d510 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/CHANGELOG.md +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.AspNetCore/CHANGELOG.md @@ -31,6 +31,7 @@ ([#44682](https://github.com/Azure/azure-sdk-for-net/pull/44682)) * Removed an experimental feature for logs emitted within an active tracing context to follow the Activity's sampling decision. + (This feature was originally introduced in 1.2.0-beta.1) ([#44745](https://github.com/Azure/azure-sdk-for-net/pull/44745)) ## 1.2.0 (2024-06-11)