-
Notifications
You must be signed in to change notification settings - Fork 2k
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 k8s objects variables in log format #1231
Conversation
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.
Please see my feedback
I think it's better to rename the PR to something like "Support k8s objects variables in log format", rather than "Update IC log format " to better reflect the PR contents.
@@ -205,7 +205,7 @@ See the doc about [VirtualServer and VirtualServerRoute resources](/nginx-ingres | |||
* - ``log-format`` | |||
- Sets the custom `log format <https://nginx.org/en/docs/http/ngx_http_log_module.html#log_format>`_ for HTTP and HTTPS traffic. For convenience, it is possible to define the log format across multiple lines (each line separated by ``\n``). In that case, the Ingress Controller will replace every ``\n`` character with a space character. All ``'`` characters must be escaped. |
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.
I think there is a misunderstanding that those variables should always be present in the log format. It is not the case - those variables should be available, but the admin may choose to not use them.
For the docs, it is necessary to describe what variables are available for admins to use and what they mean.
The example of the log line is useful. However, we should also have the example of a value you can put in the ConfigMap (the log format). So that the reader can copy that value and use it in their ConfigMap.
docs-web/configuration/global-configuration/configmap-resource.md
Outdated
Show resolved
Hide resolved
examples-of-custom-resources/basic-tcp-udp/global-configuration.yaml
Outdated
Show resolved
Hide resolved
Co-authored-by: Mike Stephen <[email protected]>
docs-web/configuration/global-configuration/configmap-resource.md
Outdated
Show resolved
Hide resolved
99791e2
to
9458ccf
Compare
Co-authored-by: Dean Coakley <[email protected]>
Co-authored-by: Dean Coakley <[email protected]>
Proposed changes
This PR updates the Ingress Controller log format for both Ingress and VirtualServer resources with k8s object details.
Checklist
Before creating a PR, run through this checklist and mark each as complete.