dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.19k stars 4.72k forks source link

[API Proposal]: Enhance LoggerMessage attribute to support logging complex object and redaction #81730

Open dpk83 opened 1 year ago

dpk83 commented 1 year ago

Background and motivation

.NET uses source generation to provide high performance logging via LoggerMessage attribute. Source generation opens up a lot of flexibility to do more things in an efficient manner. We have a telemetry solution that is used by services across our orgs, as part of our solution we have expanded the LoggerMessage to support the following features

API Proposal

Introduce LogPropertiesAttribute which is used to annotate an object that needs to be expanded for logging.


[AttributeUsage(AttributeTargets.Parameter)]
[Conditional("CODE_GENERATION_ATTRIBUTES")]
public sealed class LogPropertiesAttribute : Attribute
{
  ...
}

API Usage


public class Operation
{
  public string Id {get; set;}
  public string Name {get; set;}
}

public class ErrorDetails
{
   public Operation Operation {get; set;}
   public ErrorType Type {get; set;}
   public string ErrorMessage {get; set;}
}

public static partial class Log
{
    [LoggerMessage(
        EventId = 0,
        Level = LogLevel.Error,
        Message = "Could not open socket")]
    public static partial void CouldNotOpenSocket(
        this ILogger logger, [LogProperties] error);
}

// log the error
var errorDetails = ...
logger.CouldNotOpenSocket(error);

This will result in all parameters of the error details logged as error_Operation_Id, error_Operation_Name, error_Type, error_ErrorMessage.

This is a simplistic example. LogProperties can have parameters that extends its functionality with more options.

The above can be augmented via some attributes to indicate sensitive data so the generated code takes care of redacting it appropriately.

public static partial class Log
{
    [LoggerMessage(
        EventId = 0,
        Level = LogLevel.Information,
        Message = "Fetching profile failed for {user}")]
    public static partial void ProfileFetchFailed(
        this ILogger logger, IRedactorProvider redactorProvider, [XYZ] user); // XYZ here is a placeholder for the data classification attribute, I am intentionally omitting the real attributes
}

// Generated code will use redactorProvider to redact the `userId` according to the data class XYZ
// Usage is
logger.ProfileFetchFailed(redactorProvider, userId);

Similarly the attribute XYZ can be added to the members of the complex object and the generated code should be able to know and redact it.

Alternative Designs

No response

Risks

No response

ghost commented 1 year ago

Tagging subscribers to this area: @dotnet/area-extensions-logging See info in area-owners.md if you want to be subscribed.

Issue Details
### Background and motivation .NET uses source generation to provide high performance logging via LoggerMessage attribute. Source generation opens up a lot of flexibility to do more things in an efficient manner. We have a telemetry solution that is used by services across our orgs, as part of our solution we have expanded the LoggerMessage to support the following features - **Logging of complex object** i.e. If you have an object let's say ErrorDetails and developer needs to log the details of the error. Today it is achieved by individually logging all members of the object e.g. `logger.Log("Failed to fetch data due to error: {0}, {1}, {2}", errorDetails.operationId, errorDetails.Type, errorDetails.Message);`. Instead with the support of complex object logging in LoggerMessage attribute developer can log the object directly i.e. `logger.DataFetchFailed(errorDetails)`. This will perform the expansion of the errorDetails object as part of the compile time source generation in an efficient way (i.e. no runtime cost) - **Redacting sensitive data**: Services has need to not log sensitive data in telemetry. Today developers need to redact the data before the information is logged and in order to redact the data they need to know which fields are redacted and copy over the logic to redact it. Often times the redaction is not done in a performant way. ### API Proposal Introduce `LogPropertiesAttribute` which is used to annotate an object that needs to be expanded for logging. ```csharp [AttributeUsage(AttributeTargets.Parameter)] [Conditional("CODE_GENERATION_ATTRIBUTES")] public sealed class LogPropertiesAttribute : Attribute { ... } ``` ### API Usage ```csharp public class Operation { public string Id {get; set;} public string Name {get; set;} } public class ErrorDetails { public Operation Operation {get; set;} public ErrorType Type {get; set;} public string ErrorMessage {get; set;} } public static partial class Log { [LoggerMessage( EventId = 0, Level = LogLevel.Error, Message = "Could not open socket")] public static partial void CouldNotOpenSocket( this ILogger logger, [LogProperties] error); } // log the error var errorDetails = ... logger.CouldNotOpenSocket(error); ``` This will result in all parameters of the error details logged as `error_Operation_Id`, `error_Operation_Name`, `error_Type`, `error_ErrorMessage`. This is a simplistic example. LogProperties can have parameters that extends its functionality with more options. The above can be augmented via some attributes to indicate sensitive data so the generated code takes care of redacting it appropriately. ```csharp public static partial class Log { [LoggerMessage( EventId = 0, Level = LogLevel.Information, Message = "Fetching profile failed for {user}")] public static partial void ProfileFetchFailed( this ILogger logger, IRedactorProvider redactorProvider, [XYZ] user); // XYZ here is a placeholder for the data classification attribute, I am intentionally omitting the real attributes } // Generated code will use redactorProvider to redact the `userId` according to the data class XYZ // Usage is logger.ProfileFetchFailed(redactorProvider, userId); ``` Similarly the attribute XYZ can be added to the members of the complex object and the generated code should be able to know and redact it. ### Alternative Designs _No response_ ### Risks _No response_
Author: dpk83
Assignees: -
Labels: `api-suggestion`, `untriaged`, `area-Extensions-Logging`
Milestone: -
dpk83 commented 1 year ago

@tarekgh @geeknoid

tarekgh commented 1 year ago

CC @reyang @CodeBlanch @davidfowl

tarekgh commented 1 year ago

LogProperties can have parameters that extends its functionality with more options.

Do we have more details about the behavior in general? This is a kind of serialization and needs to define the full scope of this serialization. For example, does it retrieve all properties which have getters either public or nonpublic inside the object? What do you expect to do when having types like collections (and which collections we should support)? What to expect if there is a cyclic reference? .... etc.

pinkfloydx33 commented 1 year ago

Much prefer structured logging support of @-prefix destructuring and then handling in your logging provider. For example Serilog supports this, as do other structured logging providers. If source-gen supported it (there's some open issues for it) it would be a matter of just adapting your logging provider, I think.

dpk83 commented 1 year ago

Do we have more details about the behavior in general? This is a kind of serialization and needs to define the full scope of this serialization. For example, does it retrieve all properties which have getters either public or nonpublic inside the object? What do you expect to do when having types like collections (and which collections we should support)? What to expect if there is a cyclic reference? .... etc.

@tarekgh

Much prefer structured logging support of @-prefix destructuring and then handling in your logging provider. For example Serilog supports this, as do other structured logging providers. If source-gen supported it (there's some open issues for it) it would be a matter of just adapting your logging provider, I think.

pinkfloydx33 Doing this in source generation provides the performance benefits as all of this expansion is done at the compile time as opposed the runtime. I am not saying that support for logging complex object shouldn't be added for the normal logging provider but with source generation we should utilize it for extracting the best performance out.

tarekgh commented 1 year ago

@dpk83 per discussion I moved this issue to the future release.