Invoke ORCA Load Report callbacks from Router::Filter.
#35728
Conversation
- Add `LoadBalancerContext::setOrcaLoadReportCb(OrcaLoadReportCb callback)` method. Signed-off-by: Misha Efimov <[email protected]>
|
/assign @adisuissa |
There was a problem hiding this comment.
Overall LGTM!
Please add tests to increase coverage of the maybeProcessOrcaLoadReport() method (see https://storage.googleapis.com/envoy-pr/caeacc6/coverage/source/common/router/router.cc.gcov.html).
/wait
source/common/router/router.cc
Outdated
| @@ -2118,7 +2118,7 @@ void Filter::maybeProcessOrcaLoadReport(const Envoy::Http::HeaderMap& headers_or | |||
| auto host = upstream_request.upstreamHost(); | |||
| const bool need_to_send_load_report = | |||
| (host != nullptr) && cluster_->lrsReportMetricNames().has_value(); | |||
| if (!need_to_send_load_report) { | |||
| if (!need_to_send_load_report && !orca_load_report_cb_) { | |||
There was a problem hiding this comment.
nit: please update the comment above.
There was a problem hiding this comment.
Hrm, I think it is already correct as it says "or invoke the ORCA callbacks.", what am I missing?
|
I am back. Will take a look at this tomorrow. :) |
|
Thank you for your feedback! I was OOO last week and will address your comments shortly. |
Signed-off-by: Misha Efimov <[email protected]>
Signed-off-by: Misha Efimov <[email protected]>
|
/retest |
Thank you for quick reply and approval! |
| if (orca_load_report_callbacks_.has_value()) { | ||
| const absl::Status status = orca_load_report_callbacks_->onOrcaLoadReport(*orca_load_report); | ||
| if (!status.ok()) { | ||
| ENVOY_LOG_MISC(error, "Failed to invoke OrcaLoadReportCallbacks: {}", status.message()); |
There was a problem hiding this comment.
consider using ENVOY_LOG/ENVOY_STREAM_LOG rather than the misc one.
There was a problem hiding this comment.
@efimki Merge it due to tight schedule but please do address this comment above in a follow-up.
There was a problem hiding this comment.
Thanks, @tyxia! My apologies as I've totally missed this comment. I'll address that in a follow-up.
Commit Message:
Invoke ORCA Load Report callbacks from
Router::Filter.LoadBalancerContext::setOrcaLoadReportCb(OrcaLoadReportCb callback)method.Risk Level: Low
Release Notes: N/A
Platform Specific Features: N/A
#34777