Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Log request and response headers#592

Closed
JunTaoLuo wants to merge 1 commit into
devfrom
johluo/log-headers
Closed

Log request and response headers#592
JunTaoLuo wants to merge 1 commit into
devfrom
johluo/log-headers

Conversation

@JunTaoLuo

Copy link
Copy Markdown
Contributor

Item 2 of #4.

cc @CesarBS @halter73

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly need eventids or something more than just a string

@JunTaoLuo

Copy link
Copy Markdown
Contributor Author

I thought it was best to log the headers and such closer to when they are actually read and written but I suppose when the HttpContext was created/disposed is good enough. I'll close this PR and open a new one in Hosting and remove the logging of headers in WebListener.

@JunTaoLuo JunTaoLuo closed this Jan 21, 2016
@JunTaoLuo JunTaoLuo deleted the johluo/log-headers branch January 21, 2016 00:08
@halter73

Copy link
Copy Markdown
Member

@davidfowl I think it would be a good idea to expose a connection and request ID via the IHttpConnectionFeature and IHttpRequestFeature respectively. This should make it easier to correlate logs from different layers. I can open issues in Abstractions and Kestrel if you agree.

@davidfowl

Copy link
Copy Markdown
Member

IHttpConnectionFeature.ConnectionId I agree with but I don't think there should be a request id on the IHttpRequestFeature. I prefer what we have right now as it doesn't need to be implemented in the sever itself and it's not required for anything to function.

@Tratcher

Copy link
Copy Markdown
Member

I expect we'll need ConnectionId at some point. We already have IHttpRequestIdentifierFeature for request id. I agree that it would make correlation easier if the Kestrel implemented IHttpRequestIdentifierFeature. WebListener does.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants