From 3aeefbdd5e975e287effaa8670422837dc661d68 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Tue, 12 Sep 2023 16:09:48 -0700 Subject: [PATCH] Metrics Feature Switch (#91767) --- docs/workflow/trimming/feature-switches.md | 1 + .../ILLink/ILLink.Substitutions.Shared.xml | 10 ++++ ...System.Diagnostics.DiagnosticSource.csproj | 4 ++ .../System/Diagnostics/Metrics/Instrument.cs | 6 ++ .../src/System/Diagnostics/Metrics/Meter.cs | 10 ++++ .../Diagnostics/Metrics/MeterListener.cs | 32 ++++++++++- ....Diagnostics.DiagnosticSource.Tests.csproj | 1 + .../tests/TestNotSupported.cs | 56 +++++++++++++++++++ 8 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 src/libraries/System.Diagnostics.DiagnosticSource/src/ILLink/ILLink.Substitutions.Shared.xml create mode 100644 src/libraries/System.Diagnostics.DiagnosticSource/tests/TestNotSupported.cs diff --git a/docs/workflow/trimming/feature-switches.md b/docs/workflow/trimming/feature-switches.md index 964869833b5f6..2c25a36230d4d 100644 --- a/docs/workflow/trimming/feature-switches.md +++ b/docs/workflow/trimming/feature-switches.md @@ -13,6 +13,7 @@ configurations but their defaults might vary as any SDK can set the defaults dif | EnableUnsafeBinaryFormatterSerialization | System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization | BinaryFormatter serialization support is trimmed when set to false | | EventSourceSupport | System.Diagnostics.Tracing.EventSource.IsSupported | Any EventSource related code or logic is trimmed when set to false | | InvariantGlobalization | System.Globalization.Invariant | All globalization specific code and data is trimmed when set to true | +| MetricsSupport | System.Diagnostics.Metrics.Meter.IsSupported | Any Metrics related code or logic is trimmed when set to false | | PredefinedCulturesOnly | System.Globalization.PredefinedCulturesOnly | Don't allow creating a culture for which the platform does not have data | | HybridGlobalization | System.Globalization.Hybrid | Properties connected with the mixed: platform-specific + icu-based globalization will be trimmed | | UseSystemResourceKeys | System.Resources.UseSystemResourceKeys | Any localizable resources for system assemblies is trimmed when set to true | diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/ILLink/ILLink.Substitutions.Shared.xml b/src/libraries/System.Diagnostics.DiagnosticSource/src/ILLink/ILLink.Substitutions.Shared.xml new file mode 100644 index 0000000000000..b67ac8623c402 --- /dev/null +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/ILLink/ILLink.Substitutions.Shared.xml @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj b/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj index b565a8f55b201..2afba2938d0d8 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj @@ -20,6 +20,10 @@ System.Diagnostics.DiagnosticSource true + + + + diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Instrument.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Instrument.cs index 0cb50f523f028..c1e2fcb199b3a 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Instrument.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Instrument.cs @@ -64,6 +64,12 @@ protected Instrument(Meter meter, string name, string? unit, string? description /// protected void Publish() { + // All instruments call Publish when they are created. We don't want to publish the instrument if the Meter is not supported. + if (!Meter.IsSupported) + { + return; + } + List? allListeners = null; lock (Instrument.SyncObject) { diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Meter.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Meter.cs index a5722aa7b6c4d..60314b1d5b4c0 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Meter.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Meter.cs @@ -17,6 +17,11 @@ public class Meter : IDisposable private Dictionary> _nonObservableInstrumentsCache = new(); internal bool Disposed { get; private set; } + internal static bool IsSupported { get; } = InitializeIsSupported(); + + private static bool InitializeIsSupported() => + AppContext.TryGetSwitch("System.Diagnostics.Metrics.Meter.IsSupported", out bool isSupported) ? isSupported : true; + /// /// Initialize a new instance of the Meter using the . /// @@ -77,6 +82,11 @@ private void Initialize(string name, string? version, IEnumerable - /// A delegate to represent the Meterlistener callbacks used in measurements recording operation. + /// A delegate to represent the MeterListener callbacks used in measurements recording operation. /// public delegate void MeasurementCallback(Instrument instrument, T measurement, ReadOnlySpan> tags, object? state) where T : struct; @@ -56,6 +56,11 @@ public MeterListener() { } /// A state object which will be passed back to the callback getting measurements events. public void EnableMeasurementEvents(Instrument instrument, object? state = null) { + if (!Meter.IsSupported) + { + return; + } + bool oldStateStored = false; bool enabled = false; object? oldState = null; @@ -92,6 +97,11 @@ public void EnableMeasurementEvents(Instrument instrument, object? state = null) /// The state object originally passed to method. public object? DisableMeasurementEvents(Instrument instrument) { + if (!Meter.IsSupported) + { + return default; + } + object? state = null; lock (Instrument.SyncObject) { @@ -114,6 +124,11 @@ public void EnableMeasurementEvents(Instrument instrument, object? state = null) /// The callback which can be used to get measurement recording of numeric type T. public void SetMeasurementEventCallback(MeasurementCallback? measurementCallback) where T : struct { + if (!Meter.IsSupported) + { + return; + } + measurementCallback ??= (instrument, measurement, tags, state) => { /* no-op */}; if (typeof(T) == typeof(byte)) @@ -155,6 +170,11 @@ public void SetMeasurementEventCallback(MeasurementCallback? measurementCa /// public void Start() { + if (!Meter.IsSupported) + { + return; + } + List? publishedInstruments = null; lock (Instrument.SyncObject) { @@ -184,6 +204,11 @@ public void Start() /// public void RecordObservableInstruments() { + if (!Meter.IsSupported) + { + return; + } + List? exceptionsList = null; DiagNode? current = _enabledMeasurementInstruments.First; while (current is not null) @@ -215,6 +240,11 @@ public void RecordObservableInstruments() /// public void Dispose() { + if (!Meter.IsSupported) + { + return; + } + Dictionary? callbacksArguments = null; Action? measurementsCompleted = MeasurementsCompleted; diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj b/src/libraries/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj index 7e8136e0a1a6b..29a8e55dc2fc0 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj @@ -11,6 +11,7 @@ + diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/TestNotSupported.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/TestNotSupported.cs new file mode 100644 index 0000000000000..43fae74995720 --- /dev/null +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/TestNotSupported.cs @@ -0,0 +1,56 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.DotNet.RemoteExecutor; +using System.Diagnostics.Metrics; +using Xunit; + +namespace System.Diagnostics.Metrics.Tests +{ + public class MetricsNotSupportedTest + { + /// + /// Tests using Metrics when the System.Diagnostics.Metrics.Meter.IsSupported + /// feature switch is set to disable all metrics operations. + /// + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public void IsSupportedSwitch(bool value) + { + RemoteInvokeOptions options = new RemoteInvokeOptions(); + options.RuntimeConfigurationOptions.Add("System.Diagnostics.Metrics.Meter.IsSupported", value); + + RemoteExecutor.Invoke((val) => + { + bool isSupported = bool.Parse(val); + + Meter meter = new Meter("IsSupportedTest"); + Counter counter = meter.CreateCounter("counter"); + bool instrumentsPublished = false; + bool instrumentCompleted = false; + long counterValue = 100; + + using (MeterListener listener = new MeterListener + { + InstrumentPublished = (instruments, theListener) => instrumentsPublished = true, + MeasurementsCompleted = (instruments, state) => instrumentCompleted = true + }) + { + listener.EnableMeasurementEvents(counter, null); + listener.SetMeasurementEventCallback((inst, measurement, tags, state) => counterValue = measurement); + listener.Start(); + + Assert.Equal(isSupported, counter.Enabled); + + counter.Add(20); + } + meter.Dispose(); + + Assert.Equal(isSupported, instrumentsPublished); + Assert.Equal(isSupported, instrumentCompleted); + Assert.Equal(isSupported ? 20 : 100, counterValue); + }, value.ToString(), options).Dispose(); + } + } +}