Route-local fault filter config#3084
Conversation
Signed-off-by: Chris Roche <croche@lyft.com>
Signed-off-by: Chris Roche <croche@lyft.com>
Signed-off-by: Chris Roche <croche@lyft.com>
Signed-off-by: Chris Roche <croche@lyft.com>
Signed-off-by: Chris Roche <croche@lyft.com>
Signed-off-by: Chris Roche <croche@lyft.com>
Signed-off-by: Chris Roche <croche@lyft.com>
Signed-off-by: Chris Roche <croche@lyft.com>
Signed-off-by: Chris Roche <croche@lyft.com>
Signed-off-by: Chris Roche <croche@lyft.com>
…into perfilterconfig Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
…filterconfig Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
…into perfilterconfig Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
Signed-off-by: Chris Roche <croche@lyft.com>
…into perfilterconfig Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
|
@rodaine can you own first review on this. |
…filterconfig Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
…filterconfig Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
|
done.. |
|
Holding on merging until we resolve #3128 (review). cc @ggreenway @rodaine |
Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
|
@rshriram can you merge master and fix this up? Then we can get this in. |
Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
|
@mattklein123 updated |
| envoy::config::filter::http::fault::v2::HTTPFault proto_config; | ||
| MessageUtil::jsonConvert(struct_config, proto_config); | ||
| Router::RouteSpecificFilterConfigConstSharedPtr rconfig; | ||
| rconfig.reset(new Fault::FaultSettings(proto_config)); |
There was a problem hiding this comment.
nit: return std::make_shared<...>(...)
| // faults. In other words, runtime is supported only when faults are | ||
| // configured at the filter level. | ||
| if (callbacks_->route() && callbacks_->route()->routeEntry()) { | ||
| const std::string name = Extensions::HttpFilters::HttpFilterNames::get().FAULT; |
| const FaultSettings* per_route_settings = | ||
| route_entry->perFilterConfigTyped<FaultSettings>(name) | ||
| ?: route_entry->virtualHost().perFilterConfigTyped<FaultSettings>(name); | ||
| config_->updateFaultSettings(per_route_settings); |
There was a problem hiding this comment.
Aren't you effectively updating a global shared pointer here? I don't think this is right? Can you make sure FaultFilterConfigSharedPtr is actually FaultFilterConfigConstSharedPtr to catch this?
There was a problem hiding this comment.
now that I think about it, the reason I did this was because I wanted to avoid allocation in the request path. I couldn't create an identical config object for RouteSpecific.. because I don't have access to runtime/stats/scope etc in that API Call.
There was a problem hiding this comment.
Hmm. Then this is the same problem as Lua. We need to fix this now I guess and be able to pass the full FactoryContext into the factory function. Can you take a look at that?
There was a problem hiding this comment.
ignore please. fixed it
…filterconfig Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
| const FaultSettings settings_; | ||
| }; | ||
|
|
||
| typedef std::shared_ptr<FaultFilterConfig> FaultFilterConfigSharedPtr; |
There was a problem hiding this comment.
Can you change this typedef to std::shared_ptr<const FaultFilterConfig> FaultFilterConfigConstSharedPtr
mattklein123
left a comment
There was a problem hiding this comment.
LGTM other than small comment.
Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
|
@rshriram check OSX failure, it's real compiler error. |
Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
|
so, I cant really do |
Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
|
@rshriram you should be able to fix with a mutable on the stats. |
Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
|
Tests passing finally. |
Follow up from @rodaine's route-local config framework. Enabling per-route/per-vhost config for fault filters. (#3045).
Signed-off-by: Shriram Rajagopalan shriramr@vmware.com