-
Notifications
You must be signed in to change notification settings - Fork 98
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
Add configurable data plane log level #2561
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2561 +/- ##
==========================================
- Coverage 89.34% 89.23% -0.11%
==========================================
Files 100 101 +1
Lines 7630 7667 +37
Branches 50 50
==========================================
+ Hits 6817 6842 +25
- Misses 756 768 +12
Partials 57 57 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we think this is needed? Also, if we should add a NGINX Plus in debug mode tab?
@@ -88,6 +88,9 @@ nginx: | |||
# -- Is NGINX Plus image being used | |||
plus: false | |||
|
|||
# -- Is NGINX run in debug mode. This should be used with setting the NGINX error log level to debug. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kate-osborn Should the wording on the comment be more strict to specify "require"?
I think it'd be fine to run NGINX in debug mode without NGINX error log level being debug, there'd be no point but I'd be fine with that. I can change this to something like "In order for this to be useful, NGINX error log level should be set to debug" or anything you suggest.
deploy/nginx-debug/deploy.yaml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would an nginx-plus in debug mode be beneficial to generate?
Ready for review, still testing to make sure everything works with NGINX plus. |
be [run with NGINX in debug mode](#run-nginx-gateway-fabric-with-nginx-in-debug-mode) upon startup through the addition | ||
of a few arguments. {{</ note >}} | ||
|
||
## Run NGINX Gateway Fabric with NGINX in debug mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, this feels like it should be somewhere else...troubleshooting guide maybe?
Closing this PR and moving to #2603 |
Add Nginx error log level to NginxProxy.
Problem: Users would like to assign a log level for the data plane.
Solution: Add Nginx error log level to NginxProxy which allows users to set the error log level for Nginx.
Testing: Unit tests and manual testing.
Manual testing included
ErrorLevel
field in the NginxProxy resource.main.conf
file and runningnginx -T
) in the nginx container was updated correctly.info
log level.notice
such as theerror
level, after making changes which would result innotice
log levels being outputted (such as deleting and re-applying cafe.yaml in the cafe example), nonotice
log levels are outputted.nginx -T
correctly shows themain.conf
configuration file being included in the nginx configuration.ps aux |grep nginx
when inside the nginx container correctly showed nginx-debug processes.Closes #901
Checklist
Before creating a PR, run through this checklist and mark each as complete.
Release notes
If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.