access_log: Add %REQ_WITHOUT_QUERY()% formatter#7847
access_log: Add %REQ_WITHOUT_QUERY()% formatter#7847dio wants to merge 1 commit intoenvoyproxy:masterfrom dio:fix-7583
Conversation
Signed-off-by: Dhi Aurrahman <dio@tetrate.io>
|
/retest |
|
🔨 rebuilding |
| ``":path" : "/?ok=true"`` | ||
|
|
||
| * ``%REQ(:PATH)%`` will log: ``/?ok=true`` | ||
| * ``%REQ_WITHOUT_QUERY(:PATH)%`` will log: ``/`` |
There was a problem hiding this comment.
will this ever be used with any other header? wondering if it makes more sense to go with the other suggestion in the issue with making this a top level format
There was a problem hiding this comment.
OH, I can also see: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referer that's why I went ahead with this. But yeah, probably a suffix operator is better?
REQ(X?Y):<MAX_LENGTH>:<POST_PROCESSING_OPERATOR>, e.g. REQ(:PATH):2:REMOVE_QUERY, REQ(:PATH):-:REMOVE_QUERY. Need to go back to discuss this in the issue then.
There was a problem hiding this comment.
As long as there are more than one header this makes sense for I'm fine with this. Are suffix operators something we currently support? I don't feel super strongly about either, would probably go with whichever has precedent
| int command_end_position = pos + token.length(); | ||
|
|
||
| if (absl::StartsWith(token, "REQ(")) { | ||
| const bool req_without_query = absl::StartsWith(token, "REQ_WITHOUT_QUERY("); |
There was a problem hiding this comment.
hmm looking at this code would it make more sense to have REQ_WITHOUT_QUERY just take the header key as an input instead of a string to transform? right now it has to be used with REQ so it doesn't seem like its providing any additional flexibility
|
This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
stale
bot
added
the
stale
|
This pull request has been automatically closed because it has not had activity in the last 14 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
2 participants
Description:
This patch adds
%REQ_WITHOUT_QUERY(X?Y):Z%formatter to facilitate removing query string from a request header entry value. This is an attempt to fix #7583.Also modified the metadata else-if case to match the style with other cases.
Risk Level: Low, newly added
Testing: Added unit tests
Docs Changes: Added
Release Notes: Added
Fixes #7583
Signed-off-by: Dhi Aurrahman dio@tetrate.io