dynamic_modules: support for http callouts#39151
Conversation
|
tomorrow i will continue working on the integration test and then good to go |
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
|
Ping @mattklein123 for senior maintainer review. |
| DynamicModuleHttpFilter::HttpCalloutCallback& callback = *iterator->second; | ||
| auto request = cluster->httpAsyncClient().send(std::move(message), callback, options); | ||
| if (!request) { | ||
| ENVOY_LOG(error, "Failed to send HTTP callout"); |
There was a problem hiding this comment.
I can't remember but I think this one can fail due to legitimate runtime issues like no healthy upstream? I don't think you should log spam on this one. You need to either use a backoff log, metric, etc. The ones above seem like general config/programming errors to me so I guess OK.
There was a problem hiding this comment.
Thanks, that's a valid concern. I changed the ABI to return the status instead of the boolean so that it doesn't need to log at all in the first place. a739f72
(i think we should switch to this status style elsewhere as well but i can take a shot at it in follow ups)
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Notably, the latest main comes with: * Scheduler ABI: envoyproxy/envoy#39765 * Callouts ABI: envoyproxy/envoy#39151 Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Commit Message: dynamic_modules: support for http callouts
Additional Description:
This adds support for http callouts initiated from HTTP filter context. More precisely, this adds a new ABI function:
Risk Level: low
Testing: unit+integration
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a