From 74b650d6b49bc725bb264aa8a585b175f2d3ad35 Mon Sep 17 00:00:00 2001 From: adams85 <31276480+adams85@users.noreply.github.com> Date: Wed, 21 Aug 2024 09:26:08 +0200 Subject: [PATCH] Custom log filtering (#95) * Allow custom log filtering (ConfigCatClientOptions.LogFilter) * Use modified version of FormattableStringFactory internally to avoid unnecessary heap allocations * Reduce heap allocation by deferring formatting of log and error messages until actually accessed by consumer --- .../ConfigCat.Client.Tests.csproj | 2 +- .../ConfigCatClientTests.cs | 30 +++++- .../Helpers/LoggingHelper.cs | 4 +- src/ConfigCat.Client.Tests/LoggerTests.cs | 37 +++++++ src/ConfigCat.Client.Tests/UtilsTests.cs | 22 ++++ src/ConfigCatClient/ConfigCatClient.cs | 7 +- .../ConfigService/ConfigServiceBase.cs | 4 +- .../Configuration/ConfigCatClientOptions.cs | 6 ++ .../Evaluation/EvaluationDetails.cs | 13 ++- .../Evaluation/EvaluationHelper.cs | 6 +- src/ConfigCatClient/FetchResult.cs | 14 ++- src/ConfigCatClient/FormattableString.cs | 102 +++--------------- .../FormattableStringFactory.cs | 33 ++---- src/ConfigCatClient/Hooks/Hooks.cs | 10 +- src/ConfigCatClient/Hooks/SafeHooksWrapper.cs | 4 +- src/ConfigCatClient/HttpConfigFetcher.cs | 10 +- .../Logging/FormattableLogMessage.cs | 9 +- .../Logging/IConfigCatLogger.cs | 10 ++ src/ConfigCatClient/Logging/LogMessages.cs | 3 +- .../Logging/LoggerExtensions.cs | 4 +- src/ConfigCatClient/Logging/LoggerWrapper.cs | 9 +- src/ConfigCatClient/RefreshResult.cs | 16 ++- .../Utils/IndentedTextBuilder.cs | 2 +- src/ConfigCatClient/Utils/LazyString.cs | 56 ++++++++++ 24 files changed, 254 insertions(+), 159 deletions(-) create mode 100644 src/ConfigCatClient/Utils/LazyString.cs diff --git a/src/ConfigCat.Client.Tests/ConfigCat.Client.Tests.csproj b/src/ConfigCat.Client.Tests/ConfigCat.Client.Tests.csproj index 3195e042..1438bb31 100644 --- a/src/ConfigCat.Client.Tests/ConfigCat.Client.Tests.csproj +++ b/src/ConfigCat.Client.Tests/ConfigCat.Client.Tests.csproj @@ -8,7 +8,7 @@ 10.0 enable nullable - CS0618 + CS0618;CS1685 ..\ConfigCatClient.snk diff --git a/src/ConfigCat.Client.Tests/ConfigCatClientTests.cs b/src/ConfigCat.Client.Tests/ConfigCatClientTests.cs index 022cec61..a8cc0c79 100644 --- a/src/ConfigCat.Client.Tests/ConfigCatClientTests.cs +++ b/src/ConfigCat.Client.Tests/ConfigCatClientTests.cs @@ -2107,14 +2107,14 @@ public async Task Hooks_MockedClientRaisesEvents() hooks.FlagEvaluated += (s, e) => flagEvaluatedEvents.Add(e); hooks.Error += (s, e) => errorEvents.Add(e); - var loggerWrapper = this.loggerMock.Object.AsWrapper(hooks); + var loggerWrapper = this.loggerMock.Object.AsWrapper(hooks: hooks); var errorException = new HttpRequestException(); var onFetch = (ProjectConfig latestConfig, CancellationToken _) => { var logMessage = loggerWrapper.FetchFailedDueToUnexpectedError(errorException); - return FetchResult.Failure(latestConfig, RefreshErrorCode.HttpRequestFailure, errorMessage: logMessage.InvariantFormattedMessage, errorException: errorException); + return FetchResult.Failure(latestConfig, RefreshErrorCode.HttpRequestFailure, errorMessage: logMessage.ToLazyString(), errorException: errorException); }; this.fetcherMock.Setup(m => m.FetchAsync(It.IsAny(), It.IsAny())).ReturnsAsync(onFetch); @@ -2306,6 +2306,30 @@ void Unsubscribe(IProvidesHooks hooks) Assert.AreEqual(2, errorEvents.Count); } + [TestMethod] + public async Task LogFilter_Works() + { + var logEvents = new List(); + var logger = LoggingHelper.CreateCapturingLogger(logEvents, LogLevel.Info); + + var options = new ConfigCatClientOptions + { + Logger = logger, + LogFilter = (LogLevel level, LogEventId eventId, ref FormattableLogMessage message, Exception? exception) => eventId != 3001, + FlagOverrides = FlagOverrides.LocalFile(Path.Combine("data", "sample_variationid_v5.json"), autoReload: false, OverrideBehaviour.LocalOnly) + }; + + using var client = new ConfigCatClient("localonly", options); + + var actualValue = await client.GetValueAsync("boolean", (bool?)null); + Assert.IsFalse(actualValue); + + Assert.AreEqual(1, logEvents.Count); + Assert.AreEqual(LogLevel.Info, logEvents[0].Level); + Assert.AreEqual(5000, logEvents[0].EventId); + Assert.IsNull(logEvents[0].Exception); + } + private static IConfigCatClient CreateClientWithMockedFetcher(string cacheKey, Mock loggerMock, Mock fetcherMock, @@ -2375,7 +2399,7 @@ private static IConfigCatClient CreateClientWithMockedFetcher(string cacheKey, var overrideDataSource = overrideDataSourceFactory?.Invoke(loggerWrapper); configService = configServiceFactory(fetcherMock.Object, cacheParams, loggerWrapper, hooks); - return new ConfigCatClient(configService, loggerMock.Object, evaluator, overrideDataSource?.Item1, overrideDataSource?.Item2, hooks); + return new ConfigCatClient(configService, loggerMock.Object, evaluator, overrideDataSource?.Item1, overrideDataSource?.Item2, hooks: hooks); } private static int ParseETagAsInt32(string? etag) diff --git a/src/ConfigCat.Client.Tests/Helpers/LoggingHelper.cs b/src/ConfigCat.Client.Tests/Helpers/LoggingHelper.cs index a2b5a4ba..f6ee492b 100644 --- a/src/ConfigCat.Client.Tests/Helpers/LoggingHelper.cs +++ b/src/ConfigCat.Client.Tests/Helpers/LoggingHelper.cs @@ -19,9 +19,9 @@ public LogEvent(LogLevel level, LogEventId eventId, ref FormattableLogMessage me internal static class LoggingHelper { - public static LoggerWrapper AsWrapper(this IConfigCatLogger logger, Hooks? hooks = null) + public static LoggerWrapper AsWrapper(this IConfigCatLogger logger, LogFilterCallback? filter = null, Hooks? hooks = null) { - return new LoggerWrapper(logger, hooks); + return new LoggerWrapper(logger, filter, hooks); } public static IConfigCatLogger CreateCapturingLogger(List logEvents, LogLevel logLevel = LogLevel.Info) diff --git a/src/ConfigCat.Client.Tests/LoggerTests.cs b/src/ConfigCat.Client.Tests/LoggerTests.cs index a0383b43..53ed44c8 100644 --- a/src/ConfigCat.Client.Tests/LoggerTests.cs +++ b/src/ConfigCat.Client.Tests/LoggerTests.cs @@ -1,4 +1,7 @@ +using System; +using System.Collections.Generic; using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; namespace ConfigCat.Client.Tests; @@ -112,4 +115,38 @@ public void LoggerBase_LoglevelIsOff_ShouldNotInvokeAnyLogMessage() Assert.AreEqual(0, l.LogMessageInvokeCount); } + + [TestMethod] + public void LogFilter_ExcludesLogEvents() + { + var logEvents = new List(); + var loggerMock = new Mock(); + loggerMock.SetupGet(m => m.LogLevel).Returns(LogLevel.Info); + + LogFilterCallback logFilter = (LogLevel level, LogEventId eventId, ref FormattableLogMessage message, Exception? exception) + => eventId.Id is not (1001 or 3001 or 5001); + + var logger = loggerMock.Object.AsWrapper(logFilter); + + logger.Log(LogLevel.Debug, 0, "debug"); + logger.Log(LogLevel.Info, 5000, "info"); + logger.Log(LogLevel.Warning, 3000, "warn"); + var ex1 = new Exception(); + logger.Log(LogLevel.Error, 1000, ex1, "error"); + logger.Log(LogLevel.Info, 5001, "info"); + logger.Log(LogLevel.Warning, 3001, "warn"); + var ex2 = new Exception(); + logger.Log(LogLevel.Error, 1001, ex2, "error"); + + loggerMock.Verify(m => m.Log(LogLevel.Debug, It.IsAny(), ref It.Ref.IsAny, It.IsAny()), Times.Never); + + loggerMock.Verify(m => m.Log(LogLevel.Info, It.IsAny(), ref It.Ref.IsAny, It.IsAny()), Times.Once); + loggerMock.Verify(m => m.Log(LogLevel.Info, 5000, ref It.Ref.IsAny, null), Times.Once); + + loggerMock.Verify(m => m.Log(LogLevel.Warning, It.IsAny(), ref It.Ref.IsAny, It.IsAny()), Times.Once); + loggerMock.Verify(m => m.Log(LogLevel.Warning, 3000, ref It.Ref.IsAny, null), Times.Once); + + loggerMock.Verify(m => m.Log(LogLevel.Error, It.IsAny(), ref It.Ref.IsAny, It.IsAny()), Times.Once); + loggerMock.Verify(m => m.Log(LogLevel.Error, 1000, ref It.Ref.IsAny, ex1), Times.Once); + } } diff --git a/src/ConfigCat.Client.Tests/UtilsTests.cs b/src/ConfigCat.Client.Tests/UtilsTests.cs index 47d3f07d..783ce7dc 100644 --- a/src/ConfigCat.Client.Tests/UtilsTests.cs +++ b/src/ConfigCat.Client.Tests/UtilsTests.cs @@ -157,4 +157,26 @@ public void ModelHelper_SetEnum_Works(SettingType enumValue) Assert.ThrowsException(() => ModelHelper.SetEnum(ref field, enumValue)); } } + + [DataTestMethod] + [DataRow(null, false, true, null)] + [DataRow("abc", false, true, "abc")] + [DataRow("abc", null, false, "abc")] + [DataRow("abc", new object?[0], false, "abc")] + [DataRow("abc{0}{1}{2}", new object?[] { 0.1, null, 23 }, false, "abc0.123")] + public void LazyString_Value_Works(string? valueOrFormat, object? args, bool expectedIsValueCreated, string? expectedValue) + { + var lazyString = args is false ? new LazyString(valueOrFormat) : new LazyString(valueOrFormat!, (object?[]?)args); + + Assert.AreEqual(expectedIsValueCreated, lazyString.IsValueCreated); + + var value = lazyString.Value; + Assert.AreEqual(expectedValue, value); + + Assert.IsTrue(lazyString.IsValueCreated); + + Assert.AreSame(value, lazyString.Value); + + Assert.AreSame(expectedValue is not null ? value : string.Empty, lazyString.ToString()); + } } diff --git a/src/ConfigCatClient/ConfigCatClient.cs b/src/ConfigCatClient/ConfigCatClient.cs index 5aea65ee..75abe291 100644 --- a/src/ConfigCatClient/ConfigCatClient.cs +++ b/src/ConfigCatClient/ConfigCatClient.cs @@ -85,7 +85,7 @@ internal ConfigCatClient(string sdkKey, ConfigCatClientOptions options) // hold a strong reference to the hooks object (see also SafeHooksWrapper). var hooksWrapper = new SafeHooksWrapper(this.hooks); - var logger = new LoggerWrapper(options.Logger ?? ConfigCatClientOptions.CreateDefaultLogger(), hooksWrapper); + var logger = new LoggerWrapper(options.Logger ?? ConfigCatClientOptions.CreateDefaultLogger(), options.LogFilter, hooksWrapper); var evaluator = new RolloutEvaluator(logger); this.evaluationServices = new EvaluationServices(evaluator, hooksWrapper, logger); @@ -125,13 +125,14 @@ internal ConfigCatClient(string sdkKey, ConfigCatClientOptions options) // For test purposes only internal ConfigCatClient(IConfigService configService, IConfigCatLogger logger, IRolloutEvaluator evaluator, - OverrideBehaviour? overrideBehaviour = null, IOverrideDataSource? overrideDataSource = null, Hooks? hooks = null) + OverrideBehaviour? overrideBehaviour = null, IOverrideDataSource? overrideDataSource = null, + LogFilterCallback? logFilter = null, Hooks? hooks = null) { this.hooks = hooks ?? NullHooks.Instance; this.hooks.SetSender(this); var hooksWrapper = new SafeHooksWrapper(this.hooks); - this.evaluationServices = new EvaluationServices(evaluator, hooksWrapper, new LoggerWrapper(logger, hooks)); + this.evaluationServices = new EvaluationServices(evaluator, hooksWrapper, new LoggerWrapper(logger, logFilter, hooks)); this.configService = configService; diff --git a/src/ConfigCatClient/ConfigService/ConfigServiceBase.cs b/src/ConfigCatClient/ConfigService/ConfigServiceBase.cs index 2746cbad..765234cd 100644 --- a/src/ConfigCatClient/ConfigService/ConfigServiceBase.cs +++ b/src/ConfigCatClient/ConfigService/ConfigServiceBase.cs @@ -96,7 +96,7 @@ public virtual RefreshResult RefreshConfig() else { var logMessage = this.Logger.ConfigServiceCannotInitiateHttpCalls(); - return RefreshResult.Failure(RefreshErrorCode.OfflineClient, logMessage.InvariantFormattedMessage); + return RefreshResult.Failure(RefreshErrorCode.OfflineClient, logMessage.ToLazyString()); } } @@ -133,7 +133,7 @@ public virtual async ValueTask RefreshConfigAsync(CancellationTok else { var logMessage = this.Logger.ConfigServiceCannotInitiateHttpCalls(); - return RefreshResult.Failure(RefreshErrorCode.OfflineClient, logMessage.InvariantFormattedMessage); + return RefreshResult.Failure(RefreshErrorCode.OfflineClient, logMessage.ToLazyString()); } } diff --git a/src/ConfigCatClient/Configuration/ConfigCatClientOptions.cs b/src/ConfigCatClient/Configuration/ConfigCatClientOptions.cs index 6911dc10..2b215903 100644 --- a/src/ConfigCatClient/Configuration/ConfigCatClientOptions.cs +++ b/src/ConfigCatClient/Configuration/ConfigCatClientOptions.cs @@ -17,6 +17,12 @@ public class ConfigCatClientOptions : IProvidesHooks private Hooks hooks = new(); + /// + /// An optional callback that can be used to filter log events beyond the minimum log level setting + /// ( and ). + /// + public LogFilterCallback? LogFilter { get; set; } + /// /// The logger implementation to use for performing logging. /// If not set, with will be used by default.
diff --git a/src/ConfigCatClient/Evaluation/EvaluationDetails.cs b/src/ConfigCatClient/Evaluation/EvaluationDetails.cs index cda4c0b3..c3486ffb 100644 --- a/src/ConfigCatClient/Evaluation/EvaluationDetails.cs +++ b/src/ConfigCatClient/Evaluation/EvaluationDetails.cs @@ -1,5 +1,6 @@ using System; using ConfigCat.Client.Evaluation; +using ConfigCat.Client.Utils; namespace ConfigCat.Client; @@ -28,13 +29,13 @@ internal static EvaluationDetails FromEvaluateResult(string key, } internal static EvaluationDetails FromDefaultValue(string key, TValue defaultValue, DateTime? fetchTime, User? user, - string errorMessage, Exception? errorException = null, EvaluationErrorCode errorCode = EvaluationErrorCode.UnexpectedError) + LazyString errorMessage, Exception? errorException = null, EvaluationErrorCode errorCode = EvaluationErrorCode.UnexpectedError) { var instance = new EvaluationDetails(key, defaultValue) { User = user, IsDefaultValue = true, - ErrorMessage = errorMessage, + errorMessage = errorMessage, ErrorException = errorException, ErrorCode = errorCode, }; @@ -47,6 +48,8 @@ internal static EvaluationDetails FromDefaultValue(string key, T return instance; } + private LazyString errorMessage; + private protected EvaluationDetails(string key) { Key = key; @@ -93,7 +96,11 @@ private protected EvaluationDetails(string key) /// /// Error message in case evaluation failed. /// - public string? ErrorMessage { get; set; } + public string? ErrorMessage + { + get => this.errorMessage.Value; + set => this.errorMessage = value; + } /// /// The object related to the error in case evaluation failed (if any). diff --git a/src/ConfigCatClient/Evaluation/EvaluationHelper.cs b/src/ConfigCatClient/Evaluation/EvaluationHelper.cs index 1f540fac..09d121ef 100644 --- a/src/ConfigCatClient/Evaluation/EvaluationHelper.cs +++ b/src/ConfigCatClient/Evaluation/EvaluationHelper.cs @@ -17,15 +17,15 @@ public static EvaluationDetails Evaluate(this IRolloutEvaluator evaluator, { logMessage = logger.ConfigJsonIsNotPresent(key, nameof(defaultValue), defaultValue); return EvaluationDetails.FromDefaultValue(key, defaultValue, fetchTime: remoteConfig?.TimeStamp, user, - logMessage.InvariantFormattedMessage, errorCode: EvaluationErrorCode.ConfigJsonNotAvailable); + logMessage.ToLazyString(), errorCode: EvaluationErrorCode.ConfigJsonNotAvailable); } if (!settings.TryGetValue(key, out var setting)) { - var availableKeys = new StringListFormatter(settings.Keys).ToString(); + var availableKeys = new StringListFormatter(settings.Keys); logMessage = logger.SettingEvaluationFailedDueToMissingKey(key, nameof(defaultValue), defaultValue, availableKeys); return EvaluationDetails.FromDefaultValue(key, defaultValue, fetchTime: remoteConfig?.TimeStamp, user, - logMessage.InvariantFormattedMessage, errorCode: EvaluationErrorCode.SettingKeyMissing); + logMessage.ToLazyString(), errorCode: EvaluationErrorCode.SettingKeyMissing); } var evaluateContext = new EvaluateContext(key, setting, user, settings); diff --git a/src/ConfigCatClient/FetchResult.cs b/src/ConfigCatClient/FetchResult.cs index deeaeda0..bc8ce429 100644 --- a/src/ConfigCatClient/FetchResult.cs +++ b/src/ConfigCatClient/FetchResult.cs @@ -1,5 +1,8 @@ using System; +using System.Collections.Generic; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; +using ConfigCat.Client.Utils; namespace ConfigCat.Client; @@ -17,12 +20,13 @@ public static FetchResult NotModified(ProjectConfig config) return new FetchResult(config, RefreshErrorCode.None, NotModifiedToken); } - public static FetchResult Failure(ProjectConfig config, RefreshErrorCode errorCode, string errorMessage, Exception? errorException = null) + public static FetchResult Failure(ProjectConfig config, RefreshErrorCode errorCode, LazyString errorMessage, Exception? errorException = null) { - return new FetchResult(config, errorCode, errorMessage, errorException); + Debug.Assert(!EqualityComparer.Default.Equals(errorMessage, default)); + return new FetchResult(config, errorCode, errorMessage.IsValueCreated ? errorMessage.Value : (object)errorMessage, errorException); } - private readonly object? errorMessageOrToken; + private readonly object? errorMessageOrToken; // either null or a string or a boxed LazyString or NotModifiedToken private FetchResult(ProjectConfig config, RefreshErrorCode errorCode, object? errorMessageOrToken, Exception? errorException = null) { @@ -35,10 +39,10 @@ private FetchResult(ProjectConfig config, RefreshErrorCode errorCode, object? er public bool IsSuccess => this.errorMessageOrToken is null; public bool IsNotModified => ReferenceEquals(this.errorMessageOrToken, NotModifiedToken); [MemberNotNullWhen(true, nameof(ErrorMessage))] - public bool IsFailure => this.errorMessageOrToken is string; + public bool IsFailure => !IsSuccess && !IsNotModified; public ProjectConfig Config { get; } public RefreshErrorCode ErrorCode { get; } - public string? ErrorMessage => this.errorMessageOrToken as string; + public object? ErrorMessage => !IsNotModified ? this.errorMessageOrToken : null; public Exception? ErrorException { get; } } diff --git a/src/ConfigCatClient/FormattableString.cs b/src/ConfigCatClient/FormattableString.cs index e89839c2..77b80c68 100644 --- a/src/ConfigCatClient/FormattableString.cs +++ b/src/ConfigCatClient/FormattableString.cs @@ -1,102 +1,32 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -// Source: https://github.com/dotnet/runtime/blob/v6.0.13/src/libraries/System.Private.CoreLib/src/System/FormattableString.cs - -#if NET45 +// Based on: https://github.com/dotnet/runtime/blob/v6.0.13/src/libraries/System.Private.CoreLib/src/System/FormattableString.cs +// This is a modified version of the built-in type that is used only internally in the SDK. #pragma warning disable IDE0161 // Convert to file-scoped namespace +#pragma warning disable CS0436 // Type conflicts with imported type + +global using ValueFormattableString = System.FormattableString; namespace System { - /// - /// A composite format string along with the arguments to be formatted. An instance of this - /// type may result from the use of the C# or VB language primitive "interpolated string". - /// - internal abstract class FormattableString : IFormattable + internal readonly struct FormattableString : IFormattable { - /// - /// The composite format string. - /// - public abstract string Format { get; } - - /// - /// Returns an object array that contains zero or more objects to format. Clients should not - /// mutate the contents of the array. - /// - public abstract object?[] GetArguments(); - - /// - /// The number of arguments to be formatted. - /// - public abstract int ArgumentCount { get; } - - /// - /// Returns one argument to be formatted from argument position . - /// - public abstract object? GetArgument(int index); - - /// - /// Format to a string using the given culture. - /// - public abstract string ToString(IFormatProvider? formatProvider); - - string IFormattable.ToString(string? ignored, IFormatProvider? formatProvider) - { - return ToString(formatProvider); - } + private readonly string format; + private readonly object?[] arguments; - /// - /// Format the given object in the invariant culture. This static method may be - /// imported in C# by - /// - /// using static System.FormattableString; - /// . - /// Within the scope - /// of that import directive an interpolated string may be formatted in the - /// invariant culture by writing, for example, - /// - /// Invariant($"{{ lat = {latitude}; lon = {longitude} }}") - /// - /// - public static string Invariant(FormattableString formattable) + internal FormattableString(string format, object?[] arguments) { - if (formattable == null) - { - throw new ArgumentNullException(nameof(formattable)); - } - - return formattable.ToString(Globalization.CultureInfo.InvariantCulture); + this.format = format; + this.arguments = arguments; } - /// - /// Format the given object in the current culture. This static method may be - /// imported in C# by - /// - /// using static System.FormattableString; - /// . - /// Within the scope - /// of that import directive an interpolated string may be formatted in the - /// current culture by writing, for example, - /// - /// CurrentCulture($"{{ lat = {latitude}; lon = {longitude} }}") - /// - /// - public static string CurrentCulture(FormattableString formattable) - { - if (formattable == null) - { - throw new ArgumentNullException(nameof(formattable)); - } - - return formattable.ToString(Globalization.CultureInfo.CurrentCulture); - } + public string Format => this.format ?? string.Empty; + public object?[] GetArguments() { return this.arguments; } - public override string ToString() - { - return ToString(Globalization.CultureInfo.CurrentCulture); - } + public string ToString(string? format, IFormatProvider? formatProvider) { return ToString(formatProvider); } + public string ToString(IFormatProvider? formatProvider) { return string.Format(formatProvider, Format, this.arguments); } + public override string ToString() { return ToString(Globalization.CultureInfo.CurrentCulture); } } } - -#endif diff --git a/src/ConfigCatClient/FormattableStringFactory.cs b/src/ConfigCatClient/FormattableStringFactory.cs index 85ed8156..a21ea76e 100644 --- a/src/ConfigCatClient/FormattableStringFactory.cs +++ b/src/ConfigCatClient/FormattableStringFactory.cs @@ -1,13 +1,12 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -// Source: https://github.com/dotnet/runtime/blob/v6.0.13/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/FormattableStringFactory.cs - -#if NET45 +// Based on: https://github.com/dotnet/runtime/blob/v6.0.13/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/FormattableStringFactory.cs +// This is a modified version of the built-in type that is only used only internally in the SDK. +// It creates value type instances instead of reference type instances to avoid unnecessary heap allocations. #pragma warning disable IDE0161 // Convert to file-scoped namespace -#pragma warning disable IDE1006 // Naming Styles -#pragma warning disable IDE0009 // Member access should be qualified. +#pragma warning disable CS0436 // Type conflicts with imported type namespace System.Runtime.CompilerServices { @@ -20,7 +19,7 @@ internal static class FormattableStringFactory /// Create a from a composite format string and object /// array containing zero or more objects to format. /// - public static FormattableString Create(string format, params object?[] arguments) + public static ValueFormattableString Create(string format, params object?[] arguments) { if (format == null) { @@ -32,27 +31,7 @@ public static FormattableString Create(string format, params object?[] arguments throw new ArgumentNullException(nameof(arguments)); } - return new ConcreteFormattableString(format, arguments); - } - - private sealed class ConcreteFormattableString : FormattableString - { - private readonly string _format; - private readonly object?[] _arguments; - - internal ConcreteFormattableString(string format, object?[] arguments) - { - _format = format; - _arguments = arguments; - } - - public override string Format => _format; - public override object?[] GetArguments() { return _arguments; } - public override int ArgumentCount => _arguments.Length; - public override object? GetArgument(int index) { return _arguments[index]; } - public override string ToString(IFormatProvider? formatProvider) { return string.Format(formatProvider, _format, _arguments); } + return new ValueFormattableString(format, arguments); } } } - -#endif diff --git a/src/ConfigCatClient/Hooks/Hooks.cs b/src/ConfigCatClient/Hooks/Hooks.cs index 75b55fcc..a63f2ece 100644 --- a/src/ConfigCatClient/Hooks/Hooks.cs +++ b/src/ConfigCatClient/Hooks/Hooks.cs @@ -45,8 +45,8 @@ public void RaiseConfigFetched(RefreshResult result, bool isInitiatedByUser) public void RaiseConfigChanged(IConfig newConfig) => this.events.RaiseConfigChanged(this.client, newConfig); - public void RaiseError(string message, Exception? exception) - => this.events.RaiseError(this.client, message, exception); + public void RaiseError(ref FormattableLogMessage message, Exception? exception) + => this.events.RaiseError(this.client, ref message, exception); public event EventHandler? ClientReady { @@ -84,7 +84,7 @@ public virtual void RaiseClientReady(IConfigCatClient? client, ClientCacheState public virtual void RaiseFlagEvaluated(IConfigCatClient? client, EvaluationDetails evaluationDetails) { /* intentional no-op */ } public virtual void RaiseConfigFetched(IConfigCatClient? client, RefreshResult result, bool isInitiatedByUser) { /* intentional no-op */ } public virtual void RaiseConfigChanged(IConfigCatClient? client, IConfig newConfig) { /* intentional no-op */ } - public virtual void RaiseError(IConfigCatClient? client, string message, Exception? exception) { /* intentional no-op */ } + public virtual void RaiseError(IConfigCatClient? client, ref FormattableLogMessage message, Exception? exception) { /* intentional no-op */ } public virtual event EventHandler? ClientReady { add { /* intentional no-op */ } remove { /* intentional no-op */ } } public virtual event EventHandler? FlagEvaluated { add { /* intentional no-op */ } remove { /* intentional no-op */ } } @@ -115,9 +115,9 @@ public override void RaiseConfigChanged(IConfigCatClient? client, IConfig newCon ConfigChanged?.Invoke(client, new ConfigChangedEventArgs(newConfig)); } - public override void RaiseError(IConfigCatClient? client, string message, Exception? exception) + public override void RaiseError(IConfigCatClient? client, ref FormattableLogMessage message, Exception? exception) { - Error?.Invoke(client, new ConfigCatClientErrorEventArgs(message, exception)); + Error?.Invoke(client, new ConfigCatClientErrorEventArgs(message.InvariantFormattedMessage, exception)); } public override event EventHandler? ClientReady; diff --git a/src/ConfigCatClient/Hooks/SafeHooksWrapper.cs b/src/ConfigCatClient/Hooks/SafeHooksWrapper.cs index e5bc275a..c85aa69b 100644 --- a/src/ConfigCatClient/Hooks/SafeHooksWrapper.cs +++ b/src/ConfigCatClient/Hooks/SafeHooksWrapper.cs @@ -37,8 +37,8 @@ public void RaiseConfigChanged(IConfig newConfig) => Hooks.RaiseConfigChanged(newConfig); [MethodImpl(MethodImplOptions.NoInlining)] - public void RaiseError(string message, Exception? exception) - => Hooks.RaiseError(message, exception); + public void RaiseError(ref FormattableLogMessage message, Exception? exception) + => Hooks.RaiseError(ref message, exception); public static implicit operator SafeHooksWrapper(Hooks? hooks) => hooks is not null ? new SafeHooksWrapper(hooks) : default; } diff --git a/src/ConfigCatClient/HttpConfigFetcher.cs b/src/ConfigCatClient/HttpConfigFetcher.cs index ede9daa8..9efcbb39 100644 --- a/src/ConfigCatClient/HttpConfigFetcher.cs +++ b/src/ConfigCatClient/HttpConfigFetcher.cs @@ -90,7 +90,7 @@ private async ValueTask FetchInternalAsync(ProjectConfig lastConfig { var exception = responseWithBody.Exception; logMessage = this.logger.FetchReceived200WithInvalidBody(exception); - return FetchResult.Failure(lastConfig, RefreshErrorCode.InvalidHttpResponseContent, logMessage.InvariantFormattedMessage, exception); + return FetchResult.Failure(lastConfig, RefreshErrorCode.InvalidHttpResponseContent, logMessage.ToLazyString(), exception); } return FetchResult.Success(new ProjectConfig @@ -105,7 +105,7 @@ private async ValueTask FetchInternalAsync(ProjectConfig lastConfig if (lastConfig.IsEmpty) { logMessage = this.logger.FetchReceived304WhenLocalCacheIsEmpty((int)response.StatusCode, response.ReasonPhrase); - return FetchResult.Failure(lastConfig, RefreshErrorCode.InvalidHttpResponseWhenLocalCacheIsEmpty, logMessage.InvariantFormattedMessage); + return FetchResult.Failure(lastConfig, RefreshErrorCode.InvalidHttpResponseWhenLocalCacheIsEmpty, logMessage.ToLazyString()); } return FetchResult.NotModified(lastConfig.With(ProjectConfig.GenerateTimeStamp())); @@ -115,13 +115,13 @@ private async ValueTask FetchInternalAsync(ProjectConfig lastConfig logMessage = this.logger.FetchFailedDueToInvalidSdkKey(); // We update the timestamp for extra protection against flooding. - return FetchResult.Failure(lastConfig.With(ProjectConfig.GenerateTimeStamp()), RefreshErrorCode.InvalidSdkKey, logMessage.InvariantFormattedMessage); + return FetchResult.Failure(lastConfig.With(ProjectConfig.GenerateTimeStamp()), RefreshErrorCode.InvalidSdkKey, logMessage.ToLazyString()); default: logMessage = this.logger.FetchFailedDueToUnexpectedHttpResponse((int)response.StatusCode, response.ReasonPhrase); ReInitializeHttpClient(); - return FetchResult.Failure(lastConfig, RefreshErrorCode.UnexpectedHttpResponse, logMessage.InvariantFormattedMessage); + return FetchResult.Failure(lastConfig, RefreshErrorCode.UnexpectedHttpResponse, logMessage.ToLazyString()); } } catch (OperationCanceledException) when (this.cancellationTokenSource.IsCancellationRequested) @@ -153,7 +153,7 @@ private async ValueTask FetchInternalAsync(ProjectConfig lastConfig } ReInitializeHttpClient(); - return FetchResult.Failure(lastConfig, errorCode, logMessage.InvariantFormattedMessage, errorException); + return FetchResult.Failure(lastConfig, errorCode, logMessage.ToLazyString(), errorException); } private async ValueTask FetchRequestAsync(string? httpETag, Uri requestUri, sbyte maxExecutionCount = 3) diff --git a/src/ConfigCatClient/Logging/FormattableLogMessage.cs b/src/ConfigCatClient/Logging/FormattableLogMessage.cs index c84b1b64..b5f8ac74 100644 --- a/src/ConfigCatClient/Logging/FormattableLogMessage.cs +++ b/src/ConfigCatClient/Logging/FormattableLogMessage.cs @@ -14,7 +14,7 @@ private static string ToFormatString(string message) return message.Replace("{", "{{").Replace("}", "}}"); } - internal static FormattableLogMessage FromInterpolated(FormattableString message, string[] argNames) + internal static FormattableLogMessage FromInterpolated(ValueFormattableString message, string[] argNames) { return new FormattableLogMessage(message.Format, argNames ?? ArrayUtils.EmptyArray(), @@ -72,6 +72,13 @@ public FormattableLogMessage(string format, string[] argNames, object?[] argValu /// public string InvariantFormattedMessage => this.invariantFormattedMessage ??= ToString(CultureInfo.InvariantCulture); + internal LazyString ToLazyString() + { + return this.invariantFormattedMessage is { } invariantFormattedMessage + ? invariantFormattedMessage + : new LazyString(this.format ?? string.Empty, this.argValues); + } + /// /// Returns the log message formatted using . /// diff --git a/src/ConfigCatClient/Logging/IConfigCatLogger.cs b/src/ConfigCatClient/Logging/IConfigCatLogger.cs index 33161f8a..e3d10675 100644 --- a/src/ConfigCatClient/Logging/IConfigCatLogger.cs +++ b/src/ConfigCatClient/Logging/IConfigCatLogger.cs @@ -2,6 +2,16 @@ namespace ConfigCat.Client; +/// +/// Represents a method that is called by the SDK to decide whether a log event should be logged. +/// +/// Event severity level. +/// Event identifier. +/// Message. +/// The object related to the message (if any). +/// when the event should be logged, when it should be skipped. +public delegate bool LogFilterCallback(LogLevel level, LogEventId eventId, ref FormattableLogMessage message, Exception? exception); + /// /// Defines the interface used by the ConfigCat SDK to perform logging. /// diff --git a/src/ConfigCatClient/Logging/LogMessages.cs b/src/ConfigCatClient/Logging/LogMessages.cs index 56038150..433e40fb 100644 --- a/src/ConfigCatClient/Logging/LogMessages.cs +++ b/src/ConfigCatClient/Logging/LogMessages.cs @@ -1,5 +1,6 @@ using System; using ConfigCat.Client.ConfigService; +using ConfigCat.Client.Utils; namespace ConfigCat.Client; @@ -17,7 +18,7 @@ public static FormattableLogMessage ConfigJsonIsNotPresent(this LoggerWrapper lo $"Config JSON is not present when evaluating setting '{key}'. Returning the `{defaultParamName}` parameter that you specified in your application: '{defaultParamValue}'.", "KEY", "DEFAULT_PARAM_NAME", "DEFAULT_PARAM_VALUE"); - public static FormattableLogMessage SettingEvaluationFailedDueToMissingKey(this LoggerWrapper logger, string key, string defaultParamName, object? defaultParamValue, string availableKeys) => logger.LogInterpolated( + public static FormattableLogMessage SettingEvaluationFailedDueToMissingKey(this LoggerWrapper logger, string key, string defaultParamName, object? defaultParamValue, StringListFormatter availableKeys) => logger.LogInterpolated( LogLevel.Error, 1001, $"Failed to evaluate setting '{key}' (the key was not found in config JSON). Returning the `{defaultParamName}` parameter that you specified in your application: '{defaultParamValue}'. Available keys: [{availableKeys}].", "KEY", "DEFAULT_PARAM_NAME", "DEFAULT_PARAM_VALUE", "AVAILABLE_KEYS"); diff --git a/src/ConfigCatClient/Logging/LoggerExtensions.cs b/src/ConfigCatClient/Logging/LoggerExtensions.cs index 750c34e2..e4386ae7 100644 --- a/src/ConfigCatClient/Logging/LoggerExtensions.cs +++ b/src/ConfigCatClient/Logging/LoggerExtensions.cs @@ -46,12 +46,12 @@ public static FormattableLogMessage LogFormatted(this LoggerWrapper logger, LogL // `logger.Log(LogLevel.Error, 1234, "A message with {PARAM}", paramValue)` // but then we'd need to manually parse the format string, which is better to avoid to keep our solution simple.) - public static FormattableLogMessage LogInterpolated(this LoggerWrapper logger, LogLevel level, LogEventId eventId, FormattableString message, params string[] argNames) + public static FormattableLogMessage LogInterpolated(this LoggerWrapper logger, LogLevel level, LogEventId eventId, ValueFormattableString message, params string[] argNames) { return LogInterpolated(logger, level, eventId, exception: null, message, argNames); } - public static FormattableLogMessage LogInterpolated(this LoggerWrapper logger, LogLevel level, LogEventId eventId, Exception? exception, FormattableString message, params string[] argNames) + public static FormattableLogMessage LogInterpolated(this LoggerWrapper logger, LogLevel level, LogEventId eventId, Exception? exception, ValueFormattableString message, params string[] argNames) { if (logger is null) { diff --git a/src/ConfigCatClient/Logging/LoggerWrapper.cs b/src/ConfigCatClient/Logging/LoggerWrapper.cs index e58ebce3..8c6c8bec 100644 --- a/src/ConfigCatClient/Logging/LoggerWrapper.cs +++ b/src/ConfigCatClient/Logging/LoggerWrapper.cs @@ -1,10 +1,12 @@ using System; +using ConfigCat.Client.Configuration; namespace ConfigCat.Client; internal sealed class LoggerWrapper : IConfigCatLogger { private readonly IConfigCatLogger logger; + private readonly LogFilterCallback? filter; private readonly SafeHooksWrapper hooks; public LogLevel LogLevel @@ -13,9 +15,10 @@ public LogLevel LogLevel set => this.logger.LogLevel = value; } - internal LoggerWrapper(IConfigCatLogger logger, SafeHooksWrapper hooks = default) + internal LoggerWrapper(IConfigCatLogger logger, LogFilterCallback? filter = null, SafeHooksWrapper hooks = default) { this.logger = logger; + this.filter = filter; this.hooks = hooks; } @@ -27,14 +30,14 @@ public bool IsEnabled(LogLevel level) /// public void Log(LogLevel level, LogEventId eventId, ref FormattableLogMessage message, Exception? exception = null) { - if (IsEnabled(level)) + if (IsEnabled(level) && (this.filter is null || this.filter(level, eventId, ref message, exception))) { this.logger.Log(level, eventId, ref message, exception); } if (level == LogLevel.Error) { - this.hooks.RaiseError(message.InvariantFormattedMessage, exception); + this.hooks.RaiseError(ref message, exception); } } } diff --git a/src/ConfigCatClient/RefreshResult.cs b/src/ConfigCatClient/RefreshResult.cs index 840574ca..fc039dfc 100644 --- a/src/ConfigCatClient/RefreshResult.cs +++ b/src/ConfigCatClient/RefreshResult.cs @@ -1,5 +1,6 @@ using System; using System.Diagnostics.CodeAnalysis; +using ConfigCat.Client.Utils; namespace ConfigCat.Client; @@ -29,17 +30,24 @@ public static RefreshResult Failure(RefreshErrorCode errorCode, string errorMess errorException); } + internal static RefreshResult Failure(RefreshErrorCode errorCode, LazyString errorMessage, Exception? errorException = null) + { + return new RefreshResult(errorCode, errorMessage, errorException); + } + internal static RefreshResult From(in FetchResult fetchResult) { return !fetchResult.IsFailure ? Success() - : Failure(fetchResult.ErrorCode, fetchResult.ErrorMessage, fetchResult.ErrorException); + : new RefreshResult(fetchResult.ErrorCode, fetchResult.ErrorMessage, fetchResult.ErrorException); } - private RefreshResult(RefreshErrorCode errorCode, string? errorMessage, Exception? errorException) + private readonly object? errorMessage; // either null or a string or a boxed LazyString + + private RefreshResult(RefreshErrorCode errorCode, object? errorMessage, Exception? errorException) { ErrorCode = errorCode; - ErrorMessage = errorMessage; + this.errorMessage = errorMessage; ErrorException = errorException; } @@ -57,7 +65,7 @@ private RefreshResult(RefreshErrorCode errorCode, string? errorMessage, Exceptio /// /// Error message in case the operation failed, otherwise . /// - public string? ErrorMessage { get; } + public string? ErrorMessage => this.errorMessage?.ToString(); /// /// The object related to the error in case the operation failed (if any). diff --git a/src/ConfigCatClient/Utils/IndentedTextBuilder.cs b/src/ConfigCatClient/Utils/IndentedTextBuilder.cs index b445b87f..370c43d5 100644 --- a/src/ConfigCatClient/Utils/IndentedTextBuilder.cs +++ b/src/ConfigCatClient/Utils/IndentedTextBuilder.cs @@ -48,7 +48,7 @@ public IndentedTextBuilder Append(object value) } #if !NET6_0_OR_GREATER - public IndentedTextBuilder Append(FormattableString value) + public IndentedTextBuilder Append(ValueFormattableString value) { this.stringBuilder.AppendFormat(CultureInfo.InvariantCulture, value.Format, value.GetArguments()); return this; diff --git a/src/ConfigCatClient/Utils/LazyString.cs b/src/ConfigCatClient/Utils/LazyString.cs new file mode 100644 index 00000000..8b2f6236 --- /dev/null +++ b/src/ConfigCatClient/Utils/LazyString.cs @@ -0,0 +1,56 @@ +using System.Diagnostics; +using System.Globalization; + +namespace ConfigCat.Client.Utils; + +/// +/// Defers string formatting until the formatted value is actually needed. +/// +/// +/// It roughly achieves what new Lazy<string>(() => string.Format(CultureInfo.InvariantCulture, format, args), isThreadSafe: false) does +/// but without extra heap memory allocations. +/// +internal struct LazyString +{ + private readonly string? format; + private object? argsOrValue; + + public LazyString(string? value) + { + this.format = null; + this.argsOrValue = value; + } + + public LazyString(string format, params object?[]? args) + { + this.format = format; + this.argsOrValue = args ?? ArrayUtils.EmptyArray(); + } + + [DebuggerBrowsable(DebuggerBrowsableState.Never)] + public string? Value + { + get + { + var argsOrValue = this.argsOrValue; + if (argsOrValue is null) + { + return null; + } + + if (argsOrValue is string value) + { + return value; + } + + this.argsOrValue = value = string.Format(CultureInfo.InvariantCulture, this.format!, (object?[])argsOrValue); + return value; + } + } + + public bool IsValueCreated => this.argsOrValue is null or string; + + public override string ToString() => Value ?? string.Empty; + + public static implicit operator LazyString(string? value) => new LazyString(value); +}