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

Support access logging with Common Log Format/Extended Log Format #5894

Closed
muratg opened this issue Apr 7, 2017 · 44 comments
Closed

Support access logging with Common Log Format/Extended Log Format #5894

muratg opened this issue Apr 7, 2017 · 44 comments
Assignees
Labels
affected-medium This issue impacts approximately half of our customers area-hosting Includes Hosting area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one severity-minor This label is used by an internal tool Theme: meeting developer expectations
Milestone

Comments

@muratg
Copy link
Contributor

muratg commented Apr 7, 2017

@muratg
Copy link
Contributor Author

muratg commented Apr 7, 2017

From @Tratcher on February 3, 2017 18:13

Would this be better as middleware? Is there any information middleware wouldn't have access to?

@muratg
Copy link
Contributor Author

muratg commented Apr 7, 2017

From @CesarBS on February 3, 2017 18:14

With Kestrel at least, middleware wouldn't log bad requests rejected before the app runs.

@muratg
Copy link
Contributor Author

muratg commented Apr 7, 2017

@shirhatti @DamianEdwards thoughts?

@muratg
Copy link
Contributor Author

muratg commented Apr 7, 2017

From @DamianEdwards on February 3, 2017 21:16

This is interesting. I wasn't aware the log file format was a standard. Do we think we'd just do this internally in Kestrel or would we try to use ILoggerFactory somehow, e.g. have Kestrel support using its own ILoggerFactory instance which would have an ILoggerProvider configured for this format by default, but could add any other provider to it too via API.

@muratg
Copy link
Contributor Author

muratg commented Apr 7, 2017

@DamianEdwards Yeah, this is a simple format and there are a bunch of analysis tools since forever that can parse this format and provide information.

We discussed different options for implementing this (just doing it internally and writing a file, providing a stream that user can do whatever they want with, using the existing logging abstractions) -- each has pros and cons.

@muratg
Copy link
Contributor Author

muratg commented Apr 7, 2017

From @DamianEdwards on February 3, 2017 21:31

Would be good to capture the different approaches and their pros and cons.

@muratg
Copy link
Contributor Author

muratg commented Apr 7, 2017

From @CesarBS on February 3, 2017 21:35

@DamianEdwards The advantage of using existing log infrastructure is the flexibility and if people use Serilog they get log file rotation and such. The downside is that setting it up is more involved than just offering an option in KestrelServerOptions where they set the file name or Stream to write the logs to.

@muratg
Copy link
Contributor Author

muratg commented Apr 7, 2017

From @DamianEdwards on February 3, 2017 21:50

How would it work using ILogger? Would Kestrel do all the formatting itself internally and just log the messages through the current ILoggerFactory to whatever providers were configured? Is that enough to get the correct output?

@muratg
Copy link
Contributor Author

muratg commented Apr 7, 2017

From @CesarBS on February 17, 2017 0:10

Yeah, Kestrel would have to do the formatting. If a logger were to intercept the messages and decorate it in any way, that would make the message useless since it couldn't be parsed by tools that look into these logs.

@muratg
Copy link
Contributor Author

muratg commented Apr 7, 2017

From @halter73 on February 17, 2017 0:31

Decorated messages wouldn't exactly be useless, but they wouldn't fit the common log format. Presumably if you're using a logger that decorates messages, you aren't to interested in the common log format to begin with. There's also nothing stopping developers from configuring multiple logger providers: one that decorates and another that doesn't for common logs.

I think that common logs should have a unique source so providers can be configured to filter out everything else. Being able to use existing providers to get features like rolling log files, logging to some other persistence layer, etc. certainly would be nice.

@muratg
Copy link
Contributor Author

muratg commented Apr 7, 2017

From @benaadams on February 17, 2017 0:42

Is it an external component though? The while the layout is standard the output is configurable

For example IIS's options for W3C logging

The fields would have to be identifiable so they can be switched on and off

@muratg
Copy link
Contributor Author

muratg commented Apr 7, 2017

From @CesarBS on February 17, 2017 1:16

@benaadams Yeah, that's the extended format. The first W3C log format had fixed fields, but modern servers also support Extended Log Format which allows you to specify which fields you want logged per access.

We'd have to add a ServerOptions.AccessLogFormat option, like Apache's LogFormat directive: http://httpd.apache.org/docs/current/logs.html#accesslog

@muratg
Copy link
Contributor Author

muratg commented Apr 7, 2017

@shirhatti Needs design (though lower pri for 2.0.0)

@cwe1ss
Copy link
Contributor

cwe1ss commented May 14, 2017

How about doing this with a DiagnosticListener and one of the existing DiagnosticSource events (EndRequest, HttpRequestIn.Stop)? Requests rejected by the server (e.g. Kestrel) should call some method on Hosting that then raises another DiagnosticSource event - which would be interesting for many other use cases as well anyway.

@davidfowl
Copy link
Member

@cwe1ss That's the idea. The one thing we need to figure out is rejected requests that don't make it into hosting.

@Tratcher
Copy link
Member

@muratg at this point I assume this isn't happening for 2.0?

@muratg
Copy link
Contributor Author

muratg commented Aug 8, 2017

@glennc Could you please add this as a feature to track with the 2.1.0 project so that it gets prioritized with the rest of the features.

@aliostad
Copy link

This is a very basic feature that should have been in alpha. I understand that originally Kestrel was supposed to run behind proxy but now that it is supported as a standalone. This is especially important for us since we need to run on Service Fabric when no IIS is allowed near it.

@muratg
Copy link
Contributor Author

muratg commented Jun 14, 2018

@glennc this needs design.

@dfdumaresq
Copy link

Should this be mentioned in the 2.2 Roadmap? I don't see a mention of it in Announcements.
aspnet/Announcements#307

@DamianEdwards
Copy link
Member

It's not in scope for 2.2

@dfdumaresq
Copy link

Thanks, @DamianEdwards. When can we expect it?

@DamianEdwards DamianEdwards assigned shirhatti and unassigned glennc Jun 28, 2018
@DamianEdwards
Copy link
Member

Sorry, we have no details on that right now. It hasn't made it's way high enough on the priority list yet.

@dfdumaresq
Copy link

Okay. It is a high priority for us. I would venture to say a minimal implementation (i.e. common log format without extended logging) would be a great step forward.

@kwkaiwang
Copy link

As a workaround, I use NLog.config to filter 'Microsoft.AspNetCore.Hosting.Internal.WebHost' related entries:

<logger name="*" minlevel="Trace" writeTo="app" />
<logger name="Microsoft.AspNetCore.Hosting.Internal.WebHost" minlevel="Trace" writeTo="web" />

Sample output:
2018-09-10 23:07:31.7749,Info,WK-HOME,11844,1,Request starting HTTP/1.1 GET http://localhost:5000/api/folder
2018-09-10 23:07:31.7764,Info,WK-HOME,11844,2,Request finished in 5.3137ms 304
2018-09-10 23:07:33.1969,Info,WK-HOME,11844,1,Request starting HTTP/1.1 GET http://localhost:5000/sockjs-node/info?t=1536592053194
2018-09-10 23:07:33.1984,Info,WK-HOME,11844,2,Request finished in 9.6846ms 200 application/json; charset=UTF-8
2018-09-10 23:07:33.1984,Info,WK-HOME,11844,1,Request starting HTTP/1.1 GET http://localhost:5000/sockjs-node/659/fcfeqt2t/websocket

@JeepNL
Copy link
Contributor

JeepNL commented Sep 15, 2018

Hi @DamianEdwards,

You wrote Kestrel (post/request) logging is not high on the priority list for ASP.net Core team. Let me try to convince you otherwise.

a) For some time now you can use Kestrel as a 'stand alone' web server, without the need of a proxy server. It's fast and light weight. That's my setup and I'm loving it.

b) Post and Request logging (according to industry standards) is utterly important nowadays. You really need to have some easy way to check your log to see what's happening with your web server. Check IP addresses and check abuse.

I'm not talking about debugging, you have a couple of other solutions for what already, I'm talking about the safety and performance of my server.

Could you and your team please consider moving this issue higher up your priority list?

Many thanks for all of your hard work, your live videos etc. ASP.NET Core is what I've waited for all of my life. Since 1987 FidoNet :)

@aliostad
Copy link

@JeepNL extremely baffles me why they just don't get it.

@aspnet-hello aspnet-hello added enhancement This issue represents an ask for new feature or an enhancement to an existing one Needs: Design This issue requires design work before implementating. labels Dec 18, 2018
@keith-leung
Copy link

@JeepNL glad that worked for you. Please file issues in that repo for implementation questions so we can avoid distracting this thread.

I stared your GitHub repo, it's good.

@brianmed
Copy link

brianmed commented Dec 1, 2019

If you use Serilog, then you can try this Middleware:

@JeepNL
Copy link
Contributor

JeepNL commented Feb 14, 2020

Any new developments on this? Kestrel support for W3C extended log format out of the box? For .NET 5 maybe?

See also Microsoft Docs: W3C Logging

@ghost
Copy link

ghost commented Nov 4, 2020

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@Tratcher Tratcher added affected-medium This issue impacts approximately half of our customers severity-minor This label is used by an internal tool labels Nov 6, 2020 — with ASP.NET Core Issue Ranking
@davidfowl
Copy link
Member

This should be implemented in Kestrel directly as a structured log in a specific category.

@JeepNL
Copy link
Contributor

JeepNL commented Mar 29, 2021

@davidfowl Yes, please :)

(Just FYI): I've mentioned the log analyzer AWStats already in this thread, GoAccess is another fast and light weight log analyzer which is used by many NGINX/Apache systems. I think this (generating W3C logfiles) is an important feature which Kestrel, like any other stand alone production webserver, should support.

@CumpsD
Copy link
Contributor

CumpsD commented Jun 14, 2021

We removed nginx in front of our system recently and expose Kestrel directly, but didn't except something standard as access logs to be missing. Looking forward to seeing this added too

@JeepNL
Copy link
Contributor

JeepNL commented Jun 14, 2021

FYI @CumpsD The ASP.NET team started with this in ASP.NET Core 6 Preview 4

And about W3C logging, Justin Kotalik tweeted they're working on that for ASP.NET Core 6 Preview 5, see: #31843

Screenshot 2021-06-14 145418

@davidfowl
Copy link
Member

See #33251

@CumpsD
Copy link
Contributor

CumpsD commented Jun 14, 2021

Thanks!

@wtgodbe wtgodbe self-assigned this Jun 14, 2021
@wtgodbe
Copy link
Member

wtgodbe commented Jun 29, 2021

This is done as of #33251, will be included as part of dotnet 6.0-preview7

@wtgodbe wtgodbe closed this as completed Jun 29, 2021
@wtgodbe wtgodbe added Done This issue has been fixed and removed Needs: Design This issue requires design work before implementating. labels Jun 29, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jul 29, 2021
@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
affected-medium This issue impacts approximately half of our customers area-hosting Includes Hosting area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one severity-minor This label is used by an internal tool Theme: meeting developer expectations
Projects
None yet
Development

No branches or pull requests