Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
98d6a27
add stats macros for declaring and using a separate struct of StatNames.
jmarantz Nov 14, 2020
618834e
macro-ize generation of stat-names structure.
jmarantz Nov 14, 2020
bd5c8a8
all tests pass
jmarantz Nov 15, 2020
50976f1
format
jmarantz Nov 15, 2020
56259be
cleanups
jmarantz Nov 15, 2020
f6b5fd5
Merge branch 'master' into cluster-stats
jmarantz Nov 15, 2020
6b0c333
comment cleanups
jmarantz Nov 15, 2020
d798bb0
Merge branch 'master' into cluster-stats
jmarantz Nov 17, 2020
d437b12
add unit tests and deal with histograms and text readouts.
jmarantz Dec 1, 2020
6d1e6c7
factor our Router::FilterConfig's stat names to avoid taking contende…
jmarantz Dec 2, 2020
63d0d21
keep a ref to stat_names_ in the generated stats structure.
jmarantz Dec 2, 2020
a34cb47
Save the http async-client prefix stat name in the HTTP context so it…
jmarantz Dec 2, 2020
c046273
symbolize stats prefix at callsite.
jmarantz Dec 2, 2020
11e9bd7
remove superfluous forward decl
jmarantz Dec 2, 2020
c4bb1f8
Rename stats structures as they are not really manager stats.
jmarantz Dec 3, 2020
9060912
plumb through a new RouterContext object where needed, to hold the ro…
jmarantz Dec 3, 2020
0498546
don't return a StatName by value from an interface.
jmarantz Dec 3, 2020
02c56ae
Merge branch 'master' into cluster-stats
jmarantz Dec 3, 2020
e845693
revert naming change from CLUSTER_MANAGER to UPSTREAM
jmarantz Dec 3, 2020
5f6b47b
save cluster stat names.
jmarantz Dec 4, 2020
f702021
format, and back out a few superfluous changes
jmarantz Dec 4, 2020
588d16e
back out ClusterManagerStatNames, as there's only one of those per pr…
jmarantz Dec 4, 2020
a595f32
prefer TestStore to MockIsolatedStatsStore to get the global symbol t…
jmarantz Dec 4, 2020
4554d16
clang-tidy error
jmarantz Dec 5, 2020
5678a02
declare some member vars as const, and add a test that http.async-cli…
jmarantz Dec 7, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/envoy/http/context.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class Context {

virtual CodeStats& codeStats() PURE;
virtual const UserAgentContext& userAgentContext() const PURE;
virtual const Stats::StatName& asyncClientStatPrefix() const PURE;
};

using ContextPtr = std::unique_ptr<Context>;
Expand Down
5 changes: 5 additions & 0 deletions include/envoy/router/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ licenses(["notice"]) # Apache 2

envoy_package()

envoy_cc_library(
name = "context_interface",
hdrs = ["context.h"],
)

envoy_cc_library(
name = "rds_interface",
hdrs = ["rds.h"],
Expand Down
21 changes: 21 additions & 0 deletions include/envoy/router/context.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#pragma once

#include "envoy/common/pure.h"

namespace Envoy {
namespace Router {

struct StatNames;

class Context {
public:
virtual ~Context() = default;

/**
* @return a struct containing StatNames for router stats.
*/
virtual const StatNames& statNames() const PURE;
};

} // namespace Router
} // namespace Envoy
1 change: 1 addition & 0 deletions include/envoy/server/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ envoy_cc_library(
"//include/envoy/init:manager_interface",
"//include/envoy/local_info:local_info_interface",
"//include/envoy/network:drain_decision_interface",
"//include/envoy/router:context_interface",
"//include/envoy/runtime:runtime_interface",
"//include/envoy/server/overload:overload_manager_interface",
"//include/envoy/singleton:manager_interface",
Expand Down
11 changes: 11 additions & 0 deletions include/envoy/server/factory_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "envoy/init/manager.h"
#include "envoy/network/drain_decision.h"
#include "envoy/network/filter.h"
#include "envoy/router/context.h"
#include "envoy/runtime/runtime.h"
#include "envoy/server/admin.h"
#include "envoy/server/drain_manager.h"
Expand Down Expand Up @@ -115,6 +116,11 @@ class ServerFactoryContext : public virtual CommonFactoryContext {
*/
virtual Grpc::Context& grpcContext() PURE;

/**
* @return Router::Context& a reference to the router context.
*/
virtual Router::Context& routerContext() PURE;

/**
* @return DrainManager& the server-wide drain manager.
*/
Expand Down Expand Up @@ -223,6 +229,11 @@ class FactoryContext : public virtual CommonFactoryContext {
*/
virtual Grpc::Context& grpcContext() PURE;

/**
* @return Router::Context& a reference to the router context.
*/
virtual Router::Context& routerContext() PURE;

/**
* @return ProcessContextOptRef an optional reference to the
* process context. Will be unset when running in validation mode.
Expand Down
5 changes: 5 additions & 0 deletions include/envoy/server/instance.h
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,11 @@ class Instance {
*/
virtual Http::Context& httpContext() PURE;

/**
* @return the server-wide router context.
*/
virtual Router::Context& routerContext() PURE;

/**
* @return the server-wide process context.
*/
Expand Down
6 changes: 5 additions & 1 deletion include/envoy/stats/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@ envoy_cc_library(
envoy_cc_library(
name = "stats_macros",
hdrs = ["stats_macros.h"],
deps = [":stats_interface"],
deps = [
":stats_interface",
"//source/common/stats:symbol_table_lib",
"//source/common/stats:utility_lib",
],
)

envoy_cc_library(
Expand Down
95 changes: 95 additions & 0 deletions include/envoy/stats/stats_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
#include "envoy/stats/histogram.h"
#include "envoy/stats/stats.h"

#include "common/stats/symbol_table_impl.h"
#include "common/stats/utility.h"

#include "absl/strings/match.h"
#include "absl/strings/str_cat.h"

Expand Down Expand Up @@ -33,6 +36,36 @@ namespace Envoy {
* Finally, when you want to actually instantiate the above struct using a Stats::Pool, you do:
* MyCoolStats stats{
* MY_COOL_STATS(POOL_COUNTER(...), POOL_GAUGE(...), POOL_HISTOGRAM(...))};
*
*
* The above constructs are the simplest way to declare counters, gauges,
* histograms, and text-readouts in your data structures. However they incur
* some overhead to symbolize the names every time they are instantiated. For
* data structures that are re-instantiated extensively during operation,
* e.g. in response to an xDS update, We can separately instantiate a
* StatNameStruct, containing symbolized names for each stat. That should be
* instantiated once at startup and held in some context or factory. Do that
* with:
*
* MAKE_STAT_NAMES_STRUCT(MyStatNames, MY_COOL_STATS);
*
* This generates a structure definition with a constructor that requires a
* SymbolTable. So you must, in a context instantiated once, initialize with:
*
* : my_cool_stat_names_(stat_store.symbolTable())
*
* Once you have the StatNamesStruct instance declared, you can create a stats
* struct efficiently during operation (e.g. in an xDS handler) with
*
* MAKE_STATS_STRUCT(MyStats, MyStatNames, MY_COOL_STATS);
*
* This new structure is constructed with 2 or 3 args:
* 1. The stat_names struct created from MAKE_STAT_NAMES_STRUCT
* 2. The scope in which to instantiate the stats
* 3. An optional prefix, which will be prepended to each stat name.
* For example:
*
* : my_cool_stats_(context.my_cool_stat_names_, scope, opt_prefix)
*/

// Fully-qualified for use in external callsites.
Expand All @@ -59,6 +92,7 @@ static inline std::string statPrefixJoin(absl::string_view prefix, absl::string_
#define POOL_GAUGE_PREFIX(POOL, PREFIX) (POOL).gaugeFromString(Envoy::statPrefixJoin(PREFIX, FINISH_STAT_DECL_MODE_
#define POOL_HISTOGRAM_PREFIX(POOL, PREFIX) (POOL).histogramFromString(Envoy::statPrefixJoin(PREFIX, FINISH_STAT_DECL_UNIT_
#define POOL_TEXT_READOUT_PREFIX(POOL, PREFIX) (POOL).textReadoutFromString(Envoy::statPrefixJoin(PREFIX, FINISH_STAT_DECL_
#define POOL_STAT_NAME_PREFIX(POOL, PREFIX) (POOL).symbolTable().textReadoutFromString(Envoy::statPrefixJoin(PREFIX, FINISH_STAT_DECL_

#define POOL_COUNTER(POOL) POOL_COUNTER_PREFIX(POOL, "")
#define POOL_GAUGE(POOL) POOL_GAUGE_PREFIX(POOL, "")
Expand All @@ -69,4 +103,65 @@ static inline std::string statPrefixJoin(absl::string_view prefix, absl::string_
#define NULL_STAT_DECL_IGNORE_MODE_(X, MODE) std::string(#X)),

#define NULL_POOL_GAUGE(POOL) (POOL).nullGauge(NULL_STAT_DECL_IGNORE_MODE_

// Used for declaring StatNames in a structure.
#define GENERATE_STAT_NAME_STRUCT(NAME, ...) Envoy::Stats::StatName NAME##_;
#define GENERATE_STAT_NAME_INIT(NAME, ...) , NAME##_(pool_.add(#NAME))

// Macros for declaring stat-structures using StatNames, for those that must be
// instantiated during operation, and where speed and scale matters. These
// macros are not for direct use; they are only for use from
// MAKE_STAT_NAMES_STRUCT. and MAKE_STAT_STRUCT.
#define MAKE_STATS_STRUCT_COUNTER_HELPER_(NAME) \
, NAME##_(Envoy::Stats::Utility::counterFromStatNames(scope, {prefix, stat_names.NAME##_}))
#define MAKE_STATS_STRUCT_GAUGE_HELPER_(NAME, MODE) \
, NAME##_(Envoy::Stats::Utility::gaugeFromStatNames(scope, {prefix, stat_names.NAME##_}, \
Envoy::Stats::Gauge::ImportMode::MODE))
#define MAKE_STATS_STRUCT_HISTOGRAM_HELPER_(NAME, UNIT) \
, NAME##_(Envoy::Stats::Utility::histogramFromStatNames(scope, {prefix, stat_names.NAME##_}, \
Envoy::Stats::Histogram::Unit::UNIT))
#define MAKE_STATS_STRUCT_TEXT_READOUT_HELPER_(NAME) \
, NAME##_(Envoy::Stats::Utility::textReadoutFromStatNames(scope, {prefix, stat_names.NAME##_}))

#define MAKE_STATS_STRUCT_STATNAME_HELPER_(name)
#define GENERATE_STATNAME_STRUCT(name)

/**
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that MAKE_STAT_NAMES_STRUCT and MAKE_STATS_STRUCT do not follow the pattern of the existing macros as they require distinct initializers based on data constructed from the ctor params. There's also a bit of hackery where the HELPER_ functions above depend on formal-param names established in other macros. I couldn't figure out how to make something like your FINISH_DECL macros work for this.

* Generates a struct with StatNames for a subsystem, based on the stats macro
* with COUNTER, GAUGE, HISTOGRAM, TEXT_READOUT, and STATNAME calls. The
* ALL_STATS macro must have all 5 parameters.
*/
#define MAKE_STAT_NAMES_STRUCT(StatNamesStruct, ALL_STATS) \
struct StatNamesStruct { \
explicit StatNamesStruct(Envoy::Stats::SymbolTable& symbol_table) \
: pool_(symbol_table) \
ALL_STATS(GENERATE_STAT_NAME_INIT, GENERATE_STAT_NAME_INIT, GENERATE_STAT_NAME_INIT, \
GENERATE_STAT_NAME_INIT, GENERATE_STAT_NAME_INIT) {} \
Envoy::Stats::StatNamePool pool_; \
ALL_STATS(GENERATE_STAT_NAME_STRUCT, GENERATE_STAT_NAME_STRUCT, GENERATE_STAT_NAME_STRUCT, \
GENERATE_STAT_NAME_STRUCT, GENERATE_STAT_NAME_STRUCT) \
}

/**
* Instantiates a structure of stats based on a new scope and optional prefix,
* using a predefined structure of stat names. A reference to the stat_names is
* also stored in the structure, for two reasons: (a) as a syntactic convenience
* for using macros to generate the comma separators for the initializer and (b)
* as a convenience at the call-site to access STATNAME-declared names from the
* stats structure.
*/
#define MAKE_STATS_STRUCT(StatsStruct, StatNamesStruct, ALL_STATS) \
struct StatsStruct { \
StatsStruct(const StatNamesStruct& stat_names, Envoy::Stats::Scope& scope, \
Envoy::Stats::StatName prefix = Envoy::Stats::StatName()) \
: stat_names_(stat_names) \
ALL_STATS(MAKE_STATS_STRUCT_COUNTER_HELPER_, MAKE_STATS_STRUCT_GAUGE_HELPER_, \
MAKE_STATS_STRUCT_HISTOGRAM_HELPER_, \
MAKE_STATS_STRUCT_TEXT_READOUT_HELPER_, \
MAKE_STATS_STRUCT_STATNAME_HELPER_) {} \
const StatNamesStruct& stat_names_; \
ALL_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT, GENERATE_HISTOGRAM_STRUCT, \
GENERATE_TEXT_READOUT_STRUCT, GENERATE_STATNAME_STRUCT) \
}

} // namespace Envoy
11 changes: 11 additions & 0 deletions include/envoy/upstream/cluster_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "envoy/singleton/manager.h"
#include "envoy/ssl/context_manager.h"
#include "envoy/stats/store.h"
#include "envoy/stats/symbol_table.h"
#include "envoy/tcp/conn_pool.h"
#include "envoy/thread_local/thread_local.h"
#include "envoy/upstream/health_checker.h"
Expand Down Expand Up @@ -334,6 +335,16 @@ class ClusterManager {
* @return Config::SubscriptionFactory& the subscription factory.
*/
virtual Config::SubscriptionFactory& subscriptionFactory() PURE;

/**
* Returns a struct with all the Stats::StatName objects needed by
* Clusters. This helps factor out some relatively heavy name
* construction which occur when there is a large CDS update during operation,
* relative to recreating all stats from strings on-the-fly.
*
* @return the stat names.
*/
virtual const ClusterStatNames& clusterStatNames() const PURE;
};

using ClusterManagerPtr = std::unique_ptr<ClusterManager>;
Expand Down
7 changes: 3 additions & 4 deletions include/envoy/upstream/upstream.h
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ class PrioritySet {
/**
* All cluster stats. @see stats_macros.h
*/
#define ALL_CLUSTER_STATS(COUNTER, GAUGE, HISTOGRAM) \
#define ALL_CLUSTER_STATS(COUNTER, GAUGE, HISTOGRAM, TEXT_READOUT, STATNAME) \
COUNTER(assignment_stale) \
COUNTER(assignment_timeout_received) \
COUNTER(bind_errors) \
Expand Down Expand Up @@ -643,9 +643,8 @@ class PrioritySet {
/**
* Struct definition for all cluster stats. @see stats_macros.h
*/
struct ClusterStats {
ALL_CLUSTER_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT, GENERATE_HISTOGRAM_STRUCT)
};
MAKE_STAT_NAMES_STRUCT(ClusterStatNames, ALL_CLUSTER_STATS);
MAKE_STATS_STRUCT(ClusterStats, ClusterStatNames, ALL_CLUSTER_STATS);

/**
* Struct definition for all cluster load report stats. @see stats_macros.h
Expand Down
1 change: 1 addition & 0 deletions source/common/http/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ envoy_cc_library(
"//include/envoy/http:context_interface",
"//include/envoy/http:header_map_interface",
"//include/envoy/http:message_interface",
"//include/envoy/router:context_interface",
"//include/envoy/router:router_interface",
"//include/envoy/router:router_ratelimit_interface",
"//include/envoy/router:shadow_writer_interface",
Expand Down
8 changes: 4 additions & 4 deletions source/common/http/async_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ AsyncClientImpl::AsyncClientImpl(Upstream::ClusterInfoConstSharedPtr cluster,
Upstream::ClusterManager& cm, Runtime::Loader& runtime,
Random::RandomGenerator& random,
Router::ShadowWriterPtr&& shadow_writer,
Http::Context& http_context)
: cluster_(cluster), config_("http.async-client.", local_info, stats_store, cm, runtime, random,
std::move(shadow_writer), true, false, false, false, {},
dispatcher.timeSource(), http_context),
Http::Context& http_context, Router::Context& router_context)
: cluster_(cluster), config_(http_context.asyncClientStatPrefix(), local_info, stats_store, cm,
runtime, random, std::move(shadow_writer), true, false, false,
false, {}, dispatcher.timeSource(), http_context, router_context),
dispatcher_(dispatcher) {}

AsyncClientImpl::~AsyncClientImpl() {
Expand Down
3 changes: 2 additions & 1 deletion source/common/http/async_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "envoy/http/context.h"
#include "envoy/http/header_map.h"
#include "envoy/http/message.h"
#include "envoy/router/context.h"
#include "envoy/router/router.h"
#include "envoy/router/router_ratelimit.h"
#include "envoy/router/shadow_writer.h"
Expand Down Expand Up @@ -49,7 +50,7 @@ class AsyncClientImpl final : public AsyncClient {
Event::Dispatcher& dispatcher, const LocalInfo::LocalInfo& local_info,
Upstream::ClusterManager& cm, Runtime::Loader& runtime,
Random::RandomGenerator& random, Router::ShadowWriterPtr&& shadow_writer,
Http::Context& http_context);
Http::Context& http_context, Router::Context& router_context);
~AsyncClientImpl() override;

// Http::AsyncClient
Expand Down
3 changes: 2 additions & 1 deletion source/common/http/context_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ namespace Envoy {
namespace Http {

ContextImpl::ContextImpl(Stats::SymbolTable& symbol_table)
: code_stats_(symbol_table), user_agent_context_(symbol_table) {}
: code_stats_(symbol_table), user_agent_context_(symbol_table),
async_client_stat_prefix_(user_agent_context_.pool_.add("http.async-client")) {}

} // namespace Http
} // namespace Envoy
4 changes: 4 additions & 0 deletions source/common/http/context_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,14 @@ class ContextImpl : public Context {
}

const UserAgentContext& userAgentContext() const override { return user_agent_context_; }
const Stats::StatName& asyncClientStatPrefix() const override {
return async_client_stat_prefix_;
}

private:
CodeStatsImpl code_stats_;
UserAgentContext user_agent_context_;
const Stats::StatName async_client_stat_prefix_;
envoy::config::trace::v3::Tracing default_tracing_config_;
};

Expand Down
11 changes: 11 additions & 0 deletions source/common/router/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,16 @@ envoy_cc_library(
],
)

envoy_cc_library(
name = "context_lib",
srcs = ["context_impl.cc"],
hdrs = ["context_impl.h"],
deps = [
"//include/envoy/router:context_interface",
"//include/envoy/stats:stats_macros",
],
)

envoy_cc_library(
name = "debug_config_lib",
srcs = ["debug_config.cc"],
Expand Down Expand Up @@ -270,6 +280,7 @@ envoy_cc_library(
],
deps = [
":config_lib",
":context_lib",
":debug_config_lib",
":header_parser_lib",
":retry_state_lib",
Expand Down
9 changes: 9 additions & 0 deletions source/common/router/context_impl.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#include "common/router/context_impl.h"

namespace Envoy {
namespace Router {

ContextImpl::ContextImpl(Stats::SymbolTable& symbol_table) : stat_names_(symbol_table) {}

} // namespace Router
} // namespace Envoy
Loading