feat(accesslog): support config omit_empty_values

[j2gg0s]

Support user config omit_empty_values for Envoy's AccessLog, reference Issue/31060.

Related envoy's SubstitutionFormatString.omit_empty_values

[istio-policy-bot]

😊 Welcome @j2gg0s! This is either your first contribution to the Istio api repo, or it's been awhile since you've been here.

You can learn more about the Istio working groups, code of conduct, and contributing guidelines by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

[istio-testing]

Hi @j2gg0s. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[zirain]

/ok-to-test

[j2gg0s]

@j2gg0s: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command release-notes_api 71e0d4d link false /test release-notes_api

this pr need release notes?

[zirain]

/test release-notes_api

[j2gg0s]

Any other thing than i need to fix?

[linsun]

Yes, can we add a release note for this pls?

[istio-testing]

@j2gg0s: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
release-notes_api	37783288f25d198f7f8a5c387aea2f7c48314ab3	link	false	/test release-notes_api

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. I understand the commands that are listed here.

[zirain]

kindly ping @linsun

[istio-testing]

@j2gg0s: PR needs rebase.

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.

[akashjain971]

Anyone taking this forward?

[j2gg0s]

Anyone taking this forward?

@akashjain971 I switched to EnvoyFilters to manage logs, such as:

apiVersion: networking.istio.io/v1alpha3
kind: EnvoyFilter
metadata:
  name: log
  namespace: istio-system
spec:
  configPatches:
  - applyTo: NETWORK_FILTER
    match:
      listener:
        filterChain:
          filter:
            name: envoy.filters.network.http_connection_manager
    patch:
      operation: MERGE
      value:
        typed_config:
          '@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
          accessLog:
          - name: envoy.access_loggers.file
            typedConfig:
              '@type': type.googleapis.com/envoy.extensions.access_loggers.file.v3.FileAccessLog
              logFormat:
                jsonFormat:
                  authority: '%REQ(:AUTHORITY)%'
                  ...
                  upstream_transport_failure_reason: '%UPSTREAM_TRANSPORT_FAILURE_REASON%'
                omit_empty_values: true
              path: /dev/stdout

[akashjain971]

Anyone taking this forward?

@akashjain971 I switched to EnvoyFilters to manage logs, such as:

apiVersion: networking.istio.io/v1alpha3
kind: EnvoyFilter
metadata:
  name: log
  namespace: istio-system
spec:
  configPatches:
  - applyTo: NETWORK_FILTER
    match:
      listener:
        filterChain:
          filter:
            name: envoy.filters.network.http_connection_manager
    patch:
      operation: MERGE
      value:
        typed_config:
          '@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
          accessLog:
          - name: envoy.access_loggers.file
            typedConfig:
              '@type': type.googleapis.com/envoy.extensions.access_loggers.file.v3.FileAccessLog
              logFormat:
                jsonFormat:
                  authority: '%REQ(:AUTHORITY)%'
                  ...
                  upstream_transport_failure_reason: '%UPSTREAM_TRANSPORT_FAILURE_REASON%'
                omit_empty_values: true
              path: /dev/stdout

Thank you @j2gg0s. This should sail my boat for now but will this not add latency for every request?

[akashjain971]

Looks like OmitEmptyValues is already supported here. This required Istio code changes and cannot be done from the IstioOperator.

[istio-policy-bot]

🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2022-07-23. If you feel this issue or pull request deserves attention, please reopen the issue. Please see this wiki page for more information. Thank you for your contributions.

Created by the issue and PR lifecycle manager.