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

Make possible of RequestLoggingMiddleware to Log Headers as optional #343

Open
rodrigopinto opened this issue Aug 2, 2023 · 1 comment

Comments

@rodrigopinto
Copy link

rodrigopinto commented Aug 2, 2023

Is your feature request related to a problem? Please describe.
No.

Describe the solution you'd like
The RequestLoggingMiddleware.cs has its standard HTTP request attributes that are logged for every request. The HTTP Headers from the request/response are not part of it.

Considering asp net core 7, the standard HTTP logging allows to add headers to the logging middleware and this is not available on Serilog middleware.

Would be great to have Headers as optional, to be added to the list of fields logged. It could be disabled by default to be backward compatible.

For that work, would require adding to RequestLoggingOptions the Headers attribute with some sort of configuration similarly the standard HTTP logging has.

Below is a pseudo-code, just to give an idea.

//... PSEUDO CODE

// RequesHeaders allow list should be configured on the `UseSerilogRequestLogging` call.
public List<RequestHeaders> RequestHeaders { get; set; }

static LogEventProperty GetHeadersProperties(){
   var requestHeaders = httpContext?.Request?.Headers;
   
   List<object> headers = [];
   
   foreach (var headerName in RequestHeaders()) {
      if (requestHeaders.TryGetValue(headerName, out var headerValue))
      {
         headers.Add(headerName, headerValue)
      }
 
  }
  
  new LogEventProperty("Headers", new ScalarValue(headers)),
}

public RequestLoggingOptions()
    {
        GetLevel = DefaultGetLevel;
        MessageTemplate = DefaultRequestCompletionMessageTemplate;
        GetMessageTemplateProperties = DefaultGetMessageTemplateProperties;
        HeadersProperties = GetHeadersProperties;
    }

The configuration of which headers would be logged should happen on the UseSerilogRequestLogging. Similar to the template that can be customized, the headers list could configured the allow list no the same moment.

Describe alternatives you've considered

Using the Enricher is an option but not ideal, as Enrichers are applied any time you use a logger instance. Also, it is possible to make EnrichDiagnosticContext on the configuration of the middleware, but I think this logic could be part of the standard of the library instead of putting some logic on the builder setup.

@nblumhardt
Copy link
Member

Makes sense, thanks for the suggestion.

Currently, I believe you can actually do this using GetMessageTemplateProperties() - since the returned values are named rather than positional, you could pull out a Headers property there, alongside whatever the template needs, and have it attached to the result (without any impact on the message).

Seems like this could be improved/streamlined a bit, e.g. by replacing GetMessageTemplateProperties with GetCompletionEventProperties or similar. Needs a little bit of thought 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants