minor opt: minor optimization to the orca parser #36492
Conversation
Signed-off-by: wangbaiping <[email protected]>
|
/wait-any on CI |
There was a problem hiding this comment.
Thanks!
Overall LGTM, modulo minor comment.
Assigning Blake as the original contributor of this code.
/assign @blake-snyder
source/common/orca/orca_parser.cc
Outdated
| #else | ||
| IS_ENVOY_BUG("JSON formatted ORCA header support not implemented for this build"); | ||
| #endif // !ENVOY_ENABLE_FULL_PROTOS || !ENVOY_ENABLE_YAML | ||
| } else { | ||
| return absl::InvalidArgumentError( | ||
| fmt::format("unsupported ORCA header format: {}", split_header.first)); | ||
| return absl::InvalidArgumentError(fmt::format("unsupported ORCA header format")); |
There was a problem hiding this comment.
I'd consider keeping some string as part of the error.
I think it makes sense to keep the minimum between the first 5 characters, and the prefix up to the first occurrence of a " " (the delimiter).
It makes it easier to debug if/when things go wrong with an external upstream server.
|
/wait-any on CI |
|
(yeah I just invited blake to the org but I think it hasn't gone through yet =P) |
Signed-off-by: wangbaiping <[email protected]>
Signed-off-by: wangbaiping <[email protected]>
|
block by #36525 |
Signed-off-by: wangbaiping <[email protected]>
|
gently ping @blake-snyder cc @alyssawilk |
|
cc @adisuissa @blake-snyder thanks for the approvals 🌷 |
|
cc @alyssawilk friendly ping for an approval from senior maintainer because this change the core code. :) |
There was a problem hiding this comment.
LGTM but general question is why is this in core espeically if it's not being used? May be a question for @blake-snyder
I think this should has been used by the orca related features? |
This is currently used during ORCA header processing: https://github.com/envoyproxy/envoy/blob/main/source/common/router/router.cc#L2131 |
Commit Message: minor opt: minor optimization to the orca parser Additional Description: By this way, the parser needn't to scan the whole header value if the header value has invalid format. And the we needn't create a copy of the header value for json format now. Risk Level: low. Testing: n/a. Docs Changes: n/a. Release Notes: n/a. Platform Specific Features: n/a. --------- Signed-off-by: wangbaiping <[email protected]> Signed-off-by: Gustavo <[email protected]>
Commit Message: minor opt: minor optimization to the orca parser
Additional Description:
By this way, the parser needn't to scan the whole header value if the header value has invalid format. And the we needn't create a copy of the header value for json format now.
Risk Level: low.
Testing: n/a.
Docs Changes: n/a.
Release Notes: n/a.
Platform Specific Features: n/a.