Skip to content

Can the HostingRequestFinishedLog.ToString include the request url too? #10097

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

Closed
kevinlo opened this issue May 8, 2019 · 7 comments · Fixed by #10102
Closed

Can the HostingRequestFinishedLog.ToString include the request url too? #10097

kevinlo opened this issue May 8, 2019 · 7 comments · Fixed by #10102
Labels
area-hosting Includes Hosting area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions

Comments

@kevinlo
Copy link

kevinlo commented May 8, 2019

HostingRequestFinishedLog.ToString currently just has the elapsedTime, but not the request URL.

"Request finished in {elapsedTime}ms {statusCode} {ContentType}",

It is easier to match with the Request starting if it can include the URL like this.

Request starting HTTP/1.1 GET http://myserver/myapi/myitem/123 [ 16984:92 ]
Request finished HTTP/1.1 GET http://myserver/myapi/myitem/123 in 501.525ms 200 application/json; charset=utf-8 [ 16984:98 ]

In my log files, sometimes it has a few Request finished around the same time and I cannot tell which one is which quickly without spending time to trace back the log.

This HostingRequestFinishedLog class is internal, I cannot find an easy way I can customize it to provide a custom ToString.

The HostingRequestStartingLog and HostingRequestFinishedLog are run in different thread, I cannot use the threadId to match the log.

@benaadams
Copy link
Member

This work? #10102

@kevinlo
Copy link
Author

kevinlo commented May 9, 2019

@benaadams. That works. Which version will have it?

@benaadams
Copy link
Member

Up to ASP.NET team 😄

@davidfowl
Copy link
Member

@benaadams. That works. Which version will have it?

If we take it, 3.0.

@kevinlo
Copy link
Author

kevinlo commented May 9, 2019

@davidfowl For 2.x, is there any alternative solution?

@benaadams
Copy link
Member

You could add your own middleware at the start of the pipeline and then log on the way out; see #5894 (comment)

@kevinlo
Copy link
Author

kevinlo commented May 9, 2019

@benaadams I know I can add my own middleware at the beginning of the pipeline. But then it will have duplicated logging as the Webhost one that I could not turn off. Moreover, I think the Webhost one provide more accurate elapsed from start to finish.

I just want to check if there is a better solution. If not, I can add my own Middleware for now.

Thanks all for your help.

@kevinlo kevinlo closed this as completed May 9, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
@amcasey amcasey added the area-hosting Includes Hosting label Jun 1, 2023
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-hosting Includes Hosting area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants