Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds support for chaining HTTP header log fields with a fallback operator (?:) inside the existing %<{...}...> header-field syntax, so custom log formats can use a secondary header when the primary one is absent while preserving the “missing vs empty” behavior.
Changes:
- Add parsing and runtime support for header-field fallback chains (e.g.
%<{a}cqh?:{b}cqh>), storing the chain as a singleLogField. - Extend logging marshalling/unmarshalling to select the first present header at runtime and apply per-candidate slicing/escaping.
- Update admin documentation and extend AuTest coverage/gold output for primary/secondary/missing header cases.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/proxy/logging/LogFormat.cc |
Parses ?: fallback chains and constructs a single LogField representing the chain. |
include/proxy/logging/LogField.h |
Adds HeaderField representation and LogField constructor/helpers for fallback chains. |
src/proxy/logging/LogField.cc |
Implements fallback selection + marshal/unmarshal behavior using the chosen candidate’s slice/escaping. |
include/proxy/logging/LogAccess.h |
Adds has_http_header_field() API used to decide which candidate is present. |
src/proxy/logging/LogAccess.cc |
Implements header_for_container() mapping and has_http_header_field() lookup logic. |
tests/gold_tests/logging/log-field.test.py |
Extends the test to send primary/secondary headers and validate fallback behavior. |
tests/gold_tests/logging/gold/field-test.gold |
Updates expected log output to include the new Request-ID field and fallback results. |
doc/admin-guide/logging/formatting.en.rst |
Documents header fallback chain syntax, semantics, and per-candidate slicing. |
serrislew
reviewed
Mar 30, 2026
serrislew
reviewed
Mar 30, 2026
Allow custom log formats to fall back between header fields with `??` so configs can log a secondary header when the primary one is absent. This keeps the feature in the existing `%<...>` syntax and preserves the current distinction between missing and empty values. This also allows overriding the default static fallback from `-` to some user-specified quoted string literal.
bneradt
force-pushed
the
log_field_fallback
branch
from
6e96396 to
373cc22
Compare
zwoop
previously approved these changes
Apr 1, 2026
Contributor
zwoop
left a comment
zwoop
left a comment
There was a problem hiding this comment.
Yeah, looks good. I'm ok deferring "general log tag support" for future additions. One could imagine for example a logging of X-Client-IP with a fallback to the connection remote IP log field.
like
Client-IP:%<{X-Client-IP}cqh??chi>
or some such.
zwoop
previously approved these changes
Apr 2, 2026
src/proxy/logging/LogFormat.cc
Outdated
|
|
||
| if (parsed->fallback_symbol.has_value()) { | ||
| std::string fallback_symbol = *parsed->fallback_symbol; | ||
| std::string field_symbol = fallback_symbol; |
Contributor
There was a problem hiding this comment.
Do we really need two copies here for the string?
The previous header fallback implementation only allowed
header fields in the chain. This adds support for a final
non-header log field (e.g., chi, cqup) as the fallback
term, letting users write expressions like
%<{x-remote-ip}cqh??chi>.
Rename LogHeaderFallback to LogFieldFallback to reflect the
broader scope. Update documentation, unit tests, and the
end-to-end autest.
bneradt
force-pushed
the
log_field_fallback
branch
from
f2a2e87 to
211e433
Compare
zwoop
approved these changes
Apr 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allow custom log formats to fall back between header fields with
??so configs can log a secondary header when the primary one isabsent. This keeps the feature in the existing
%<...>syntax andpreserves the current distinction between missing and empty values.
Store fallback chains as a single log field so the selected header
still uses normal escaping and its own slice settings. Update the
logging admin guide and extend the logging AuTest coverage for
primary, secondary, and missing header cases.