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

Support "none" keyword in log-format escape configuration #8693

Closed
knbnnate opened this issue Jun 14, 2022 · 5 comments · Fixed by #8692
Closed

Support "none" keyword in log-format escape configuration #8693

knbnnate opened this issue Jun 14, 2022 · 5 comments · Fixed by #8692
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-priority triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@knbnnate
Copy link
Contributor

I would like the ingress-nginx ingress controller to support the native nginx functionality of disabling escaping characters in access logs. The specific requirement that is driving me here is that I'd like to log all of the request and response headers going through nginx in a json format. I've got set_by_lua_block and log_by_lua_block code to do that, and I've got my log-format-upstream set so that I can create the exact json log message that I want. The problem is, the request headers and response headers in the message are a string containing escaped json rather than json. Example:

Configmap entries:

  location-snippet: |
    set $request_headers '';
    set_by_lua_block $request_headers{
      local cjson = require("cjson")
      local headers = ngx.req.get_headers()
      return cjson.encode(headers)
    }
  log-format-escape-json: true
  log-format-upstream: '{"host":"$host","request":"$request","request_headers":"$request_headers"}'

Desired configmap entry instead of log-format-escape-json:

  log-format-escape-none: true

Generated nginx.conf line inside the ingress controller pod:

# kubectl -n ingress exec ingress-nginx-controller-597785cb88-xr7gr -- cat nginx.conf | grep log.format
        log_format upstreaminfo escape=json '{"host":"$host","request":"$request","request_headers":"$request_headers"}';

Desired nginx.conf line inside the ingress controller pod:

# kubectl -n ingress exec ingress-nginx-controller-597785cb88-xr7gr -- cat nginx.conf | grep log.format
        log_format upstreaminfo escape=none '{"host":"$host","request":"$request","request_headers":"$request_headers"}';

Request:

# curl -H "request-foo: bar" https://endpoint.local

Logs:

{"host":"endpoint.local","request":"GET / HTTP/2.0","request_headers":"{\"request-foo\": \"bar\"}"}

Desired logs:

{"host":"endpoint.local","request":"GET / HTTP/2.0","request_headers":{"request-foo": "bar"}}

Nginx supports the "none" keyword in the log-format escape option specifically for this use case. The ingress controller simply hasn't exposed the option.

There is currently another issue associated with this: #8105
This issue failed to get traction; I've taken a more incremental approach here that should be easier to swallow.

Implementation of the more incremental approach is at #8692

This does not require a particular kubernetes version.

The documentation should be updated to reflect that log-format-escape-none can be used to disable escaping entirely.

/kind documentation

@knbnnate knbnnate added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 14, 2022
@k8s-ci-robot k8s-ci-robot added kind/documentation Categorizes issue or PR as related to documentation. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority labels Jun 14, 2022
@longwuyuan
Copy link
Contributor

/kind feature
/triage accepted
/remove-kind documentation

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed kind/documentation Categorizes issue or PR as related to documentation. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jun 14, 2022
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 12, 2022
@knbnnate
Copy link
Contributor Author

@longwuyuan is there a current estimate on how much longer the stability work will take priority? I'm guessing three more months or so based on the original assessment. Just checking in, I don't have access to the kubernetes slack so I can't really keep an eye on the conversation. Thanks!

@longwuyuan
Copy link
Contributor

More details are available here https://youtu.be/UBt4N82ymOE and here https://kubernetes.slack.com/archives/CANQGM8BA/p1656020331133589.
The announcement in the dev mailing list is here https://groups.google.com/a/kubernetes.io/g/dev/c/rxtrKvT_Q8E

@knbnnate
Copy link
Contributor Author

More details are available here https://youtu.be/UBt4N82ymOE and here https://kubernetes.slack.com/archives/CANQGM8BA/p1656020331133589. The announcement in the dev mailing list is here https://groups.google.com/a/kubernetes.io/g/dev/c/rxtrKvT_Q8E

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-priority triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants