Ingress-nginx: Add support of conditional logging

[skandyla]

It would be good to have an ability to enable\disable conditional logging.
Ideally this functionality should be set at annotations level allowing per ingress customization.
This can be very useful for setups when we want to skip logging for normal requests but still track errors.

https://docs.nginx.com/nginx/admin-guide/monitoring/logging/#enabling-conditional-logging
Conditional logging allows excluding trivial or unimportant log entries from the access log. In NGINX, conditional logging is enabled by the if parameter to the access_log directive.

This example excludes requests with HTTP status codes 2xx (Success) and 3xx (Redirection):

map $status $loggable {
    ~^[23]  0;
    default 1;
}
access_log /path/to/access.log combined if=$loggable;

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[skandyla]

/remove-lifecycle rotten

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[pgdagenais]

/remove-lifecycle rotten

[monkeymorgan]

Hey @skandyla - I managed to get this working by using

https://github.com/kubernetes/ingress-nginx/blob/master/docs/user-guide/nginx-configuration/custom-template.md

I first exported the nginx tmpl bundled in the nginx controller image :

kubectl exec -it nginx-ingress-controller-<redacted> -- cat templat/nginx.tmpl > nginx.tmpl

Edited it and created a configmap :

vim nginx.tmpl
kubectl create configmap nginx-tracing --from-file=nginx-tmpl=nginx.tmpl

Then set the custom template options / deployed

  ## Override NGINX template
  customTemplate:
    configMapName: "nginx-tracing"
    configMapKey: "nginx-tmpl"

And checked it applied :

kubectl exec -it nginx-ingress-controller-<redacted> -- cat nginx.conf > nginx.conf.new

[monkeymorgan]

I should add that in my case I wanted to only log requests selected for tracing as identified by the zipkin b3_sampled http header :

    map $http_x_b3_sampled $loggable {
        1 1;
        default 0;
    }

but obviously you can alter any part of the nginx config

[skandyla]

@monkeymorgan, thanks for the solution. It's quite obvious to use custom template, but I believe such functionality should be in the core of the nginx ingress controller, this why I've opened this feature request.

[mjeri]

Ack to @skandyla. It would be really nice to have that feature in the core without using custom templates. In the end I think it's a really common use-case to constraint access logs by status codes.

[dudubis]

Its would be very useful to have this ability. Currently I need to drop 98% of the nginx-controller logs on the filebeat/logstash level to get the logs I need. This is a lot of cpu and throughput that can be prevented with a simple nginx config. Hopefully this this issue gets promoted.

[rikatz]

/assign @rikatz

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[tiras-j]

Is there any efforts to provide this functionality (I'm assuming not currently given the stale/"rotten" state of this issue)?

One simple (albeit still non-ideal) approach for this would be to insert the http-snippet configuration before the access log declaration, so that map directives in the snippet can be used for the access-log-params config map key. I'm sure there are other reasons why this isn't quite so simple, but it's the current blocker I'm running into in getting this working for the simple case of only logging access logs on 5xx responses.

[aledbf]

@tiras-j one of the issues is that we already use the if confidition in the access_log directive https://github.com/kubernetes/ingress-nginx/blob/master/rootfs/etc/nginx/template/nginx.tmpl#L311
That said, PRs are always welcome

[tiras-j]

@aledbf Ahhh... I didn't realize the if condition couldn't be repeated (I have admittedly minimal knowledge of NGINX), but that certainly makes sense.

Do you have any recommendations for conditioning access logs on non-2xx/3xx responses with the current configuration?

Perhaps we can re-map $loggable at the server context? Oh or now that I think about it perhaps the error I was seeing wasn't about the multiple map directives for $loggable but rather the multiple if= parameters. I'll tinker around with the configs.

[aledbf]

@tiras-j it can be repeated with something like the next snippet under the mentioned position in the template or using the http-snippet

map $status $loggable {
    default $loggable;
    ~^[23]  0;
}

[tiras-j]

@aledbf it looks like I'm getting cycle errors when trying to evaluate that map (seemingly because the default $loggable is cyclical on the map directive?). Specifically:

2020/04/01 21:02:56 [error] 4302#4302: *2877399 cycle while evaluating variable "status" while logging request, ...
2020/04/01 21:02:18 [error] 4302#4302: *2877399 cycle while evaluating variable "loggable" while logging request, ...

I'm a bit surprised by the $status error since that's technically not cyclical. Changing the map target to a different (not yet defined) variable seems to solve the issue but of course that's not what's configured for the access log conditional. I'll try doing this at the server block context via server-snippet and see if it remains an issue.

EDIT: Ah right, map directives are only valid in the http context so server-snippet is out for that.

[dudubis]

Hi @tiras-j, have you managed to get it working? Can you post a short description of your configuration?