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

Access Log Filter runtime_key field behaves unexpectedly #9995

Open
jbohanon opened this issue Sep 6, 2024 · 1 comment
Open

Access Log Filter runtime_key field behaves unexpectedly #9995

jbohanon opened this issue Sep 6, 2024 · 1 comment
Labels
Type: Bug Something isn't working

Comments

@jbohanon
Copy link
Contributor

jbohanon commented Sep 6, 2024

Gloo Edge Product

Open Source

Gloo Edge Version

v1.17.0

Kubernetes Version

N/A

Describe the bug

The implementation of AccessLogFilters implicitly relies on the default value unless the end user defines the runtime values at the configured key themselves. As the default field is not required, and we compare against 0 in the absence of a value, this results in the access logging filter for status code being silently inert without a default set. Our guide currently does not show a default set. fixed in #9998

Expected Behavior

Configuring should not be so obtuse. The default field should be required, or we should set the runtime key based on the user config (non-trivial, probably bad) <-- this is a bad idea and we should not do it.

At a MINIMUM, the documentation should be updated to indicate this gotcha.

Steps to reproduce the bug

  1. Install Gloo Edge 1.17.X
  2. Follow the guide for access log filtering based on 400+ status code here
  3. Note that it is ineffective.
  4. Add default: 400 as a sibling to runtimeKey: "400" and note that it now works as expected.
  5. Remove runtimeKey and note that it still works as expected.

I suspect that if we went in and manually set a runtime key access_log_status_filter: 400 in the running envoy instance we would be able to use that key via runtimeKey: "access_log_status_filter" but I have not tested this.

Additional Environment Detail

No response

Additional Context

Envoy code where comparison occurs: https://github.com/envoyproxy/envoy/blob/v1.30.4/source/common/access_log/access_log_impl.cc#L36-L41

Implementation PR of AccessLogFilters: #7627

Internal slack: https://solo-io-corp.slack.com/archives/CEDCS8TAP/p1725636045571749

@jbohanon
Copy link
Contributor Author

This has been largely mitigated by the documentation update in #9998, probably enough to resolve this issue given the minimal value:high investment ratio of implementing a more permanent fix that would definitely be a breaking change.

@jbohanon jbohanon changed the title Access Log Filter runtime_key field not properly implemented Access Log Filter runtime_key field behaves unexpectedly Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant