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

Remove IKestrelTrace #37242

Merged
merged 8 commits into from
Oct 6, 2021
Merged

Remove IKestrelTrace #37242

merged 8 commits into from
Oct 6, 2021

Conversation

JamesNK
Copy link
Member

@JamesNK JamesNK commented Oct 4, 2021

Fixes #31581

Yeet IKestrelTrace and associated implementations.

TODO:

  • Seal KestrelTrace
  • Remove virtual methods from KestrelTrace
  • Remove commented out files

I will do the same for SocketsTrace and QuicTrace in future PRs.

@davidfowl
Copy link
Member

Why do we still have kestrel trace?

@JamesNK
Copy link
Member Author

JamesNK commented Oct 4, 2021

It wraps a bunch of differently named loggers:

internal partial class KestrelTrace : IKestrelTrace
{
protected readonly ILogger _generalLogger;
protected readonly ILogger _badRequestsLogger;
protected readonly ILogger _connectionsLogger;
protected readonly ILogger _http2Logger;
protected readonly ILogger _http3Logger;
public KestrelTrace(ILoggerFactory loggerFactory)
{
_generalLogger = loggerFactory.CreateLogger("Microsoft.AspNetCore.Server.Kestrel");
_badRequestsLogger = loggerFactory.CreateLogger("Microsoft.AspNetCore.Server.Kestrel.BadRequests");
_connectionsLogger = loggerFactory.CreateLogger("Microsoft.AspNetCore.Server.Kestrel.Connections");
_http2Logger = loggerFactory.CreateLogger("Microsoft.AspNetCore.Server.Kestrel.Http2");
_http3Logger = loggerFactory.CreateLogger("Microsoft.AspNetCore.Server.Kestrel.Http3");
}

It could be split out into BadRequestsLog, Http2Log, etc, but I think a centralized type that ensures the right logger is used for each log message seems worthwhile.

Copy link
Member

@Tratcher Tratcher left a comment

Choose a reason for hiding this comment

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

Very happy to see these test trace classes gone.

@JamesNK JamesNK merged commit 6c5e668 into main Oct 6, 2021
@JamesNK JamesNK deleted the jamesnk/kestreltrace branch October 6, 2021 17:47
@ghost ghost added this to the 7.0-preview1 milestone Oct 6, 2021
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove IKestrelTrace
5 participants