ext_authz: Track Fail Open & ProcessingEffects in ExtAuthzLoggingInfo

[melginaldi]

Commit Message: Track fail open behavior and processing effects to request headers as part of ExtAuthzLoggingInfo to match parity with ext_proc filter.
Additional Description: This information will be used by ServiceExtensions for debugging/enhanced monitoring. This functionality was added to the ext_proc filter in #41295 and #41691, these bits are to keep parity with the ext_proc filter. last_req_processing_effect will hold the value of the last mutation event the filter completed on the request headers, whether successful or not. Therefore if 5 headers were successfully added but the 6 was invalid and the filter stopped processing any further mutations, the value of last_processing_effect will be InvalidMutationRejected.

Risk Level:Low
Testing:Tested in unit/integration tests
Release Notes: Added new tracking bit last_req_processing_effect and failed_open to ExtAuthzLoggingInfo Filter State to track mutation events and failed_open occurrences.
Docs Changes: N/A
Platform Specific Features: N/A

/assign @tyxia

[repokitteh-read-only]

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #43558 was opened by melginaldi.

see: more, trace.

[melginaldi]

/assign @tyxia

[tyxia]

Thanks for working on this. Fleshing out some comments...

/wait

[tyxia]

do you need similar tracking for response header?

[melginaldi]

No. I would prefer to only track request header mutations. I have changes the name of the field to match this expectation and update the code to only care about changes to the request headers.

[tyxia]

But I think your ext_proc changes are tracking both? processHeaderMutation handles both request and response path.

[melginaldi]

They were. I don't need both but I am happy to add the functionality of both for others

[tyxia]

Yea, I don't have strong opinion and i am fine that you only focus on request mutation in this PR.

Forward looking, if we want to handle both, should we rename MutationApplied to RequestMutationApplied . I guess this also depends on how you are going to differentiate it in ext_proc. I think it is totally doable but it probably requires some extra plumbing.

[melginaldi]

In ext_proc, I had two different fields. One was called encode_processing_effects and the other decode_processing_effect which then tracked the header request effect separate from the response.

This way the enum value can be MutationApplied for both and it is clear which path is taken.
I could change the field name in this PR to match and call it decode_processing_effect_. Thoughts?

[tyxia]

Even if you have decode and encode effect, they are eventually mapped down to enum below which only has
MutationApplied, Would the consumer(i.e., access log) be able to differentiate this? if not, I meant differentiate it have RequestMutationApplied and Response* there

enum class Effect : uint8_t {
// No processing effect. This is the default value except for body request/responses with body
// processing mode
// FULL_DUPLEX_STREAMED. In this case MutationApplied if the default.
None,
// The processor response sent a mutation that successfully modified the body or headers.
// This is the default value for body requests/responses using
// FULL_DUPLEX_STREAMED processing mode.
MutationApplied,
// The processor response sent a mutation that was attempted to modify the headers or trailers
// but was not applied due to invalid name or value.
InvalidMutationRejected,
// The processor response sent a mutation that was attempted to modify the headers or trailers
// but was not applied due to size limit exceeded.
MutationRejectedSizeLimitExceeded,
// The processor response sent a mutation that was attempted to modify the headers or trailers
// but was not applied due to failure.
MutationFailed,
};

[melginaldi]

Yes, the consumer could differentiate if the request path or response path had any mutations with the current enum. The req_processing_effect field reflects what mutations happened to the request headers, regardless of which direction/message caused these effects.

So for example, in OnComplete if the only effects are adding headers to the response then req_processing_effect will be None. Even though the request header message added these headers, the mutation did not occur to the request headers themself, so the effect is None.

The goal is to understand which part of the HTTP request message were mutated not which messages requested the mutations. Does this make sense?

[tyxia]

SG.

in the next PR, please address or add to TODO for response mutation tracking

[tyxia]

LGTM, Thanks!

@melginaldi I guess you will also need to test this change e2e (for example, with consumer) Have that been done?

Please also address the response mutation track (or a todo) in the future PR. I will merge this PR since CI is not stable at the moment

[tyxia]

SG.

in the next PR, please address or add to TODO for response mutation tracking