-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
downstream: refactoring code to remove listener hard deps #24394
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,8 +12,9 @@ namespace Matching { | |
namespace Actions { | ||
namespace FormatString { | ||
|
||
const Network::FilterChain* ActionImpl::get(const Server::FilterChainsByName& filter_chains_by_name, | ||
const StreamInfo::StreamInfo& info) const { | ||
const Network::FilterChain* | ||
ActionImpl::get(const Server::Configuration::FilterChainsByName& filter_chains_by_name, | ||
const StreamInfo::StreamInfo& info) const { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of curiosity, why did this line change? It looks identical but just re-wrapped. Did clang-format dislike the old formatting, I guess? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Server::FilterChainsByName -> Server::Configuration::FilterChainsByName There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wow! I read that probably 10 times before sending that comment and yet still failed to notice the difference! facepalm Thanks! |
||
const std::string name = | ||
formatter_->format(*Http::StaticEmptyHeaders::get().request_headers, | ||
*Http::StaticEmptyHeaders::get().response_headers, | ||
|
@@ -27,7 +28,7 @@ const Network::FilterChain* ActionImpl::get(const Server::FilterChainsByName& fi | |
|
||
Matcher::ActionFactoryCb | ||
ActionFactory::createActionFactoryCb(const Protobuf::Message& proto_config, | ||
Server::FilterChainActionFactoryContext& context, | ||
FilterChainActionFactoryContext& context, | ||
ProtobufMessage::ValidationVisitor& validator) { | ||
const auto& config = | ||
MessageUtil::downcastAndValidate<const envoy::config::core::v3::SubstitutionFormatString&>( | ||
|
@@ -37,7 +38,7 @@ ActionFactory::createActionFactoryCb(const Protobuf::Message& proto_config, | |
return [formatter]() { return std::make_unique<ActionImpl>(formatter); }; | ||
} | ||
|
||
REGISTER_FACTORY(ActionFactory, Matcher::ActionFactory<Server::FilterChainActionFactoryContext>); | ||
REGISTER_FACTORY(ActionFactory, Matcher::ActionFactory<FilterChainActionFactoryContext>); | ||
|
||
} // namespace FormatString | ||
} // namespace Actions | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
#pragma once | ||
|
||
#include "envoy/stats/scope.h" | ||
#include "envoy/stats/stats_macros.h" | ||
|
||
namespace Envoy { | ||
namespace Server { | ||
|
||
#define ALL_LISTENER_STATS(COUNTER, GAUGE, HISTOGRAM) \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding a brief comment here. |
||
COUNTER(downstream_cx_destroy) \ | ||
COUNTER(downstream_cx_overflow) \ | ||
COUNTER(downstream_cx_total) \ | ||
COUNTER(downstream_cx_transport_socket_connect_timeout) \ | ||
COUNTER(downstream_cx_overload_reject) \ | ||
COUNTER(downstream_global_cx_overflow) \ | ||
COUNTER(downstream_pre_cx_timeout) \ | ||
COUNTER(downstream_listener_filter_remote_close) \ | ||
COUNTER(downstream_listener_filter_error) \ | ||
COUNTER(no_filter_chain_match) \ | ||
GAUGE(downstream_cx_active, Accumulate) \ | ||
GAUGE(downstream_pre_cx_active, Accumulate) \ | ||
HISTOGRAM(downstream_cx_length_ms, Milliseconds) | ||
|
||
/** | ||
* Wrapper struct for listener stats. @see stats_macros.h | ||
*/ | ||
struct ListenerStats { | ||
ALL_LISTENER_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT, GENERATE_HISTOGRAM_STRUCT) | ||
}; | ||
|
||
#define ALL_PER_HANDLER_LISTENER_STATS(COUNTER, GAUGE) \ | ||
COUNTER(downstream_cx_total) \ | ||
GAUGE(downstream_cx_active, Accumulate) | ||
|
||
/** | ||
* Wrapper struct for per-handler listener stats. @see stats_macros.h | ||
*/ | ||
struct PerHandlerListenerStats { | ||
ALL_PER_HANDLER_LISTENER_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT) | ||
}; | ||
|
||
} // namespace Server | ||
} // namespace Envoy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in retrospect I should have commented this up when moving it. If there end up being no other comments I'm inclined to do in the immediate follow-up to spare the 4 hour CI process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM! I did have one additional comment but it was to add a comment, so feel free to address both in a follow up instead of waiting on CI again!