Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
dpk83 opened this issue Feb 6, 2023 · 7 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Extensions-Logging partner-impact This issue impacts a partner who needs to be kept updated
Milestone

Comments

@dpk83
Copy link

dpk83 commented Feb 6, 2023

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.

[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

@dpk83 dpk83 added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Feb 6, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 6, 2023
@ghost
Copy link

ghost commented Feb 6, 2023

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.

[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

Author: dpk83
Assignees: -
Labels:

api-suggestion, untriaged, area-Extensions-Logging

Milestone: -

@dpk83
Copy link
Author

dpk83 commented Feb 6, 2023

@tarekgh @geeknoid

@tarekgh tarekgh added this to the 8.0.0 milestone Feb 6, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Feb 6, 2023
@tarekgh tarekgh added untriaged New issue has not been triaged by the area owner partner-impact This issue impacts a partner who needs to be kept updated and removed untriaged New issue has not been triaged by the area owner labels Feb 6, 2023
@tarekgh
Copy link
Member

tarekgh commented Feb 6, 2023

CC @reyang @CodeBlanch @davidfowl

@tarekgh
Copy link
Member

tarekgh commented Feb 6, 2023

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
Copy link

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
Copy link
Author

dpk83 commented Feb 7, 2023

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

  • Retrieves all the public properties
  • Walks up and down the hierarchy i.e. base classes and containing objects
  • Having a limit to the depth it walks is something we don't have it yet but should be added
  • Attribute for ignoring a property from logging while logging complex object.
  • Here are few things that we generate errors on
    • cyclic reference detection
    • Applying attribute on non complex object
    • property name collision
    • There are more related to data classifications
    • There are checks that are related to the custom provider (where if custom provider constructor or required properties are not accessible etc.)
  • Ability to omit the parameter name prefix
  • Ability to provide the separator (currently we use _ as the default separator e.g. error_Operation_Id)

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
Copy link
Member

tarekgh commented Mar 26, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Extensions-Logging partner-impact This issue impacts a partner who needs to be kept updated
Projects
None yet
Development

No branches or pull requests

3 participants