Skip to content

/ reverse-proxy

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

Use LoggerMessage.Define for logs #82

Merged
merged 4 commits into from Apr 24, 2020
Merged

Use LoggerMessage.Define for logs #82

merged 4 commits into from Apr 24, 2020

Conversation

@Kahbazi
Copy link
Contributor

Kahbazi commented Apr 23, 2020 •
edited

  • Added CategoryName to IOperationLogger
  • Use LoggerMessage.Define
  • Added Event Ids in a single file so it's easier to prevent using a same event id

Fixes #20
Fixes #66
@Kahbazi
Use LoggerMessage.Define for logs
Verified
This commit was signed with a verified signature.
Kahbazi Kahbazi
GPG key ID: BC038B2B3DE848C0 Learn about signing commits
Loading status checks…
9897912
@Kahbazi Kahbazi requested review from anurse, halter73 and Tratcher as code owners Apr 23, 2020
@Kahbazi
Add a Missed a parameter for HealthCheckDisabled log
Verified
This commit was signed with a verified signature.
Kahbazi Kahbazi
GPG key ID: BC038B2B3DE848C0 Learn about signing commits
Loading status checks…
32f1505
@Tratcher
Tratcher reviewed Apr 24, 2020
View changes
src/ReverseProxy.Core/Configuration/ProxyConfigLoader.cs Outdated
@@ -83,7 +84,7 @@ private async Task ApplyAsync(ProxyConfigOptions config)
return;
}

_logger.LogInformation("Applying proxy configs");
_logger.LogInformation(EventIds.ApplyProxyConfig, "Applying proxy configs");

This comment has been minimized.

Sign in to view
Copy link
@Tratcher

Tratcher Apr 24, 2020

Member

No wrapper for this one?

This comment has been minimized.

Sign in to view
Copy link
@Kahbazi

Kahbazi Apr 24, 2020

Author Contributor

I wanted to ask, should I use LoggerMessage.Define for log messages that don't have any parameters?

This comment has been minimized.

Sign in to view
Copy link
@Tratcher

Tratcher Apr 24, 2020

Member

I'm not sure what the official guidance is, but the inconsistency bothers me.

@rynowak you changed the logger define pattern last, no?
src/ReverseProxy.Core/Middleware/EndpointInitializerMiddleware.cs Outdated Show resolved Hide resolved
src/ReverseProxy.Utilities/RecurrentActivities/RecurrentActivityManager.cs Outdated Show resolved Hide resolved
Kahbazi added 2 commits Apr 24, 2020
@Kahbazi
Resolve Conflict
Verified
This commit was signed with a verified signature.
Kahbazi Kahbazi
GPG key ID: BC038B2B3DE848C0 Learn about signing commits
39cd915
@Kahbazi
Address review points
Verified
This commit was signed with a verified signature.
Kahbazi Kahbazi
GPG key ID: BC038B2B3DE848C0 Learn about signing commits
Loading status checks…
7c18414
@Tratcher
Tratcher approved these changes Apr 24, 2020
View changes
@Tratcher Tratcher added this to the 1.0.0-preview1 milestone Apr 24, 2020
@Tratcher Tratcher merged commit 00c4e7c into microsoft:master Apr 24, 2020
5 checks passed
5 checks passed
license/cla All CLA requirements met.
Details
microsoft-reverse-proxy-ci Build #20200424.6 succeeded
Details
microsoft-reverse-proxy-ci (Build Ubuntu 16.04) Build Ubuntu 16.04 succeeded
Details
microsoft-reverse-proxy-ci (Build Windows) Build Windows succeeded
Details
microsoft-reverse-proxy-ci (Build macOS 10.14) Build macOS 10.14 succeeded
Details
@Tratcher
Copy link
Member

Tratcher commented Apr 24, 2020

Thanks
@Kahbazi Kahbazi deleted the Kahbazi:kahbazi/log branch Apr 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tratcher Tratcher

@anurse anurse

@halter73 halter73

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
  1.0.0-preview1
Linked issues

Successfully merging this pull request may close these issues.

Add CategoryName To IOperationLogger
Logging review
2 participants
@Kahbazi @Tratcher
You can’t perform that action at this time.