aspnetcore icon indicating copy to clipboard operation
aspnetcore copied to clipboard

[Hosting] Add {OriginalFormat} to start & stop log state objects

Open CodeBlanch opened this issue 2 years ago • 2 comments

Description

The ILogger extensions and source generator return the original template logged as the {OriginalFormat} key on the IReadOnlyList<KeyValuePair<string, object>> state representing the log being written.

AspNetCore has a couple custom log states which do not mirror the {OriginalFormat} behavior. By default OpenTelemetry only captures message templates but for these logs it must either have empty template/body (confusing for users) or invoke ToString (and incur allocations) to generate a formatted string because no template is available.

In order to make these high value logs play nicely with structured logging and to enable high performance capture I'm proposing we should mirror the {OriginalFormat} behavior.

Changes

  • Returns {OrignalFormat} as the last key on HostingRequestStartingLog & HostingRequestFinishedLog state objects
  • Tweaks the log messages to be more consistent & deterministic...
    • Start log now writes content type & length after a "-" character
    • Stop log no longer writes out request content type & length

Final templates look like this:

Start: Request starting {Protocol} {Method} {Scheme}://{Host}{PathBase}{Path}{QueryString} - {ContentType} {ContentLength}

Stop: Request finished {Protocol} {Method} {Scheme}://{Host}{PathBase}{Path}{QueryString} - {StatusCode} {ContentLength} {ContentType} {ElapsedMilliseconds}ms

/cc @davidfowl @tarekgh @noahfalk @cijothomas @utpilla @reyang

CodeBlanch avatar Nov 23 '22 21:11 CodeBlanch

Thanks for your PR, @CodeBlanch. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

ghost avatar Nov 23 '22 21:11 ghost

If we're going to do this, we should do it for all our custom logging: https://grep.app/search?q=IReadOnlyList%3CKeyValuePair%3Cstring%2C%20object%3F%3E%3E&filter[repo][0]=dotnet/aspnetcore

BrennanConroy avatar Nov 23 '22 23:11 BrennanConroy

@BrennanConroy Thanks for the link! I don't think {OriginalFormat} is really needed on scopes. HttpRequestLog & HttpResponseLog probably need it, but not sure what the template would be exactly. In any case, happy to look into those things if this goes in 👍

CodeBlanch avatar Nov 28 '22 19:11 CodeBlanch

cc @samsp-msft

adityamandaleeka avatar Dec 06 '22 01:12 adityamandaleeka