From b8c91f255f92e96e26eb32688844aa9ef1ca308e Mon Sep 17 00:00:00 2001 From: Martin Taillefer Date: Wed, 27 Dec 2023 05:56:58 -0800 Subject: [PATCH 1/4] Address some trivial experimental items (#4837) Fixes #4819 Co-authored-by: Martin Taillefer --- .../IAsyncLocalContext.cs | 3 -- .../HmacRedactor.cs | 18 ++++++++--- .../HmacRedactorOptions.cs | 4 +-- ...osoft.Extensions.Compliance.Redaction.json | 32 +++++++++++++++---- .../RedactionExtensions.cs | 3 -- .../Logging/FakeLogRecord.cs | 3 -- ...rosoft.Extensions.Diagnostics.Testing.json | 6 +++- 7 files changed, 45 insertions(+), 24 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.AsyncState/IAsyncLocalContext.cs b/src/Libraries/Microsoft.Extensions.AsyncState/IAsyncLocalContext.cs index 8934fef1b9d..746909d1b8c 100644 --- a/src/Libraries/Microsoft.Extensions.AsyncState/IAsyncLocalContext.cs +++ b/src/Libraries/Microsoft.Extensions.AsyncState/IAsyncLocalContext.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.ComponentModel; -using System.Diagnostics.CodeAnalysis; -using Microsoft.Shared.DiagnosticIds; namespace Microsoft.Extensions.AsyncState; @@ -12,7 +10,6 @@ namespace Microsoft.Extensions.AsyncState; /// /// The type of the asynchronous state. /// This type is intended for internal use. Use instead. -[Experimental(diagnosticId: DiagnosticIds.Experiments.AsyncState, UrlFormat = DiagnosticIds.UrlFormat)] [EditorBrowsable(EditorBrowsableState.Never)] #pragma warning disable S4023 // Interfaces should not be empty public interface IAsyncLocalContext : IAsyncContext diff --git a/src/Libraries/Microsoft.Extensions.Compliance.Redaction/HmacRedactor.cs b/src/Libraries/Microsoft.Extensions.Compliance.Redaction/HmacRedactor.cs index 0f76e3a9c00..47352c849aa 100644 --- a/src/Libraries/Microsoft.Extensions.Compliance.Redaction/HmacRedactor.cs +++ b/src/Libraries/Microsoft.Extensions.Compliance.Redaction/HmacRedactor.cs @@ -17,7 +17,10 @@ namespace Microsoft.Extensions.Compliance.Redaction; -internal sealed class HmacRedactor : Redactor +/// +/// A redactor using HMACSHA256 to encode data being redacted. +/// +public sealed class HmacRedactor : Redactor { #if NET6_0_OR_GREATER private const int SHA256HashSizeInBytes = 32; @@ -33,18 +36,23 @@ internal sealed class HmacRedactor : Redactor private readonly byte[] _hashKey; private readonly string _keyId; + /// + /// Initializes a new instance of the class. + /// + /// Controls the behavior of the redactor. public HmacRedactor(IOptions options) { - var value = Throw.IfMemberNull(options, options.Value); + var value = Throw.IfMemberNull(options, options?.Value); _hashKey = Convert.FromBase64String(value.Key); _keyId = value.KeyId.HasValue ? value.KeyId.Value.ToInvariantString() + ':' : string.Empty; _redactedLength = Base64HashLength + _keyId.Length; } - public override int GetRedactedLength(ReadOnlySpan source) + /// + public override int GetRedactedLength(ReadOnlySpan input) { - if (source.IsEmpty) + if (input.IsEmpty) { return 0; } @@ -53,6 +61,7 @@ public override int GetRedactedLength(ReadOnlySpan source) } #if NET6_0_OR_GREATER + /// public override int Redact(ReadOnlySpan source, Span destination) { var length = GetRedactedLength(source); @@ -82,6 +91,7 @@ private static int CreateSha256Hash(ReadOnlySpan source, Span destin #else + /// public override int Redact(ReadOnlySpan source, Span destination) { const int RemainingBytesToPadForBase64Hash = BytesOfHashWeUse % 3; diff --git a/src/Libraries/Microsoft.Extensions.Compliance.Redaction/HmacRedactorOptions.cs b/src/Libraries/Microsoft.Extensions.Compliance.Redaction/HmacRedactorOptions.cs index a66b6aefd47..1a110339ad1 100644 --- a/src/Libraries/Microsoft.Extensions.Compliance.Redaction/HmacRedactorOptions.cs +++ b/src/Libraries/Microsoft.Extensions.Compliance.Redaction/HmacRedactorOptions.cs @@ -2,14 +2,12 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics.CodeAnalysis; -using Microsoft.Shared.DiagnosticIds; namespace Microsoft.Extensions.Compliance.Redaction; /// -/// A redactor using HMACSHA256 to encode data being redacted. +/// Options to control the . /// -[Experimental(diagnosticId: DiagnosticIds.Experiments.Compliance, UrlFormat = DiagnosticIds.UrlFormat)] public class HmacRedactorOptions { /// diff --git a/src/Libraries/Microsoft.Extensions.Compliance.Redaction/Microsoft.Extensions.Compliance.Redaction.json b/src/Libraries/Microsoft.Extensions.Compliance.Redaction/Microsoft.Extensions.Compliance.Redaction.json index 88f44e26362..7c646a7e9b1 100644 --- a/src/Libraries/Microsoft.Extensions.Compliance.Redaction/Microsoft.Extensions.Compliance.Redaction.json +++ b/src/Libraries/Microsoft.Extensions.Compliance.Redaction/Microsoft.Extensions.Compliance.Redaction.json @@ -1,5 +1,5 @@ { - "Name": "Microsoft.Extensions.Compliance.Redaction, Version=8.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35", + "Name": "Microsoft.Extensions.Compliance.Redaction, Version=8.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35", "Types": [ { "Type": "sealed class Microsoft.Extensions.Compliance.Redaction.ErasingRedactor : Microsoft.Extensions.Compliance.Redaction.Redactor", @@ -25,23 +25,41 @@ } ] }, + { + "Type": "sealed class Microsoft.Extensions.Compliance.Redaction.HmacRedactor : Microsoft.Extensions.Compliance.Redaction.Redactor", + "Stage": "Stable", + "Methods": [ + { + "Member": "Microsoft.Extensions.Compliance.Redaction.HmacRedactor.HmacRedactor(Microsoft.Extensions.Options.IOptions options);", + "Stage": "Stable" + }, + { + "Member": "override int Microsoft.Extensions.Compliance.Redaction.HmacRedactor.GetRedactedLength(System.ReadOnlySpan input);", + "Stage": "Stable" + }, + { + "Member": "override int Microsoft.Extensions.Compliance.Redaction.HmacRedactor.Redact(System.ReadOnlySpan source, System.Span destination);", + "Stage": "Stable" + } + ] + }, { "Type": "class Microsoft.Extensions.Compliance.Redaction.HmacRedactorOptions", - "Stage": "Experimental", + "Stage": "Stable", "Methods": [ { "Member": "Microsoft.Extensions.Compliance.Redaction.HmacRedactorOptions.HmacRedactorOptions();", - "Stage": "Experimental" + "Stage": "Stable" } ], "Properties": [ { "Member": "string Microsoft.Extensions.Compliance.Redaction.HmacRedactorOptions.Key { get; set; }", - "Stage": "Experimental" + "Stage": "Stable" }, { "Member": "int? Microsoft.Extensions.Compliance.Redaction.HmacRedactorOptions.KeyId { get; set; }", - "Stage": "Experimental" + "Stage": "Stable" } ] }, @@ -51,11 +69,11 @@ "Methods": [ { "Member": "static Microsoft.Extensions.Compliance.Redaction.IRedactionBuilder Microsoft.Extensions.Compliance.Redaction.RedactionExtensions.SetHmacRedactor(this Microsoft.Extensions.Compliance.Redaction.IRedactionBuilder builder, System.Action configure, params Microsoft.Extensions.Compliance.Classification.DataClassificationSet[] classifications);", - "Stage": "Experimental" + "Stage": "Stable" }, { "Member": "static Microsoft.Extensions.Compliance.Redaction.IRedactionBuilder Microsoft.Extensions.Compliance.Redaction.RedactionExtensions.SetHmacRedactor(this Microsoft.Extensions.Compliance.Redaction.IRedactionBuilder builder, Microsoft.Extensions.Configuration.IConfigurationSection section, params Microsoft.Extensions.Compliance.Classification.DataClassificationSet[] classifications);", - "Stage": "Experimental" + "Stage": "Stable" } ] }, diff --git a/src/Libraries/Microsoft.Extensions.Compliance.Redaction/RedactionExtensions.cs b/src/Libraries/Microsoft.Extensions.Compliance.Redaction/RedactionExtensions.cs index e3bee7e2605..191d8af6f55 100644 --- a/src/Libraries/Microsoft.Extensions.Compliance.Redaction/RedactionExtensions.cs +++ b/src/Libraries/Microsoft.Extensions.Compliance.Redaction/RedactionExtensions.cs @@ -6,7 +6,6 @@ using Microsoft.Extensions.Compliance.Classification; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Shared.DiagnosticIds; using Microsoft.Shared.Diagnostics; namespace Microsoft.Extensions.Compliance.Redaction; @@ -24,7 +23,6 @@ public static class RedactionExtensions /// The data classifications for which the redactor type should be used. /// The value of . /// , , or is . - [Experimental(diagnosticId: DiagnosticIds.Experiments.Compliance, UrlFormat = DiagnosticIds.UrlFormat)] public static IRedactionBuilder SetHmacRedactor(this IRedactionBuilder builder, Action configure, params DataClassificationSet[] classifications) { _ = Throw.IfNull(builder); @@ -47,7 +45,6 @@ public static IRedactionBuilder SetHmacRedactor(this IRedactionBuilder builder, /// The data classifications for which the redactor type should be used. /// The value of . /// , , or is . - [Experimental(diagnosticId: DiagnosticIds.Experiments.Compliance, UrlFormat = DiagnosticIds.UrlFormat)] [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(HmacRedactorOptions))] [UnconditionalSuppressMessage( "Trimming", diff --git a/src/Libraries/Microsoft.Extensions.Diagnostics.Testing/Logging/FakeLogRecord.cs b/src/Libraries/Microsoft.Extensions.Diagnostics.Testing/Logging/FakeLogRecord.cs index e9828cff6a0..8a3b184732d 100644 --- a/src/Libraries/Microsoft.Extensions.Diagnostics.Testing/Logging/FakeLogRecord.cs +++ b/src/Libraries/Microsoft.Extensions.Diagnostics.Testing/Logging/FakeLogRecord.cs @@ -3,9 +3,7 @@ using System; using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; using Microsoft.Extensions.Logging; -using Microsoft.Shared.DiagnosticIds; using Microsoft.Shared.Diagnostics; namespace Microsoft.Extensions.Logging.Testing; @@ -77,7 +75,6 @@ public FakeLogRecord(LogLevel level, EventId id, object? state, Exception? excep /// The value associated with the key, or if the key was not found. If the structured /// state contains multiple entries with the same key, the value associated with the first matching key encountered is returned. /// - [Experimental(diagnosticId: DiagnosticIds.Experiments.Telemetry, UrlFormat = DiagnosticIds.UrlFormat)] public string? GetStructuredStateValue(string key) { if (StructuredState is not null) diff --git a/src/Libraries/Microsoft.Extensions.Diagnostics.Testing/Microsoft.Extensions.Diagnostics.Testing.json b/src/Libraries/Microsoft.Extensions.Diagnostics.Testing/Microsoft.Extensions.Diagnostics.Testing.json index 4432a3ca1c5..b2e7a70ed15 100644 --- a/src/Libraries/Microsoft.Extensions.Diagnostics.Testing/Microsoft.Extensions.Diagnostics.Testing.json +++ b/src/Libraries/Microsoft.Extensions.Diagnostics.Testing/Microsoft.Extensions.Diagnostics.Testing.json @@ -1,5 +1,5 @@ { - "Name": "Microsoft.Extensions.Diagnostics.Testing, Version=8.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35", + "Name": "Microsoft.Extensions.Diagnostics.Testing, Version=8.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35", "Types": [ { "Type": "sealed class Microsoft.Extensions.Diagnostics.Metrics.Testing.CollectedMeasurement where T : struct", @@ -257,6 +257,10 @@ "Member": "Microsoft.Extensions.Logging.Testing.FakeLogRecord.FakeLogRecord(Microsoft.Extensions.Logging.LogLevel level, Microsoft.Extensions.Logging.EventId id, object? state, System.Exception? exception, string message, System.Collections.Generic.IReadOnlyList scopes, string? category, bool enabled, System.DateTimeOffset timestamp);", "Stage": "Stable" }, + { + "Member": "string? Microsoft.Extensions.Logging.Testing.FakeLogRecord.GetStructuredStateValue(string key);", + "Stage": "Stable" + }, { "Member": "override string Microsoft.Extensions.Logging.Testing.FakeLogRecord.ToString();", "Stage": "Stable" From 40fdaf9b3142ef8506b3012da6b44ba8ced93756 Mon Sep 17 00:00:00 2001 From: Martin Taillefer Date: Wed, 27 Dec 2023 06:17:17 -0800 Subject: [PATCH 2/4] Separate EnableRedaction and EnableEnrichment (#4838) * Separate EnableRedaction and EnableEnrichment - Prior, calling EnableRedaction or EnableEnrichment would always enable both enrichment and redaction together. Now, each call only affects the specific feature it is intended. Fixes #4683 * Update src/Libraries/Microsoft.Extensions.Telemetry/Logging/ExtendedLoggerFactory.cs Co-authored-by: Igor Velikorossov --------- Co-authored-by: Martin Taillefer Co-authored-by: Igor Velikorossov --- .../Logging/ExtendedLoggerFactory.cs | 12 ++++- .../Logging/ExtendedLoggerTests.cs | 49 +++++++++++++++---- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.Telemetry/Logging/ExtendedLoggerFactory.cs b/src/Libraries/Microsoft.Extensions.Telemetry/Logging/ExtendedLoggerFactory.cs index 4f3a0525aae..9a98b8446f4 100644 --- a/src/Libraries/Microsoft.Extensions.Telemetry/Logging/ExtendedLoggerFactory.cs +++ b/src/Libraries/Microsoft.Extensions.Telemetry/Logging/ExtendedLoggerFactory.cs @@ -63,6 +63,16 @@ public ExtendedLoggerFactory( _filterOptionsChangeTokenRegistration = filterOptions.OnChange(RefreshFilters); RefreshFilters(filterOptions.CurrentValue); + if (enrichmentOptions is null) + { + // enrichmentOptions is only present if EnableEnrichment was called, so if it's null + // then ignore all the supplied enrichers, we're not doing enrichment +#pragma warning disable S1226 + enrichers = []; + staticEnrichers = []; +#pragma warning restore S1226 + } + _enrichers = enrichers.Select>(e => e.Enrich).ToArray(); _enrichmentOptionsChangeTokenRegistration = enrichmentOptions?.OnChange(UpdateEnrichmentOptions); _redactionOptionsChangeTokenRegistration = redactionOptions?.OnChange(UpdateRedactionOptions); @@ -80,7 +90,7 @@ public ExtendedLoggerFactory( } _staticTags = [.. tags]; - Config = ComputeConfig(enrichmentOptions?.CurrentValue, redactionOptions?.CurrentValue); + Config = ComputeConfig(enrichmentOptions?.CurrentValue ?? new(), redactionOptions?.CurrentValue ?? new() { ApplyDiscriminator = false }); } public void Dispose() diff --git a/test/Libraries/Microsoft.Extensions.Telemetry.Tests/Logging/ExtendedLoggerTests.cs b/test/Libraries/Microsoft.Extensions.Telemetry.Tests/Logging/ExtendedLoggerTests.cs index abc274e2cf0..69d1d0e8f57 100644 --- a/test/Libraries/Microsoft.Extensions.Telemetry.Tests/Logging/ExtendedLoggerTests.cs +++ b/test/Libraries/Microsoft.Extensions.Telemetry.Tests/Logging/ExtendedLoggerTests.cs @@ -16,8 +16,9 @@ namespace Microsoft.Extensions.Logging.Test; public static class ExtendedLoggerTests { - [Fact] - public static void Basic() + [Theory] + [CombinatorialData] + public static void FeatureEnablement(bool enableRedaction, bool enableEnrichment) { const string Category = "C1"; @@ -39,11 +40,14 @@ public static void Basic() RedactionFormat = "REDACTED<{0}>", }); + var enrichmentOptions = enableEnrichment ? new StaticOptionsMonitor(new()) : null; + var redactionOptions = enableRedaction ? new StaticOptionsMonitor(new() { ApplyDiscriminator = false }) : null; + using var lf = new ExtendedLoggerFactory( providers: new[] { provider }, filterOptions: new StaticOptionsMonitor(new()), - enrichmentOptions: new StaticOptionsMonitor(new()), - redactionOptions: new StaticOptionsMonitor(new() { ApplyDiscriminator = false }), + enrichmentOptions: enrichmentOptions, + redactionOptions: redactionOptions, enrichers: new[] { enricher }, staticEnrichers: new[] { staticEnricher }, redactorProvider: redactorProvider, @@ -74,18 +78,45 @@ public static void Basic() Assert.Null(snap[0].Exception); Assert.Equal(new EventId(0), snap[0].Id); Assert.Equal("MSG0", snap[0].Message); - Assert.Equal("EV1", snap[0].GetStructuredStateValue("EK1")); - Assert.Equal("SEV1", snap[0].GetStructuredStateValue("SEK1")); + + if (enableEnrichment) + { + Assert.Equal("SEV1", snap[0].GetStructuredStateValue("SEK1")); + Assert.Equal("EV1", snap[0].GetStructuredStateValue("EK1")); + } + else + { + Assert.Null(snap[0].GetStructuredStateValue("SEK1")); + Assert.Null(snap[0].GetStructuredStateValue("EK1")); + } Assert.Equal(Category, snap[1].Category); Assert.Null(snap[1].Exception); Assert.Equal(new EventId(2, "ID2"), snap[1].Id); Assert.Equal("MSG2", snap[1].Message); Assert.Equal("PV2", snap[1].GetStructuredStateValue("PK2")); - Assert.Equal("REDACTED", snap[1].GetStructuredStateValue("PK3")); + + if (enableRedaction) + { + Assert.Equal("REDACTED", snap[1].GetStructuredStateValue("PK3")); + } + else + { + Assert.Equal("PV3", snap[1].GetStructuredStateValue("PK3")); + } + Assert.Null(snap[1].GetStructuredStateValue("PK4")); - Assert.Equal("EV1", snap[1].GetStructuredStateValue("EK1")); - Assert.Equal("SEV1", snap[1].GetStructuredStateValue("SEK1")); + + if (enableEnrichment) + { + Assert.Equal("SEV1", snap[1].GetStructuredStateValue("SEK1")); + Assert.Equal("EV1", snap[1].GetStructuredStateValue("EK1")); + } + else + { + Assert.Null(snap[1].GetStructuredStateValue("SEK1")); + Assert.Null(snap[1].GetStructuredStateValue("EK1")); + } } [Theory] From f4315cd121a4b4006db2ba5744c602e4fef13508 Mon Sep 17 00:00:00 2001 From: Martin Taillefer Date: Wed, 27 Dec 2023 11:22:32 -0800 Subject: [PATCH 3/4] Work on the logging generator. (#4840) - The generator now produces a warning when asked to log an object which doesn't implement ToString(), IConvertible, or IFormattable. Fixed #4835. - Added support for the Transitive property in the LogProperties attribute. When set to true, this causes automatic transitive traversal of a complex object, instead of requiring manual annotations of individual properties. Fixes #4738. - Introduce the [TagName] attribute to make it possible to control the tag name used when logging a parameter or property. Fixes #4576. - Fixed some situations where unnatural errors were produced as a result of a prior error. The dummy follow-on errors are now avoided. - Fixed handling of cases where parameters or properties were of type of the non-generic IEnumerable. The specific type wasn't being recorgnized and treated as an enumerable. Co-authored-by: Martin Taillefer --- docs/list-of-diagnostics.md | 54 +--------- .../Emission/Emitter.Method.cs | 100 +++++++++--------- .../Emission/Emitter.Utils.cs | 2 +- .../Model/LoggingMethod.cs | 36 ++++++- .../Model/LoggingMethodParameter.cs | 7 +- .../Model/LoggingMethodParameterExtensions.cs | 3 +- .../Model/LoggingProperty.cs | 7 +- .../Parsing/AttributeProcessors.cs | 16 ++- .../Parsing/DiagDescriptors.cs | 13 ++- .../Parsing/Parser.LogProperties.cs | 58 +++++++--- .../Microsoft.Gen.Logging/Parsing/Parser.cs | 90 ++++++++++++---- .../Parsing/Resources.Designer.cs | 54 ++++++---- .../Parsing/Resources.resx | 14 ++- .../Parsing/SymbolHolder.cs | 3 +- .../Parsing/SymbolLoader.cs | 4 + .../Parsing/TemplateExtractor.cs | 17 +-- .../Parsing/TypeSymbolExtensions.cs | 22 +++- .../Logging/LogPropertiesAttribute.cs | 21 +++- .../Logging/TagNameAttribute.cs | 34 ++++++ src/Shared/DiagnosticIds/DiagnosticIds.cs | 28 +---- .../Generated/TagNameTests.cs | 28 +++++ .../Generated/TransitiveTests.cs | 40 +++++++ .../LogPropertiesSpecialTypesExtensions.cs | 2 + .../TestClasses/TagNameExtensions.cs | 13 +++ .../TestClasses/TransitiveTestExtensions.cs | 28 +++++ .../Unit/EmitterTests.cs | 2 + .../Unit/EmitterUtilsTests.cs | 2 +- .../Unit/LogParserUtilitiesTests.cs | 3 + .../Unit/LoggingMethodParameterTests.cs | 8 +- .../Unit/LoggingMethodTests.cs | 18 ---- .../Unit/ParserTests.LogProperties.cs | 62 +++++++++-- .../Microsoft.Gen.Logging/Unit/ParserTests.cs | 2 + .../Logging/LogPropertiesAttributeTests.cs | 10 ++ .../Logging/TagNameAttributeTests.cs | 19 ++++ 34 files changed, 566 insertions(+), 254 deletions(-) create mode 100644 src/Libraries/Microsoft.Extensions.Telemetry.Abstractions/Logging/TagNameAttribute.cs create mode 100644 test/Generators/Microsoft.Gen.Logging/Generated/TagNameTests.cs create mode 100644 test/Generators/Microsoft.Gen.Logging/Generated/TransitiveTests.cs create mode 100644 test/Generators/Microsoft.Gen.Logging/TestClasses/TagNameExtensions.cs create mode 100644 test/Generators/Microsoft.Gen.Logging/TestClasses/TransitiveTestExtensions.cs create mode 100644 test/Libraries/Microsoft.Extensions.Telemetry.Abstractions.Tests/Logging/TagNameAttributeTests.cs diff --git a/docs/list-of-diagnostics.md b/docs/list-of-diagnostics.md index db04bdd4529..47778c3774c 100644 --- a/docs/list-of-diagnostics.md +++ b/docs/list-of-diagnostics.md @@ -8,56 +8,6 @@ | `CTXOPTGEN002` | The options context type does not have usable properties | | `CTXOPTGEN003` | The options context cannot be a ref-like type | - -# Design - -| Diagnostic ID | Description | -| :---------------- | :---------- | -| `AUTOCLIENTGEN001` | API client interfaces must not be nested types | -| `AUTOCLIENTGEN002` | REST API client does not have methods defined | -| `AUTOCLIENTGEN003` | An API method must not contain more than one REST method attribute | -| `AUTOCLIENTGEN004` | Invalid API method return type | -| `AUTOCLIENTGEN005` | API methods can't be generic | -| `AUTOCLIENTGEN006` | The current HTTP method does not support the body tag | -| `AUTOCLIENTGEN007` | API methods must not be static | -| `AUTOCLIENTGEN008` | HTTP method missing | -| `AUTOCLIENTGEN009` | The API interface cannot be generic | -| `AUTOCLIENTGEN010` | Invalid API interface name | -| `AUTOCLIENTGEN011` | Duplicate body attribute | -| `AUTOCLIENTGEN012` | URL parameter missing from path | -| `AUTOCLIENTGEN013` | REST API method has more than one cancellation token | -| `AUTOCLIENTGEN014` | Missing CancellationToken from REST API method | -| `AUTOCLIENTGEN015` | API method path should not contain query | -| `AUTOCLIENTGEN016` | A REST API method's request name must be unique | -| `AUTOCLIENTGEN017` | Invalid HttpClient name | -| `AUTOCLIENTGEN018` | Invalid dependency name | -| `AUTOCLIENTGEN019` | Invalid header name | -| `AUTOCLIENTGEN020` | Invalid header value | -| `AUTOCLIENTGEN021` | Invalid REST method path | -| `AUTOCLIENTGEN022` | Invalid request name | - - -# ExtraAnalyzers - -| Diagnostic ID | Category | Description | -| :---------------- | :---------- | :---------- | -| `EA0000` | Performance | Use source generated logging methods for improved performance | -| `EA0001` | Performance | Perform message formatting in the body of the logging method | -| `EA0002` | Reliability | Use 'System.TimeProvider' to make the code easier to test | -| `EA0003` | Performance | Use the character-based overloads of 'String.StartsWith' or 'String.EndsWith' | -| `EA0004` | Performance | Make types declared in an executable internal | -| `EA0005` | Performance | Consider using an array instead of a collection | -| `EA0006` | Performance | Replace uses of 'Enum.GetName' and 'Enum.ToString' for improved performance | -| `EA0007` | Performance | Use 'System.ValueTuple' instead of 'System.Tuple' for improved performance | -| `EA0008` | Performance | Use generic collections instead of legacy collections for improved performance | -| `EA0009` | Performance | Use 'System.MemoryExtensions.Split' for improved performance | -| `EA0010` | Correctness | Fire-and-forget async call inside a 'using' block | -| `EA0011` | Performance | Consider removing unnecessary conditional access operator (?) | -| `EA0012` | Performance | Consider removing unnecessary null coalescing assignment (??=) | -| `EA0013` | Performance | Consider removing unnecessary null coalescing operator (??) | -| `EA0014` | Resilience | The async method doesn't support cancellation | - - # Experiments As new functionality is introduced to this repo, new in-development APIs are marked as being experimental. Experimental APIs offer no @@ -72,14 +22,12 @@ If you use experimental APIs, you will get one of the diagnostic shown below. Th using such an API so that you can avoid accidentally depending on experimental features. You may suppress these diagnostics if desired. - | Diagnostic ID | Description | | :---------------- | :---------- | | `EXTEXP0001` | Resilience experiments | | `EXTEXP0002` | Compliance experiments | | `EXTEXP0003` | Telemetry experiments | | `EXTEXP0004` | TimeProvider experiments | -| `EXTEXP0005` | AutoClient experiments | | `EXTEXP0006` | AsyncState experiments | | `EXTEXP0007` | Health check experiments | | `EXTEXP0008` | Resource monitoring experiments | @@ -134,7 +82,7 @@ if desired. | `LOGGEN033` | Method parameter can't be used with a tag provider | | `LOGGEN034` | Attribute can't be used in this context | | `LOGGEN035` | The logging method parameter leaks sensitive data | - +| `LOGGEN036` | A value being logged doesn't have an effective way to be converted into a string | # Metrics diff --git a/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs b/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs index 2269012bec3..33567379bf9 100644 --- a/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs +++ b/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs @@ -74,10 +74,10 @@ private void GenLogMethod(LoggingMethod lm) OutLn(); } - var stateName = PickUniqueName("state", lm.Parameters.Select(p => p.Name)); + var stateName = PickUniqueName("state", lm.Parameters.Select(p => p.ParameterName)); OutLn($"var {stateName} = {LoggerMessageHelperType}.ThreadLocalState;"); - GenPropertyLoads(lm, stateName, out int numReservedUnclassifiedTags, out int numReservedClassifiedTags); + GenTagWrites(lm, stateName, out int numReservedUnclassifiedTags, out int numReservedClassifiedTags); OutLn(); OutLn($"{logger}.Log("); @@ -97,7 +97,7 @@ private void GenLogMethod(LoggingMethod lm) OutLn($"{stateName},"); OutLn($"{exceptionArg},"); - var lambdaStateName = PickUniqueName("s", lm.TemplateToParameterName.Select(kvp => kvp.Key)); + var lambdaStateName = PickUniqueName("s", lm.Templates); OutLn($"[{GeneratorUtilities.GeneratedCodeAttribute}] static string ({lambdaStateName}, {exceptionLambdaName}) =>"); OutOpenBrace(); @@ -158,7 +158,7 @@ static bool ShouldStringifyProperty(LoggingProperty p) if (p.ImplementsISpanFormattable) { - // pass object as it, it will be formatted directly into the output buffer + // pass object as is, it will be formatted directly into the output buffer return false; } @@ -211,11 +211,11 @@ static string ConvertPropertyToString(LoggingProperty lp, string arg) { if (p.IsException) { - exceptionArg = p.Name; + exceptionArg = p.ParameterName; if (p.UsedAsTemplate) { - exceptionLambdaArg = lm.GetParameterNameInTemplate(p); + exceptionLambdaArg = lm.GetTemplatesForParameter(p)[0]; } break; @@ -235,11 +235,11 @@ static bool NeedsASlot(LoggingMethodParameter p) return p.IsNormalParameter && !p.HasTagProvider && !p.HasProperties; } - void GenPropertyLoads(LoggingMethod lm, string stateName, out int numReservedUnclassifiedTags, out int numReservedClassifiedTags) + void GenTagWrites(LoggingMethod lm, string stateName, out int numReservedUnclassifiedTags, out int numReservedClassifiedTags) { int numUnclassifiedTags = 0; int numClassifiedTags = 0; - var tmpVarName = PickUniqueName("tmp", lm.Parameters.Select(p => p.Name)); + var tmpVarName = PickUniqueName("tmp", lm.Parameters.Select(p => p.ParameterName)); foreach (var p in lm.Parameters) { @@ -288,20 +288,20 @@ void GenPropertyLoads(LoggingMethod lm, string stateName, out int numReservedUnc { if (NeedsASlot(p) && !p.HasDataClassification) { - var key = $"\"{lm.GetParameterNameInTemplate(p)}\""; + var key = $"\"{p.TagName}\""; string value; if (p.IsEnumerable) { value = p.PotentiallyNull - ? $"{p.NameWithAt} != null ? {LoggerMessageHelperType}.Stringify({p.NameWithAt}) : null" - : $"{LoggerMessageHelperType}.Stringify({p.NameWithAt})"; + ? $"{p.ParameterNameWithAt} != null ? {LoggerMessageHelperType}.Stringify({p.ParameterNameWithAt}) : null" + : $"{LoggerMessageHelperType}.Stringify({p.ParameterNameWithAt})"; } else { value = ShouldStringifyParameter(p) - ? ConvertParameterToString(p, p.NameWithAt) - : p.NameWithAt; + ? ConvertParameterToString(p, p.ParameterNameWithAt) + : p.ParameterNameWithAt; } OutLn($"{stateName}.TagArray[{--count}] = new({key}, {value});"); @@ -348,12 +348,12 @@ void GenPropertyLoads(LoggingMethod lm, string stateName, out int numReservedUnc { if (NeedsASlot(p) && p.HasDataClassification) { - var key = $"\"{lm.GetParameterNameInTemplate(p)}\""; + var key = $"\"{p.TagName}\""; var classification = MakeClassificationValue(p.ClassificationAttributeTypes); var value = ShouldStringifyParameter(p) - ? ConvertParameterToString(p, p.NameWithAt) - : p.NameWithAt; + ? ConvertParameterToString(p, p.ParameterNameWithAt) + : p.ParameterNameWithAt; OutLn($"{stateName}.ClassifiedTagArray[{--count}] = new({key}, {value}, {classification});"); } @@ -469,10 +469,10 @@ void GenPropertyLoads(LoggingMethod lm, string stateName, out int numReservedUnc } else { - OutLn($"{stateName}.TagNamePrefix = nameof({p.NameWithAt});"); + OutLn($"{stateName}.TagNamePrefix = nameof({p.ParameterNameWithAt});"); } - OutLn($"{p.TagProvider!.ContainingType}.{p.TagProvider.MethodName}({stateName}, {p.NameWithAt});"); + OutLn($"{p.TagProvider!.ContainingType}.{p.TagProvider.MethodName}({stateName}, {p.ParameterNameWithAt});"); } } } @@ -488,20 +488,22 @@ bool GenVariableAssignments(LoggingMethod lm, string lambdaStateName, int numRes { if (p.UsedAsTemplate) { - var key = lm.GetParameterNameInTemplate(p); - - var atSign = p.NeedsAtSign ? "@" : string.Empty; - if (p.PotentiallyNull) - { - const string Null = "\"(null)\""; - OutLn($"var {atSign}{key} = {lambdaStateName}.TagArray[{index}].Value ?? {Null};"); - } - else + var templates = lm.GetTemplatesForParameter(p); + foreach (var t in templates) { - OutLn($"var {atSign}{key} = {lambdaStateName}.TagArray[{index}].Value;"); - } + var atSign = p.NeedsAtSign ? "@" : string.Empty; + if (p.PotentiallyNull) + { + const string Null = "\"(null)\""; + OutLn($"var {atSign}{t} = {lambdaStateName}.TagArray[{index}].Value ?? {Null};"); + } + else + { + OutLn($"var {atSign}{t} = {lambdaStateName}.TagArray[{index}].Value;"); + } - generatedAssignments = true; + generatedAssignments = true; + } } index--; @@ -515,20 +517,22 @@ bool GenVariableAssignments(LoggingMethod lm, string lambdaStateName, int numRes { if (p.UsedAsTemplate) { - var key = lm.GetParameterNameInTemplate(p); - - var atSign = p.NeedsAtSign ? "@" : string.Empty; - if (p.PotentiallyNull) - { - const string Null = "\"(null)\""; - OutLn($"var {atSign}{key} = {lambdaStateName}.RedactedTagArray[{index}].Value ?? {Null};"); - } - else + var templates = lm.GetTemplatesForParameter(p); + foreach (var t in templates) { - OutLn($"var {atSign}{key} = {lambdaStateName}.RedactedTagArray[{index}].Value;"); - } + var atSign = p.NeedsAtSign ? "@" : string.Empty; + if (p.PotentiallyNull) + { + const string Null = "\"(null)\""; + OutLn($"var {atSign}{t} = {lambdaStateName}.RedactedTagArray[{index}].Value ?? {Null};"); + } + else + { + OutLn($"var {atSign}{t} = {lambdaStateName}.RedactedTagArray[{index}].Value;"); + } - generatedAssignments = true; + generatedAssignments = true; + } } index--; @@ -547,7 +551,7 @@ bool GenVariableAssignments(LoggingMethod lm, string lambdaStateName, int numRes { if (p.IsLogger) { - logger = p.Name; + logger = p.ParameterName; isNullable = p.IsNullable; break; } @@ -597,7 +601,7 @@ private string AddAtSymbolsToTemplates(string template, IEnumerable Parameters = []; - public readonly Dictionary TemplateToParameterName = new(StringComparer.OrdinalIgnoreCase); + public readonly List Templates = []; public string Name = string.Empty; public string Message = string.Empty; public int? Level; @@ -28,8 +29,33 @@ internal sealed class LoggingMethod public bool LoggerMemberNullable; public bool HasXmlDocumentation; - public string GetParameterNameInTemplate(LoggingMethodParameter parameter) - => TemplateToParameterName.TryGetValue(parameter.Name, out var value) - ? value - : parameter.Name; + public LoggingMethodParameter? GetParameterForTemplate(string templateName) + { + foreach (var p in Parameters) + { + if (templateName.Equals(p.ParameterName, StringComparison.OrdinalIgnoreCase)) + { + return p; + } + } + + return null; + } + + public List GetTemplatesForParameter(LoggingMethodParameter lp) + => GetTemplatesForParameter(lp.ParameterName); + + public List GetTemplatesForParameter(string parameterName) + { + HashSet templates = []; + foreach (var t in Templates) + { + if (parameterName.Equals(t, StringComparison.OrdinalIgnoreCase)) + { + _ = templates.Add(t); + } + } + + return templates.ToList(); + } } diff --git a/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameter.cs b/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameter.cs index cf58a1754c9..80021aba766 100644 --- a/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameter.cs +++ b/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameter.cs @@ -13,7 +13,8 @@ namespace Microsoft.Gen.Logging.Model; [DebuggerDisplay("{Name}")] internal sealed class LoggingMethodParameter { - public string Name = string.Empty; + public string ParameterName = string.Empty; + public string TagName = string.Empty; public string Type = string.Empty; public string? Qualifier; public bool NeedsAtSign; @@ -26,6 +27,7 @@ internal sealed class LoggingMethodParameter public bool ImplementsIConvertible; public bool ImplementsIFormattable; public bool ImplementsISpanFormattable; + public bool HasCustomToString; public bool SkipNullProperties; public bool OmitReferenceName; public bool UsedAsTemplate; @@ -33,7 +35,7 @@ internal sealed class LoggingMethodParameter public List Properties = []; public TagProvider? TagProvider; - public string NameWithAt => NeedsAtSign ? "@" + Name : Name; + public string ParameterNameWithAt => NeedsAtSign ? "@" + ParameterName : ParameterName; public string PotentiallyNullableType => (IsReference && !IsNullable) @@ -48,4 +50,5 @@ public string PotentiallyNullableType public bool HasProperties => Properties.Count > 0; public bool HasTagProvider => TagProvider is not null; public bool PotentiallyNull => (IsReference && !IsLogger) || IsNullable; + public bool IsStringifiable => HasCustomToString || ImplementsIConvertible || ImplementsIFormattable || IsEnumerable; } diff --git a/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameterExtensions.cs b/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameterExtensions.cs index 54ace58c21f..d70b8207fc2 100644 --- a/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameterExtensions.cs +++ b/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameterExtensions.cs @@ -16,7 +16,8 @@ internal static void TraverseParameterPropertiesTransitively( var firstProperty = new LoggingProperty { - Name = parameter.Name, + PropertyName = parameter.ParameterName, + TagName = parameter.TagName, NeedsAtSign = parameter.NeedsAtSign, Type = parameter.Type, IsNullable = parameter.IsNullable, diff --git a/src/Generators/Microsoft.Gen.Logging/Model/LoggingProperty.cs b/src/Generators/Microsoft.Gen.Logging/Model/LoggingProperty.cs index ee7871c0a3f..cb954921b78 100644 --- a/src/Generators/Microsoft.Gen.Logging/Model/LoggingProperty.cs +++ b/src/Generators/Microsoft.Gen.Logging/Model/LoggingProperty.cs @@ -9,7 +9,8 @@ namespace Microsoft.Gen.Logging.Model; [DebuggerDisplay("{Name}")] internal sealed class LoggingProperty { - public string Name = string.Empty; + public string PropertyName = string.Empty; + public string TagName = string.Empty; public string Type = string.Empty; public HashSet ClassificationAttributeTypes = []; public bool NeedsAtSign; @@ -19,6 +20,7 @@ internal sealed class LoggingProperty public bool ImplementsIConvertible; public bool ImplementsIFormattable; public bool ImplementsISpanFormattable; + public bool HasCustomToString; public List Properties = []; public bool OmitReferenceName; public TagProvider? TagProvider; @@ -26,6 +28,7 @@ internal sealed class LoggingProperty public bool HasDataClassification => ClassificationAttributeTypes.Count > 0; public bool HasProperties => Properties.Count > 0; public bool HasTagProvider => TagProvider is not null; - public string NameWithAt => NeedsAtSign ? "@" + Name : Name; + public string PropertyNameWithAt => NeedsAtSign ? "@" + PropertyName : PropertyName; public bool PotentiallyNull => IsReference || IsNullable; + public bool IsStringifiable => HasCustomToString || ImplementsIConvertible || ImplementsIFormattable || IsEnumerable; } diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/AttributeProcessors.cs b/src/Generators/Microsoft.Gen.Logging/Parsing/AttributeProcessors.cs index dcafa051ac1..f20dd59f52b 100644 --- a/src/Generators/Microsoft.Gen.Logging/Parsing/AttributeProcessors.cs +++ b/src/Generators/Microsoft.Gen.Logging/Parsing/AttributeProcessors.cs @@ -15,6 +15,7 @@ internal static class AttributeProcessors private const string SkipEnabledCheckProperty = "SkipEnabledCheck"; private const string SkipNullProperties = "SkipNullProperties"; private const string OmitReferenceName = "OmitReferenceName"; + private const string Transitive = "Transitive"; private const int LogLevelError = 4; private const int LogLevelCritical = 5; @@ -124,10 +125,11 @@ public static (int? eventId, int? level, string message, string? eventName, bool return (eventId, level, message, eventName, skipEnabledCheck); } - public static (bool skipNullProperties, bool omitReferenceName) ExtractLogPropertiesAttributeValues(AttributeData attr) + public static (bool skipNullProperties, bool omitReferenceName, bool transitive) ExtractLogPropertiesAttributeValues(AttributeData attr) { bool skipNullProperties = false; bool omitReferenceName = false; + bool transitive = false; foreach (var a in attr.NamedArguments) { @@ -148,10 +150,17 @@ public static (bool skipNullProperties, bool omitReferenceName) ExtractLogProper omitReferenceName = b; } } + else if (a.Key == Transitive) + { + if (v is bool b) + { + transitive = b; + } + } } } - return (skipNullProperties, omitReferenceName); + return (skipNullProperties, omitReferenceName, transitive); } public static (bool omitReferenceName, ITypeSymbol providerType, string providerMethodName) ExtractTagProviderAttributeValues(AttributeData attr) @@ -180,4 +189,7 @@ public static (bool omitReferenceName, ITypeSymbol providerType, string provider return (omitReferenceName, providerType!, providerMethodName!); } + + public static string ExtractTagNameAttributeValues(AttributeData attr) + => attr.ConstructorArguments[0].Value as string ?? string.Empty; } diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/DiagDescriptors.cs b/src/Generators/Microsoft.Gen.Logging/Parsing/DiagDescriptors.cs index bac2e7a81e8..081e098ccde 100644 --- a/src/Generators/Microsoft.Gen.Logging/Parsing/DiagDescriptors.cs +++ b/src/Generators/Microsoft.Gen.Logging/Parsing/DiagDescriptors.cs @@ -196,10 +196,10 @@ internal sealed class DiagDescriptors : DiagDescriptorsBase messageFormat: Resources.LogPropertiesHiddenPropertyDetectedMessage, category: Category); - public static DiagnosticDescriptor LogPropertiesNameCollision { get; } = Make( + public static DiagnosticDescriptor TagNameCollision { get; } = Make( id: DiagnosticIds.LoggerMessage.LOGGEN029, - title: Resources.LogPropertiesNameCollisionTitle, - messageFormat: Resources.LogPropertiesNameCollisionMessage, + title: Resources.TagNameCollisionTitle, + messageFormat: Resources.TagNameCollisionMessage, category: Category); public static DiagnosticDescriptor EmptyLoggingMethod { get; } = Make( @@ -237,4 +237,11 @@ internal sealed class DiagDescriptors : DiagDescriptorsBase title: Resources.RecordTypeSensitiveArgumentIsInTemplateTitle, messageFormat: Resources.RecordTypeSensitiveArgumentIsInTemplateMessage, category: Category); + + public static DiagnosticDescriptor DefaultToString { get; } = Make( + id: DiagnosticIds.LoggerMessage.LOGGEN036, + title: Resources.DefaultToStringTitle, + messageFormat: Resources.DefaultToStringMessage, + category: Category, + DiagnosticSeverity.Warning); } diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.LogProperties.cs b/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.LogProperties.cs index 4fabb10e603..3f5cf8988ef 100644 --- a/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.LogProperties.cs +++ b/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.LogProperties.cs @@ -47,13 +47,13 @@ internal bool ProcessLogPropertiesForParameter( paramTypeSymbol = ((INamedTypeSymbol)paramTypeSymbol).TypeArguments[0]; } - (lp.SkipNullProperties, lp.OmitReferenceName) = AttributeProcessors.ExtractLogPropertiesAttributeValues(logPropertiesAttribute); + (lp.SkipNullProperties, lp.OmitReferenceName, bool transitive) = AttributeProcessors.ExtractLogPropertiesAttributeValues(logPropertiesAttribute); var typesChain = new HashSet(SymbolEqualityComparer.Default); _ = typesChain.Add(paramTypeSymbol); // Add itself - var props = GetTypePropertiesToLog(paramTypeSymbol, typesChain, symbols, ref foundDataClassificationAttributes); + var props = GetTypePropertiesToLog(paramTypeSymbol, typesChain, symbols, transitive, ref foundDataClassificationAttributes); if (props == null) { return false; @@ -86,6 +86,7 @@ static string GetPropertyIdentifier(IPropertySymbol property, CancellationToken ITypeSymbol type, ISet typesChain, SymbolHolder symbols, + bool transitive, ref bool foundDataClassificationAttributes) { var result = new List(); @@ -182,9 +183,15 @@ static string GetPropertyIdentifier(IPropertySymbol property, CancellationToken extractedType = ((INamedTypeSymbol)extractedType).TypeArguments[0]; } + var tagNameAttribute = ParserUtilities.GetSymbolAttributeAnnotationOrDefault(symbols.TagNameAttribute, property); + var tagName = tagNameAttribute != null + ? AttributeProcessors.ExtractTagNameAttributeValues(tagNameAttribute) + : property.Name; + var lp = new LoggingProperty { - Name = property.Name, + PropertyName = property.Name, + TagName = tagName, Type = property.Type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat), ClassificationAttributeTypes = classification, IsReference = property.Type.IsReferenceType, @@ -193,6 +200,7 @@ static string GetPropertyIdentifier(IPropertySymbol property, CancellationToken ImplementsIConvertible = property.Type.ImplementsIConvertible(symbols), ImplementsIFormattable = property.Type.ImplementsIFormattable(symbols), ImplementsISpanFormattable = property.Type.ImplementsISpanFormattable(symbols), + HasCustomToString = property.Type.HasCustomToString(), }; if (!property.DeclaringSyntaxReferences.IsDefaultOrEmpty) @@ -233,14 +241,16 @@ static string GetPropertyIdentifier(IPropertySymbol property, CancellationToken return null; } - if (logPropertiesAttribute != null) + if (logPropertiesAttribute != null || (transitive && tagProviderAttribute == null && logPropertyIgnoreAttribute == null)) { - _ = CanLogProperties(property, property.Type, symbols); - if ((property.DeclaredAccessibility != Accessibility.Public || property.IsStatic) || (property.GetMethod == null || property.GetMethod.DeclaredAccessibility != Accessibility.Public)) { - Diag(DiagDescriptors.InvalidAttributeUsage, logPropertiesAttribute.ApplicationSyntaxReference?.GetSyntax(_cancellationToken).GetLocation(), "LogProperties"); + if (logPropertiesAttribute != null) + { + Diag(DiagDescriptors.InvalidAttributeUsage, logPropertiesAttribute.ApplicationSyntaxReference?.GetSyntax(_cancellationToken).GetLocation(), "LogProperties"); + } + continue; } @@ -260,13 +270,16 @@ static string GetPropertyIdentifier(IPropertySymbol property, CancellationToken extractedType = ((INamedTypeSymbol)extractedType).TypeArguments[0]; } - _ = typesChain.Add(namedType); - var props = GetTypePropertiesToLog(extractedType, typesChain, symbols, ref foundDataClassificationAttributes); - _ = typesChain.Remove(namedType); - - if (props != null) + if (CanLogProperties(property, property.Type, symbols, silent: logPropertiesAttribute == null)) { - lp.Properties.AddRange(props); + _ = typesChain.Add(namedType); + var props = GetTypePropertiesToLog(extractedType, typesChain, symbols, transitive, ref foundDataClassificationAttributes); + _ = typesChain.Remove(namedType); + + if (props != null) + { + lp.Properties.AddRange(props); + } } } @@ -284,7 +297,7 @@ static string GetPropertyIdentifier(IPropertySymbol property, CancellationToken } } - if (tagProviderAttribute == null && logPropertiesAttribute == null) + if (tagProviderAttribute == null && logPropertiesAttribute == null && !transitive) { if ((property.DeclaredAccessibility != Accessibility.Public || property.IsStatic) || (property.GetMethod == null || property.GetMethod.DeclaredAccessibility != Accessibility.Public) @@ -301,6 +314,15 @@ static string GetPropertyIdentifier(IPropertySymbol property, CancellationToken lp.ClassificationAttributeTypes.Clear(); } + if ((logPropertiesAttribute is null) + && (tagProviderAttribute is null) + && !lp.IsStringifiable + && property.Type.Kind != SymbolKind.TypeParameter + && !transitive) + { + Diag(DiagDescriptors.DefaultToString, property.GetLocation(), property.Type, property.Name); + } + result.Add(lp); } @@ -311,7 +333,7 @@ static string GetPropertyIdentifier(IPropertySymbol property, CancellationToken return result; } - bool CanLogProperties(ISymbol sym, ITypeSymbol symType, SymbolHolder symbols) + bool CanLogProperties(ISymbol sym, ITypeSymbol symType, SymbolHolder symbols, bool silent = false) { var isRegularType = symType.Kind == SymbolKind.NamedType && @@ -326,7 +348,11 @@ bool CanLogProperties(ISymbol sym, ITypeSymbol symType, SymbolHolder symbols) if (!isRegularType || symType.IsSpecialType(symbols)) { - Diag(DiagDescriptors.InvalidTypeToLogProperties, sym.GetLocation(), symType.ToDisplayString()); + if (!silent) + { + Diag(DiagDescriptors.InvalidTypeToLogProperties, sym.GetLocation(), symType.ToDisplayString()); + } + return false; } diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.cs b/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.cs index 4a8793c49b5..00b2f2b2efa 100644 --- a/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.cs +++ b/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.cs @@ -84,7 +84,7 @@ public IReadOnlyList GetLogTypes(IEnumerable foreach (var paramSymbol in methodSymbol.Parameters) { - var lp = ProcessParameter(paramSymbol, symbols, ref parsingState); + var lp = ProcessParameter(lm, paramSymbol, symbols, ref parsingState); if (lp == null) { keepMethod = false; @@ -94,6 +94,7 @@ public IReadOnlyList GetLogTypes(IEnumerable parameterSymbols[lp] = paramSymbol; var foundDataClassificationAttributesInProps = false; + var logPropertiesAttribute = ParserUtilities.GetSymbolAttributeAnnotationOrDefault(symbols.LogPropertiesAttribute, paramSymbol); if (logPropertiesAttribute is not null) { @@ -135,22 +136,43 @@ public IReadOnlyList GetLogTypes(IEnumerable lp.TagProvider = null; } +#pragma warning disable S1067 // Expressions should not be too complex + if (lp.IsNormalParameter + && (logPropertiesAttribute is null) + && (tagProviderAttribute is null) + && !lp.IsStringifiable + && paramSymbol.Type.Kind != SymbolKind.TypeParameter) + { + Diag(DiagDescriptors.DefaultToString, paramSymbol.GetLocation(), paramSymbol.Type, paramSymbol.Name); + } +#pragma warning restore S1067 // Expressions should not be too complex + bool forceAsTemplateParam = false; - bool parameterInTemplate = lm.TemplateToParameterName.ContainsKey(lp.Name); + + bool parameterInTemplate = false; + foreach (var t in lm.Templates) + { + if (lp.ParameterName.Equals(t, StringComparison.OrdinalIgnoreCase)) + { + parameterInTemplate = true; + break; + } + } + var loggingProperties = logPropertiesAttribute != null || tagProviderAttribute != null; if (lp.IsLogger && parameterInTemplate) { - Diag(DiagDescriptors.ShouldntMentionLoggerInMessage, attrLoc, lp.Name); + Diag(DiagDescriptors.ShouldntMentionLoggerInMessage, attrLoc, lp.ParameterName); forceAsTemplateParam = true; } else if (lp.IsException && parameterInTemplate) { - Diag(DiagDescriptors.ShouldntMentionExceptionInMessage, attrLoc, lp.Name); + Diag(DiagDescriptors.ShouldntMentionExceptionInMessage, attrLoc, lp.ParameterName); forceAsTemplateParam = true; } else if (lp.IsLogLevel && parameterInTemplate) { - Diag(DiagDescriptors.ShouldntMentionLogLevelInMessage, attrLoc, lp.Name); + Diag(DiagDescriptors.ShouldntMentionLogLevelInMessage, attrLoc, lp.ParameterName); forceAsTemplateParam = true; } else if (lp.IsNormalParameter @@ -158,7 +180,7 @@ public IReadOnlyList GetLogTypes(IEnumerable && !loggingProperties && !string.IsNullOrEmpty(lm.Message)) { - Diag(DiagDescriptors.ParameterHasNoCorrespondingTemplate, paramSymbol.GetLocation(), lp.Name); + Diag(DiagDescriptors.ParameterHasNoCorrespondingTemplate, paramSymbol.GetLocation(), lp.ParameterName); } var purelyStructuredLoggingParameter = loggingProperties && !parameterInTemplate; @@ -170,7 +192,7 @@ public IReadOnlyList GetLogTypes(IEnumerable if (foundDataClassificationAttributesInProps || RecordHasSensitivePublicMembers(paramSymbol.Type, symbols)) { - Diag(DiagDescriptors.RecordTypeSensitiveArgumentIsInTemplate, paramSymbol.GetLocation(), lp.Name, lm.Name); + Diag(DiagDescriptors.RecordTypeSensitiveArgumentIsInTemplate, paramSymbol.GetLocation(), lp.ParameterName, lm.Name); keepMethod = false; } } @@ -246,12 +268,12 @@ public IReadOnlyList GetLogTypes(IEnumerable } } - foreach (var t in lm.TemplateToParameterName) + foreach (var t in lm.Templates) { bool found = false; foreach (var p in lm.Parameters) { - if (t.Key.Equals(p.Name, StringComparison.OrdinalIgnoreCase)) + if (t.Equals(p.ParameterName, StringComparison.OrdinalIgnoreCase)) { found = true; break; @@ -260,11 +282,11 @@ public IReadOnlyList GetLogTypes(IEnumerable if (!found) { - Diag(DiagDescriptors.TemplateHasNoCorrespondingParameter, attrLoc, t.Key); + Diag(DiagDescriptors.TemplateHasNoCorrespondingParameter, attrLoc, t); } } - CheckMethodParametersAreUnique(lm, parameterSymbols); + CheckTagNamesAreUnique(lm, parameterSymbols); } if (lt == null) @@ -367,8 +389,6 @@ static bool IsAllowedKind(SyntaxKind kind) => HasXmlDocumentation = HasXmlDocumentation(method), }; - TemplateExtractor.ExtractTemplates(message, lm.TemplateToParameterName, out var templatesWithAtSymbol); - var keepMethod = true; if (!methodSymbol.ReturnsVoid) @@ -378,12 +398,26 @@ static bool IsAllowedKind(SyntaxKind kind) => keepMethod = false; } - if (templatesWithAtSymbol.Count > 0) + TemplateExtractor.ExtractTemplates(message, lm.Templates); + +#pragma warning disable EA0003 // Use the character-based overloads of 'String.StartsWith' or 'String.EndsWith' + var templatesWithAtSymbol = lm.Templates.Where(x => x.StartsWith("@", StringComparison.Ordinal)).ToArray(); + if (templatesWithAtSymbol.Length > 0) { // there is/are template(s) that start with @, which is not allowed Diag(DiagDescriptors.TemplateStartsWithAtSymbol, attrLoc, method.Identifier.Text, string.Join("; ", templatesWithAtSymbol)); keepMethod = false; + + for (int i = 0; i < lm.Templates.Count; i++) + { + if (lm.Templates[i].StartsWith("@", StringComparison.Ordinal)) + { + lm.Templates[i] = lm.Templates[i].Substring(1); + } + } + } +#pragma warning restore EA0003 // Use the character-based overloads of 'String.StartsWith' or 'String.EndsWith' if (method.Arity > 0) { @@ -462,15 +496,14 @@ private static List GetDataClassificationAttributes(ISymbol sy .Select(static x => x!) .ToList(); - private void CheckMethodParametersAreUnique(LoggingMethod lm, Dictionary parameterSymbols) + private void CheckTagNamesAreUnique(LoggingMethod lm, Dictionary parameterSymbols) { var names = new HashSet(StringComparer.Ordinal); foreach (var parameter in lm.Parameters) { - var parameterName = lm.GetParameterNameInTemplate(parameter); - if (!names.Add(parameterName)) + if (!parameter.IsNormalParameter) { - Diag(DiagDescriptors.LogPropertiesNameCollision, parameterSymbols[parameter].GetLocation(), parameter.Name, parameterName, lm.Name); + continue; } if (parameter.HasProperties) @@ -482,17 +515,25 @@ private void CheckMethodParametersAreUnique(LoggingMethod lm, Dictionary x.Name)); + var fullName = string.Join("_", chain.Concat(new[] { leaf }).Select(static x => x.TagName)); if (!names.Add(fullName)) { - Diag(DiagDescriptors.LogPropertiesNameCollision, parameterSymbols[parameter].GetLocation(), parameter.Name, fullName, lm.Name); + Diag(DiagDescriptors.TagNameCollision, parameterSymbols[parameter].GetLocation(), parameter.ParameterName, fullName, lm.Name); } }); } + else + { + if (!names.Add(parameter.TagName)) + { + Diag(DiagDescriptors.TagNameCollision, parameterSymbols[parameter].GetLocation(), parameter.ParameterName, parameter.TagName, lm.Name); + } + } } } private LoggingMethodParameter? ProcessParameter( + LoggingMethod lm, IParameterSymbol paramSymbol, SymbolHolder symbols, ref MethodParsingState parsingState) @@ -550,9 +591,15 @@ private void CheckMethodParametersAreUnique(LoggingMethod lm, Dictionary + /// Looks up a localized string similar to The type "{0}" doesn't implement ToString(), IConvertible, or IFormattable (did you forget to apply [LogProperties] or [TagProvider] to "{1}"?). + /// + internal static string DefaultToStringMessage { + get { + return ResourceManager.GetString("DefaultToStringMessage", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to A value being logged doesn't have an effective way to be converted into a string. + /// + internal static string DefaultToStringTitle { + get { + return ResourceManager.GetString("DefaultToStringTitle", resourceCulture); + } + } + /// /// Looks up a localized string similar to Logging method "{0}" doesn't have anything to be logged. /// @@ -312,24 +330,6 @@ internal static string LogPropertiesInvalidUsageTitle { } } - /// - /// Looks up a localized string similar to Parameter "{0}" causes name conflict with name "{1}" within logging method "{2}". - /// - internal static string LogPropertiesNameCollisionMessage { - get { - return ResourceManager.GetString("LogPropertiesNameCollisionMessage", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to A logging method parameter causes name conflicts. - /// - internal static string LogPropertiesNameCollisionTitle { - get { - return ResourceManager.GetString("LogPropertiesNameCollisionTitle", resourceCulture); - } - } - /// /// Looks up a localized string similar to Type "{0}" used with parameter "{1}" doesn't have any public properties to log. /// @@ -582,6 +582,24 @@ internal static string ShouldntReuseEventNamesTitle { } } + /// + /// Looks up a localized string similar to Parameter "{0}" causes a tag name conflict with name "{1}" within logging method "{2}". + /// + internal static string TagNameCollisionMessage { + get { + return ResourceManager.GetString("TagNameCollisionMessage", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to A logging method parameter causes a tag name conflicts. + /// + internal static string TagNameCollisionTitle { + get { + return ResourceManager.GetString("TagNameCollisionTitle", resourceCulture); + } + } + /// /// Looks up a localized string similar to Parameter "{0}" is annotated to use a tag provider but it has special semantics (ILogger, LogLevel, Exception, etc.). /// diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/Resources.resx b/src/Generators/Microsoft.Gen.Logging/Parsing/Resources.resx index 1a56b134b3f..48c2f6d6de6 100644 --- a/src/Generators/Microsoft.Gen.Logging/Parsing/Resources.resx +++ b/src/Generators/Microsoft.Gen.Logging/Parsing/Resources.resx @@ -282,11 +282,11 @@ Logging method parameter's type has a hidden property - - Parameter "{0}" causes name conflict with name "{1}" within logging method "{2}" + + Parameter "{0}" causes a tag name conflict with name "{1}" within logging method "{2}" - - A logging method parameter causes name conflicts + + A logging method parameter causes a tag name conflicts Logging method doesn't log anything @@ -333,4 +333,10 @@ The logging method parameter leaks sensitive data + + The type "{0}" doesn't implement ToString(), IConvertible, or IFormattable (did you forget to apply [LogProperties] or [TagProvider] to "{1}"?) + + + A value being logged doesn't have an effective way to be converted into a string + \ No newline at end of file diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/SymbolHolder.cs b/src/Generators/Microsoft.Gen.Logging/Parsing/SymbolHolder.cs index 275938c29e7..f8bfa87c05c 100644 --- a/src/Generators/Microsoft.Gen.Logging/Parsing/SymbolHolder.cs +++ b/src/Generators/Microsoft.Gen.Logging/Parsing/SymbolHolder.cs @@ -13,7 +13,8 @@ internal sealed record class SymbolHolder( INamedTypeSymbol LoggerMessageAttribute, INamedTypeSymbol LogPropertiesAttribute, INamedTypeSymbol TagProviderAttribute, - INamedTypeSymbol? LogPropertyIgnoreAttribute, + INamedTypeSymbol TagNameAttribute, + INamedTypeSymbol LogPropertyIgnoreAttribute, INamedTypeSymbol ITagCollectorSymbol, INamedTypeSymbol ILoggerSymbol, INamedTypeSymbol LogLevelSymbol, diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/SymbolLoader.cs b/src/Generators/Microsoft.Gen.Logging/Parsing/SymbolLoader.cs index 4dce3561635..6f8d2a93fe9 100644 --- a/src/Generators/Microsoft.Gen.Logging/Parsing/SymbolLoader.cs +++ b/src/Generators/Microsoft.Gen.Logging/Parsing/SymbolLoader.cs @@ -12,6 +12,7 @@ internal static class SymbolLoader internal const string LoggerMessageAttribute = "Microsoft.Extensions.Logging.LoggerMessageAttribute"; internal const string LogPropertiesAttribute = "Microsoft.Extensions.Logging.LogPropertiesAttribute"; internal const string TagProviderAttribute = "Microsoft.Extensions.Logging.TagProviderAttribute"; + internal const string TagNameAttribute = "Microsoft.Extensions.Logging.TagNameAttribute"; internal const string LogPropertyIgnoreAttribute = "Microsoft.Extensions.Logging.LogPropertyIgnoreAttribute"; internal const string ITagCollectorType = "Microsoft.Extensions.Logging.ITagCollector"; internal const string ILoggerType = "Microsoft.Extensions.Logging.ILogger"; @@ -55,6 +56,7 @@ internal static class SymbolLoader var loggerMessageAttributeSymbol = compilation.GetTypeByMetadataName(LoggerMessageAttribute); var logPropertiesAttributeSymbol = compilation.GetTypeByMetadataName(LogPropertiesAttribute); var tagProviderAttributeSymbol = compilation.GetTypeByMetadataName(TagProviderAttribute); + var tagNameAttributeSymbol = compilation.GetTypeByMetadataName(TagNameAttribute); var tagCollectorSymbol = compilation.GetTypeByMetadataName(ITagCollectorType); var logPropertyIgnoreAttributeSymbol = compilation.GetTypeByMetadataName(LogPropertyIgnoreAttribute); var dataClassificationAttribute = compilation.GetTypeByMetadataName(DataClassificationAttribute); @@ -65,6 +67,7 @@ internal static class SymbolLoader || loggerMessageAttributeSymbol == null || logPropertiesAttributeSymbol == null || tagProviderAttributeSymbol == null + || tagNameAttributeSymbol == null || tagCollectorSymbol == null || logPropertyIgnoreAttributeSymbol == null) { @@ -100,6 +103,7 @@ internal static class SymbolLoader loggerMessageAttributeSymbol, logPropertiesAttributeSymbol, tagProviderAttributeSymbol, + tagNameAttributeSymbol, logPropertyIgnoreAttributeSymbol, tagCollectorSymbol, loggerSymbol, diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/TemplateExtractor.cs b/src/Generators/Microsoft.Gen.Logging/Parsing/TemplateExtractor.cs index bd0015164b8..ddee2884b61 100644 --- a/src/Generators/Microsoft.Gen.Logging/Parsing/TemplateExtractor.cs +++ b/src/Generators/Microsoft.Gen.Logging/Parsing/TemplateExtractor.cs @@ -13,19 +13,15 @@ internal static class TemplateExtractor /// /// Finds the template arguments contained in the message string. /// - internal static void ExtractTemplates(string? message, IDictionary templateToParameterName, out ICollection templatesWithAtSymbol) + internal static void ExtractTemplates(string? message, List templates) { if (string.IsNullOrEmpty(message)) { - templatesWithAtSymbol = Array.Empty(); return; } var scanIndex = 0; var endIndex = message!.Length; -#pragma warning disable CA1859 // Use concrete types when possible for improved performance - ICollection? foundAtTemplates = null; -#pragma warning restore CA1859 // Use concrete types when possible for improved performance while (scanIndex < endIndex) { var openBraceIndex = FindBraceIndex(message, '{', scanIndex, endIndex); @@ -41,19 +37,10 @@ internal static void ExtractTemplates(string? message, IDictionary(); - foundAtTemplates.Add(templateName); - templateName = templateName.Substring(1); - } - - templateToParameterName[templateName] = templateName; + templates.Add(templateName); scanIndex = closeBraceIndex + 1; } } - - templatesWithAtSymbol = foundAtTemplates ?? Array.Empty(); } internal static int FindIndexOfAny(string message, char[] chars, int startIndex, int endIndex) diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/TypeSymbolExtensions.cs b/src/Generators/Microsoft.Gen.Logging/Parsing/TypeSymbolExtensions.cs index a18dc32e598..d52796e85ca 100644 --- a/src/Generators/Microsoft.Gen.Logging/Parsing/TypeSymbolExtensions.cs +++ b/src/Generators/Microsoft.Gen.Logging/Parsing/TypeSymbolExtensions.cs @@ -10,7 +10,8 @@ namespace Microsoft.Gen.Logging.Parsing; internal static class TypeSymbolExtensions { internal static bool IsEnumerable(this ITypeSymbol sym, SymbolHolder symbols) - => sym.ImplementsInterface(symbols.EnumerableSymbol) && sym.SpecialType != SpecialType.System_String; + => (sym.ImplementsInterface(symbols.EnumerableSymbol) || SymbolEqualityComparer.Default.Equals(sym, symbols.EnumerableSymbol)) + && sym.SpecialType != SpecialType.System_String; internal static bool ImplementsIConvertible(this ITypeSymbol sym, SymbolHolder symbols) { @@ -56,7 +57,7 @@ internal static bool ImplementsIFormattable(this ITypeSymbol sym, SymbolHolder s } internal static bool ImplementsISpanFormattable(this ITypeSymbol sym, SymbolHolder symbols) - => symbols.SpanFormattableSymbol != null && sym.ImplementsInterface(symbols.SpanFormattableSymbol); + => symbols.SpanFormattableSymbol != null && (sym.ImplementsInterface(symbols.SpanFormattableSymbol) || SymbolEqualityComparer.Default.Equals(sym, symbols.SpanFormattableSymbol)); internal static bool IsSpecialType(this ITypeSymbol typeSymbol, SymbolHolder symbols) => typeSymbol.SpecialType != SpecialType.None || @@ -64,4 +65,21 @@ internal static bool IsSpecialType(this ITypeSymbol typeSymbol, SymbolHolder sym #pragma warning disable RS1024 symbols.IgnorePropertiesSymbols.Contains(typeSymbol); #pragma warning restore RS1024 + + internal static bool HasCustomToString(this ITypeSymbol type) + { + ITypeSymbol? current = type; + while (current != null && current.SpecialType != SpecialType.System_Object) + { + if (current.GetMembers("ToString").Where(m => m.Kind == SymbolKind.Method && m.DeclaredAccessibility == Accessibility.Public).Cast().Any(m => m.Parameters.Length == 0)) + { + return true; + } + + current = current.BaseType; + } + + return false; + } + } diff --git a/src/Libraries/Microsoft.Extensions.Telemetry.Abstractions/Logging/LogPropertiesAttribute.cs b/src/Libraries/Microsoft.Extensions.Telemetry.Abstractions/Logging/LogPropertiesAttribute.cs index 80ed7b9a058..c2d6a65cd38 100644 --- a/src/Libraries/Microsoft.Extensions.Telemetry.Abstractions/Logging/LogPropertiesAttribute.cs +++ b/src/Libraries/Microsoft.Extensions.Telemetry.Abstractions/Logging/LogPropertiesAttribute.cs @@ -3,12 +3,14 @@ using System; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using Microsoft.Extensions.Logging; +using Microsoft.Shared.DiagnosticIds; namespace Microsoft.Extensions.Logging; /// -/// Marks a logging method parameter whose public tags need to be logged. +/// Marks a logging method parameter whose public properties need to be logged as log tags. /// /// [AttributeUsage(AttributeTargets.Parameter | AttributeTargets.Property)] @@ -16,7 +18,7 @@ namespace Microsoft.Extensions.Logging; public sealed class LogPropertiesAttribute : Attribute { /// - /// Gets or sets a value indicating whether tags are logged. + /// Gets or sets a value indicating whether properties are logged. /// /// /// Defaults to . @@ -30,4 +32,19 @@ public sealed class LogPropertiesAttribute : Attribute /// Defaults to . /// public bool OmitReferenceName { get; set; } + + /// + /// Gets or sets a value indicating whether to transitively visit properties which are complex objects. + /// + /// + /// When logging the properties of an object, this property controls the behavior for each encountered property. + /// When this property is , then each property is serialized by calling to + /// generate a string for the property. When this property is , then each property of any complex objects are + /// expanded individually. + /// + /// + /// Defaults to . + /// + [Experimental(diagnosticId: DiagnosticIds.Experiments.Telemetry, UrlFormat = DiagnosticIds.UrlFormat)] + public bool Transitive { get; set; } } diff --git a/src/Libraries/Microsoft.Extensions.Telemetry.Abstractions/Logging/TagNameAttribute.cs b/src/Libraries/Microsoft.Extensions.Telemetry.Abstractions/Logging/TagNameAttribute.cs new file mode 100644 index 00000000000..05d1dc00fca --- /dev/null +++ b/src/Libraries/Microsoft.Extensions.Telemetry.Abstractions/Logging/TagNameAttribute.cs @@ -0,0 +1,34 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using Microsoft.Shared.DiagnosticIds; +using Microsoft.Shared.Diagnostics; + +namespace Microsoft.Extensions.Logging; + +/// +/// Defines the tag name to use for a logged parameter or property. +/// +/// +[AttributeUsage(AttributeTargets.Parameter | AttributeTargets.Property)] +[Conditional("CODE_GENERATION_ATTRIBUTES")] +[Experimental(diagnosticId: DiagnosticIds.Experiments.Telemetry, UrlFormat = DiagnosticIds.UrlFormat)] +public sealed class TagNameAttribute : Attribute +{ + /// + /// Initializes a new instance of the class. + /// + /// The tag name to use when logging the annotated parameter or property. + public TagNameAttribute(string name) + { + Name = Throw.IfNull(name); + } + + /// + /// Gets the name of the tag to be used when logging the parameter or property. + /// + public string Name { get; } +} diff --git a/src/Shared/DiagnosticIds/DiagnosticIds.cs b/src/Shared/DiagnosticIds/DiagnosticIds.cs index dbc82065488..da7d80a43aa 100644 --- a/src/Shared/DiagnosticIds/DiagnosticIds.cs +++ b/src/Shared/DiagnosticIds/DiagnosticIds.cs @@ -28,32 +28,6 @@ internal static class ContextualOptions internal const string CTXOPTGEN003 = nameof(CTXOPTGEN003); } - internal static class Design - { - internal const string AUTOCLIENTGEN001 = nameof(AUTOCLIENTGEN001); - internal const string AUTOCLIENTGEN002 = nameof(AUTOCLIENTGEN002); - internal const string AUTOCLIENTGEN003 = nameof(AUTOCLIENTGEN003); - internal const string AUTOCLIENTGEN004 = nameof(AUTOCLIENTGEN004); - internal const string AUTOCLIENTGEN005 = nameof(AUTOCLIENTGEN005); - internal const string AUTOCLIENTGEN006 = nameof(AUTOCLIENTGEN006); - internal const string AUTOCLIENTGEN007 = nameof(AUTOCLIENTGEN007); - internal const string AUTOCLIENTGEN008 = nameof(AUTOCLIENTGEN008); - internal const string AUTOCLIENTGEN009 = nameof(AUTOCLIENTGEN009); - internal const string AUTOCLIENTGEN010 = nameof(AUTOCLIENTGEN010); - internal const string AUTOCLIENTGEN011 = nameof(AUTOCLIENTGEN011); - internal const string AUTOCLIENTGEN012 = nameof(AUTOCLIENTGEN012); - internal const string AUTOCLIENTGEN013 = nameof(AUTOCLIENTGEN013); - internal const string AUTOCLIENTGEN014 = nameof(AUTOCLIENTGEN014); - internal const string AUTOCLIENTGEN015 = nameof(AUTOCLIENTGEN015); - internal const string AUTOCLIENTGEN016 = nameof(AUTOCLIENTGEN016); - internal const string AUTOCLIENTGEN017 = nameof(AUTOCLIENTGEN017); - internal const string AUTOCLIENTGEN018 = nameof(AUTOCLIENTGEN018); - internal const string AUTOCLIENTGEN019 = nameof(AUTOCLIENTGEN019); - internal const string AUTOCLIENTGEN020 = nameof(AUTOCLIENTGEN020); - internal const string AUTOCLIENTGEN021 = nameof(AUTOCLIENTGEN021); - internal const string AUTOCLIENTGEN022 = nameof(AUTOCLIENTGEN022); - } - /// /// Experiments supported by this repo. /// @@ -63,7 +37,6 @@ internal static class Experiments internal const string Compliance = "EXTEXP0002"; internal const string Telemetry = "EXTEXP0003"; internal const string TimeProvider = "EXTEXP0004"; - internal const string AutoClient = "EXTEXP0005"; internal const string AsyncState = "EXTEXP0006"; internal const string HealthChecks = "EXTEXP0007"; internal const string ResourceMonitoring = "EXTEXP0008"; @@ -112,6 +85,7 @@ internal static class LoggerMessage internal const string LOGGEN033 = nameof(LOGGEN033); internal const string LOGGEN034 = nameof(LOGGEN034); internal const string LOGGEN035 = nameof(LOGGEN035); + internal const string LOGGEN036 = nameof(LOGGEN036); } internal static class Metrics diff --git a/test/Generators/Microsoft.Gen.Logging/Generated/TagNameTests.cs b/test/Generators/Microsoft.Gen.Logging/Generated/TagNameTests.cs new file mode 100644 index 00000000000..fd9c5f6cba2 --- /dev/null +++ b/test/Generators/Microsoft.Gen.Logging/Generated/TagNameTests.cs @@ -0,0 +1,28 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using FluentAssertions; +using Microsoft.Extensions.Logging.Testing; +using TestClasses; +using Xunit; + +namespace Microsoft.Gen.Logging.Test; + +public class TagNameTests +{ + [Fact] + public void Basic() + { + var logger = new FakeLogger(); + + TagNameExtensions.M0(logger, 0); + + var expectedState = new Dictionary + { + ["TN1"] = "0", + }; + + logger.Collector.LatestRecord.StructuredState.Should().NotBeNull().And.Equal(expectedState); + } +} diff --git a/test/Generators/Microsoft.Gen.Logging/Generated/TransitiveTests.cs b/test/Generators/Microsoft.Gen.Logging/Generated/TransitiveTests.cs new file mode 100644 index 00000000000..6540825dd90 --- /dev/null +++ b/test/Generators/Microsoft.Gen.Logging/Generated/TransitiveTests.cs @@ -0,0 +1,40 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using FluentAssertions; +using Microsoft.Extensions.Logging.Testing; +using TestClasses; +using Xunit; + +namespace Microsoft.Gen.Logging.Test; + +public class TransitiveTests +{ + [Fact] + public void Basic() + { + var logger = new FakeLogger(); + var c = new TransitiveTestExtensions.C0(); + + TransitiveTestExtensions.M0(logger, c); + + var expectedState = new Dictionary + { + ["p0.P1"] = "V1", + ["p0.P0.P2"] = "V2", + }; + + logger.Collector.LatestRecord.StructuredState.Should().NotBeNull().And.Equal(expectedState); + + TransitiveTestExtensions.M1(logger, c); + + expectedState = new Dictionary + { + ["p0.P1"] = "V1", + ["p0.P0"] = "TS1", + }; + + logger.Collector.LatestRecord.StructuredState.Should().NotBeNull().And.Equal(expectedState); + } +} diff --git a/test/Generators/Microsoft.Gen.Logging/TestClasses/LogPropertiesSpecialTypesExtensions.cs b/test/Generators/Microsoft.Gen.Logging/TestClasses/LogPropertiesSpecialTypesExtensions.cs index f8b16489ce6..45239d41b8a 100644 --- a/test/Generators/Microsoft.Gen.Logging/TestClasses/LogPropertiesSpecialTypesExtensions.cs +++ b/test/Generators/Microsoft.Gen.Logging/TestClasses/LogPropertiesSpecialTypesExtensions.cs @@ -19,7 +19,9 @@ internal class MyProps public Version? P4 { get; set; } public Uri? P5 { get; set; } public IPAddress? P6 { get; set; } +#pragma warning disable LOGGEN036 public EndPoint? P7 { get; set; } +#pragma warning restore LOGGEN036 public IPEndPoint? P8 { get; set; } public DnsEndPoint? P9 { get; set; } public BigInteger P10 { get; set; } diff --git a/test/Generators/Microsoft.Gen.Logging/TestClasses/TagNameExtensions.cs b/test/Generators/Microsoft.Gen.Logging/TestClasses/TagNameExtensions.cs new file mode 100644 index 00000000000..d1d4329041e --- /dev/null +++ b/test/Generators/Microsoft.Gen.Logging/TestClasses/TagNameExtensions.cs @@ -0,0 +1,13 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Extensions.Logging; + +namespace TestClasses +{ + internal static partial class TagNameExtensions + { + [LoggerMessage(LogLevel.Warning)] + internal static partial void M0(ILogger logger, [TagName("TN1")] int p0); + } +} diff --git a/test/Generators/Microsoft.Gen.Logging/TestClasses/TransitiveTestExtensions.cs b/test/Generators/Microsoft.Gen.Logging/TestClasses/TransitiveTestExtensions.cs new file mode 100644 index 00000000000..964918ae9df --- /dev/null +++ b/test/Generators/Microsoft.Gen.Logging/TestClasses/TransitiveTestExtensions.cs @@ -0,0 +1,28 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Extensions.Logging; + +namespace TestClasses +{ + internal static partial class TransitiveTestExtensions + { + public class C0 + { + public C1 P0 { get; set; } = new C1(); + public string P1 { get; set; } = "V1"; + } + + public class C1 + { + public string P2 { get; set; } = "V2"; + public override string ToString() => "TS1"; + } + + [LoggerMessage(LogLevel.Debug)] + public static partial void M0(ILogger logger, [LogProperties(Transitive = true)] C0 p0); + + [LoggerMessage(LogLevel.Debug)] + public static partial void M1(ILogger logger, [LogProperties(Transitive = false)] C0 p0); + } +} diff --git a/test/Generators/Microsoft.Gen.Logging/Unit/EmitterTests.cs b/test/Generators/Microsoft.Gen.Logging/Unit/EmitterTests.cs index e910a99149e..c2075b51f8e 100644 --- a/test/Generators/Microsoft.Gen.Logging/Unit/EmitterTests.cs +++ b/test/Generators/Microsoft.Gen.Logging/Unit/EmitterTests.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Numerics; using System.Reflection; using System.Threading.Tasks; using Microsoft.Extensions.Compliance.Classification; @@ -44,6 +45,7 @@ public async Task TestEmitter() Assembly.GetAssembly(typeof(DataClassification))!, Assembly.GetAssembly(typeof(IRedactorProvider))!, Assembly.GetAssembly(typeof(PrivateDataAttribute))!, + Assembly.GetAssembly(typeof(BigInteger))!, }, sources, symbols) diff --git a/test/Generators/Microsoft.Gen.Logging/Unit/EmitterUtilsTests.cs b/test/Generators/Microsoft.Gen.Logging/Unit/EmitterUtilsTests.cs index 88fee7d7739..b4e2652244e 100644 --- a/test/Generators/Microsoft.Gen.Logging/Unit/EmitterUtilsTests.cs +++ b/test/Generators/Microsoft.Gen.Logging/Unit/EmitterUtilsTests.cs @@ -58,7 +58,7 @@ public void ShouldFindLogLevelFromParameter() lm.Parameters.Add(new LoggingMethodParameter { IsLogLevel = true, - Name = ParamName + ParameterName = ParamName }); Assert.Equal(ParamName, Emitter.GetLoggerMethodLogLevel(lm)); diff --git a/test/Generators/Microsoft.Gen.Logging/Unit/LogParserUtilitiesTests.cs b/test/Generators/Microsoft.Gen.Logging/Unit/LogParserUtilitiesTests.cs index 1db966137c3..0647db0791b 100644 --- a/test/Generators/Microsoft.Gen.Logging/Unit/LogParserUtilitiesTests.cs +++ b/test/Generators/Microsoft.Gen.Logging/Unit/LogParserUtilitiesTests.cs @@ -64,6 +64,7 @@ public void RecordHasSensitivePublicMembers_ShouldReturnFalse_WhenNoDataClasses( null!, null!, null!, + null!, null!); var diagMock = new Mock>(); @@ -110,6 +111,7 @@ public void RecordHasSensitivePublicMembers_ShouldNotThrow_WhenNoMembersOnType(b null!, null!, null!, + null!, Mock.Of()); var diagMock = new Mock>(); @@ -134,6 +136,7 @@ public void ProcessLogPropertiesForParameter_ShouldNotThrow_WhenNoMembersOnType( null!, null!, null!, + null!, new HashSet(SymbolEqualityComparer.Default), null!, null!, diff --git a/test/Generators/Microsoft.Gen.Logging/Unit/LoggingMethodParameterTests.cs b/test/Generators/Microsoft.Gen.Logging/Unit/LoggingMethodParameterTests.cs index 34242fba9eb..1820f402246 100644 --- a/test/Generators/Microsoft.Gen.Logging/Unit/LoggingMethodParameterTests.cs +++ b/test/Generators/Microsoft.Gen.Logging/Unit/LoggingMethodParameterTests.cs @@ -14,7 +14,7 @@ public class LoggingMethodParameterTests public void Fields_Should_BeInitialized() { var instance = new LoggingMethodParameter(); - Assert.Empty(instance.Name); + Assert.Empty(instance.ParameterName); Assert.Empty(instance.Type); } @@ -57,13 +57,13 @@ public void Misc() { var lp = new LoggingMethodParameter { - Name = "Foo", + ParameterName = "Foo", NeedsAtSign = false, }; - Assert.Equal(lp.Name, lp.NameWithAt); + Assert.Equal(lp.ParameterName, lp.ParameterNameWithAt); lp.NeedsAtSign = true; - Assert.Equal("@" + lp.Name, lp.NameWithAt); + Assert.Equal("@" + lp.ParameterName, lp.ParameterNameWithAt); lp.Type = "Foo"; lp.IsReference = false; diff --git a/test/Generators/Microsoft.Gen.Logging/Unit/LoggingMethodTests.cs b/test/Generators/Microsoft.Gen.Logging/Unit/LoggingMethodTests.cs index 5c7ff5533a4..2d92e4d9b4c 100644 --- a/test/Generators/Microsoft.Gen.Logging/Unit/LoggingMethodTests.cs +++ b/test/Generators/Microsoft.Gen.Logging/Unit/LoggingMethodTests.cs @@ -17,22 +17,4 @@ public void Fields_Should_BeInitialized() Assert.Empty(instance.Modifiers); Assert.Equal("_logger", instance.LoggerMember); } - - [Fact] - public void ShouldReturnParameterNameIfNotFoundInMap() - { - var p = new LoggingMethodParameter { Name = "paramName" }; - var method = new LoggingMethod(); - Assert.Equal(p.Name, method.GetParameterNameInTemplate(p)); - } - - [Fact] - public void ShouldReturnNameForParameterFromMap() - { - var p = new LoggingMethodParameter { Name = "paramName" }; - var method = new LoggingMethod(); - method.TemplateToParameterName[p.Name] = "Name from the map"; - - Assert.Equal(method.TemplateToParameterName[p.Name], method.GetParameterNameInTemplate(p)); - } } diff --git a/test/Generators/Microsoft.Gen.Logging/Unit/ParserTests.LogProperties.cs b/test/Generators/Microsoft.Gen.Logging/Unit/ParserTests.LogProperties.cs index 98d30ccdd99..a5e663dc57e 100644 --- a/test/Generators/Microsoft.Gen.Logging/Unit/ParserTests.LogProperties.cs +++ b/test/Generators/Microsoft.Gen.Logging/Unit/ParserTests.LogProperties.cs @@ -53,11 +53,11 @@ class MyType partial class C { - [LoggerMessage(0, LogLevel.Debug, ""Parameterless..."")] - static partial void M0(ILogger logger, [LogProperties(OmitReferenceName = true)] MyType /*0+*/p0/*-0*/); + [LoggerMessage(LogLevel.Debug)] + static partial void M0(ILogger logger, int p0, [LogProperties(OmitReferenceName = true)] MyType /*0+*/p1/*-0*/); }"; - await RunGenerator(Source, DiagDescriptors.LogPropertiesNameCollision); + await RunGenerator(Source, DiagDescriptors.TagNameCollision); } [Fact] @@ -207,7 +207,7 @@ partial class C static partial void M(ILogger logger, string param, string /*0+*/Param/*-0*/); }"; - await RunGenerator(Source, DiagDescriptors.LogPropertiesNameCollision); + await RunGenerator(Source, DiagDescriptors.TagNameCollision); } [Fact] @@ -217,15 +217,21 @@ public async Task LogPropertiesNameCollision() class MyClass { public int A { get; set; } + + [LogPropertyIgnore] + public int B { get; set; } } partial class C { - [LoggerMessage(0, LogLevel.Debug, ""{param_A}"")] - static partial void M(ILogger logger, string param_A, [LogProperties] MyClass /*0+*/param/*-0*/); + [LoggerMessage(LogLevel.Debug)] + static partial void M0(ILogger logger, string param_A, [LogProperties] MyClass /*0+*/param/*-0*/); + + [LoggerMessage(LogLevel.Debug)] + static partial void M1(ILogger logger, string param_B, [LogProperties] MyClass param); }"; - await RunGenerator(Source, DiagDescriptors.LogPropertiesNameCollision); + await RunGenerator(Source, DiagDescriptors.TagNameCollision); } [Fact] @@ -251,7 +257,7 @@ partial class C static partial void M(ILogger logger, [LogProperties] MyClass /*0+*/param/*-0*/); }"; - await RunGenerator(Source, DiagDescriptors.LogPropertiesNameCollision); + await RunGenerator(Source, DiagDescriptors.TagNameCollision); } [Theory] @@ -387,4 +393,44 @@ partial class LoggerClass await RunGenerator(Source, DiagDescriptors.LogPropertiesHiddenPropertyDetected); } + + [Fact] + public async Task DefaultToString() + { + await RunGenerator(@" + record class MyRecordClass(int x); + record struct MyRecordStruct(int x); + + class MyClass2 + { + } + + class MyClass3 + { + public override string ToString() => ""FIND ME!""; + } + + class MyClass + { + public object /*0+*/P0/*-0*/ { get; set; } + public MyClass2 /*1+*/P1/*-1*/ { get; set; } + public MyClass3 P2 { get; set; } + public int P3 { get; set; } + public System.Numerics.BigInteger P4 { get; set; } + public T P5 { get; set; } + } + + partial class C + { + [LoggerMessage(LogLevel.Debug)] + static partial void M0(this ILogger logger, + object /*2+*/p0/*-2*/, + MyClass2 /*3+*/p1/*-3*/, + MyClass3 p2, + [LogProperties] MyClass p3, + T p4, + MyRecordClass p5, + MyRecordStruct p6); + }", DiagDescriptors.DefaultToString); + } } diff --git a/test/Generators/Microsoft.Gen.Logging/Unit/ParserTests.cs b/test/Generators/Microsoft.Gen.Logging/Unit/ParserTests.cs index d2824eb28cc..80d401b7ff3 100644 --- a/test/Generators/Microsoft.Gen.Logging/Unit/ParserTests.cs +++ b/test/Generators/Microsoft.Gen.Logging/Unit/ParserTests.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Numerics; using System.Reflection; using System.Threading; using System.Threading.Tasks; @@ -1005,6 +1006,7 @@ private static async Task RunGenerator( Assembly.GetAssembly(typeof(IEnrichmentTagCollector))!, Assembly.GetAssembly(typeof(DataClassification))!, Assembly.GetAssembly(typeof(PrivateDataAttribute))!, + Assembly.GetAssembly(typeof(BigInteger))!, }; } diff --git a/test/Libraries/Microsoft.Extensions.Telemetry.Abstractions.Tests/Logging/LogPropertiesAttributeTests.cs b/test/Libraries/Microsoft.Extensions.Telemetry.Abstractions.Tests/Logging/LogPropertiesAttributeTests.cs index b3efaaffa31..cedab2d23ba 100644 --- a/test/Libraries/Microsoft.Extensions.Telemetry.Abstractions.Tests/Logging/LogPropertiesAttributeTests.cs +++ b/test/Libraries/Microsoft.Extensions.Telemetry.Abstractions.Tests/Logging/LogPropertiesAttributeTests.cs @@ -26,4 +26,14 @@ public void OmiReferenceName() lpa.OmitReferenceName = true; Assert.True(lpa.OmitReferenceName); } + + [Fact] + public void Transitive() + { + var lpa = new LogPropertiesAttribute(); + Assert.False(lpa.Transitive); + + lpa.Transitive = true; + Assert.True(lpa.Transitive); + } } diff --git a/test/Libraries/Microsoft.Extensions.Telemetry.Abstractions.Tests/Logging/TagNameAttributeTests.cs b/test/Libraries/Microsoft.Extensions.Telemetry.Abstractions.Tests/Logging/TagNameAttributeTests.cs new file mode 100644 index 00000000000..49cccd6cf3b --- /dev/null +++ b/test/Libraries/Microsoft.Extensions.Telemetry.Abstractions.Tests/Logging/TagNameAttributeTests.cs @@ -0,0 +1,19 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using Xunit; + +namespace Microsoft.Extensions.Logging.Test; + +public class TagNameAttributeTests +{ + [Fact] + public void Basic() + { + var a = new TagNameAttribute("a"); + Assert.Equal("a", a.Name); + + Assert.Throws(() => new TagNameAttribute(null!)); + } +} From da5693b6191b964c2864b1b7b38f3fb00e31cab0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Ros?= Date: Wed, 27 Dec 2023 16:33:10 -0800 Subject: [PATCH 4/4] Remove ExperimentalAttribute from previous merge conflict --- .../RedactionExtensions.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Libraries/Microsoft.Extensions.Compliance.Redaction/RedactionExtensions.cs b/src/Libraries/Microsoft.Extensions.Compliance.Redaction/RedactionExtensions.cs index 036d3a33736..191d8af6f55 100644 --- a/src/Libraries/Microsoft.Extensions.Compliance.Redaction/RedactionExtensions.cs +++ b/src/Libraries/Microsoft.Extensions.Compliance.Redaction/RedactionExtensions.cs @@ -50,7 +50,6 @@ public static IRedactionBuilder SetHmacRedactor(this IRedactionBuilder builder, "Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = "Addressed with [DynamicDependency]")] - [Experimental(diagnosticId: DiagnosticIds.Experiments.Compliance, UrlFormat = DiagnosticIds.UrlFormat)] public static IRedactionBuilder SetHmacRedactor(this IRedactionBuilder builder, IConfigurationSection section, params DataClassificationSet[] classifications) { _ = Throw.IfNull(builder);