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

Better diagnostics when missing HttpModule or MiddleWare #979

Merged

Conversation

bakgerman
Copy link
Contributor

@bakgerman bakgerman commented Aug 5, 2023

Uses ctor for middleware and httpmodule, and initializelayoutrender/initializetarget, instead of warning every time, as requested.

Resolves #954 and resolves #953

@snakefoot
Copy link
Contributor

snakefoot commented Aug 5, 2023

Maybe having the alert in the initialize-section might produce too many false-positives. Ex. loaded NLog configuration, but not yet setup http-pipelines with middleware etc.

Maybe it should be the on the first LogEvent where HttpContext is available.

Also could it become InternalLogger.Info instead of InternalLogger.Warn, and something like "NLogRequestPostedBodyMiddleware is not yet initialized, which is required by aspnet-request-posted-body.".

Burak Akgerman added 2 commits August 5, 2023 23:39
…only upon first log event. Used double lock pattern.
@bakgerman
Copy link
Contributor Author

Changed to first log event

Copy link
Contributor

@snakefoot snakefoot left a comment

Choose a reason for hiding this comment

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

Think it looks great. Just have some small suggestions.

@bakgerman
Copy link
Contributor Author

Thank you for the always good suggestions. Modified as suggested.

(Have you pondered, when it may be possible to work on items marked as 'breaking changes' in the issues list?)

@bakgerman bakgerman changed the title Issue 934 and 935, second attempt (less overhead version) Issue 934 and 935, Middleware Initialization Debugging Aug 6, 2023
@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 6, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell C 6 Code Smells

75.0% 75.0% Coverage
0.0% 0.0% Duplication

warning The version of Java (11.0.20) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@snakefoot
Copy link
Contributor

Looks good. Thank you for another nice contribution.

@snakefoot snakefoot merged commit 4e496e8 into NLog:master Aug 6, 2023
@snakefoot snakefoot changed the title Issue 934 and 935, Middleware Initialization Debugging Better diagnostics when missing HttpModule or MiddleWare Aug 6, 2023
@bakgerman bakgerman deleted the middleware-not-installed-warnings-2 branch August 6, 2023 15:49
@snakefoot snakefoot modified the milestone: 5.3.2 Sep 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants