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

HttpClientFactory logs - "StatusCode" property is inconsistent with other internal logs #1549

Closed
jschneider1207 opened this issue Apr 24, 2019 · 5 comments
Assignees

Comments

@jschneider1207
Copy link

jschneider1207 commented Apr 24, 2019

Info

Internal handlers used in the HttpClientFactory pattern, the LoggingHttpMessageHandler and the LoggingScopeHttpMessageHandler, create log messages after successful http requests in the following formats

LoggingHttpMessageHandler:

Received HTTP response after {ElapsedMilliseconds}ms - {StatusCode}

LoggingScopeHttpMessageHandler:

End processing HTTP request after {ElapsedMilliseconds}ms - {StatusCode}

An example of these are:

Received HTTP response after 56.0044ms - OK
End processing HTTP request after 70.0862ms - OK

As you can see, the StatusCode property is sent as the textual representation of the status code instead of the integer version (e.g. 200 instead of OK).

This conflicts with an internal log, the HostingRequestFinishedLog, which generates logs in the following format:

Request finished in {0}ms {1} {2}

An example of this is:

Request finished in 605.5877ms 200 application/json

Here, the StatusCode property is the integer representation of the status code and not the text like it is in the above two message handlers.

Problem

This conflict is causing an issue with dropped log messages in our system because we send our logs to Elasticsearch (via the Serilog sink), and Elasticsearch requires consistent types for properties with the same name.

In this case, if a HostingRequestFinishedLog log is sent first, Elasticsearch maps the StatusCode property to a long and all the logs from the two message handlers are discarded due to their StatusCode property being the wrong type. Vice versa if the message handlers get their log in first.

I think the message handler logs should be consistent with the internal framework logs and send their StatusCode property as an integer, it would resolve this conflict and I generally think the integer is more useful anyway. I'd be happy to submit a PR for this others agree.

@jschneider1207 jschneider1207 changed the title HttpClientFactory logs - Status Code property is inconsistent with other internal logs HttpClientFactory logs - "StatusCode" property is inconsistent with other internal logs Apr 24, 2019
@ghost
Copy link

ghost commented Nov 5, 2019

We have also identified this issue, so we would be very happy to see this completed.

@rynowak
Copy link
Member

rynowak commented Nov 6, 2019

Agreed, we should be consistent.

@rynowak rynowak added the bug label Nov 6, 2019
@rynowak rynowak added this to the 5.0.0 milestone Nov 6, 2019
@pranavkm pranavkm modified the milestones: 5.0.0, 5.0.0-preview1 Nov 25, 2019
@pranavkm
Copy link

The announcement should include possible workarounds in case users want to continue getting the older data type.

@rynowak
Copy link
Member

rynowak commented Nov 26, 2019

This has been merged. I'll work on the announcement.

@rynowak
Copy link
Member

rynowak commented Nov 26, 2019

Thanks for the feedback. This change has been merged for 5.0 and announced officially. If you want to get this behavior in 2.1+ you can follow the steps of the workaround described here, and customize the logging behavior to your liking.

@rynowak rynowak closed this as completed Nov 26, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants