Skip to content

Commit

Permalink
logger: use server stats scope at grpc logger (#18067)
Browse files Browse the repository at this point in the history
Signed-off-by: Yuchen Dai <[email protected]>
  • Loading branch information
lambdai authored Oct 1, 2021
1 parent b2db7a9 commit 34edbca
Show file tree
Hide file tree
Showing 38 changed files with 175 additions and 97 deletions.
5 changes: 5 additions & 0 deletions envoy/server/factory_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ class FactoryContextBase {
*/
virtual Stats::Scope& scope() PURE;

/**
* @return Stats::Scope& the server wide stats scope.
*/
virtual Stats::Scope& serverScope() PURE;

/**
* @return ThreadLocal::SlotAllocator& the thread local storage engine for the server. This is
* used to allow runtime lockless updates to configuration, etc. across multiple threads.
Expand Down
2 changes: 2 additions & 0 deletions envoy/upstream/cluster_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ class ClusterFactoryContext : public Server::Configuration::FactoryContextBase {

// Server::Configuration::FactoryContextBase
Stats::Scope& scope() override { return stats(); }

Stats::Scope& serverScope() override { return stats(); }
};

/**
Expand Down
7 changes: 5 additions & 2 deletions source/common/upstream/upstream_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -676,8 +676,9 @@ class FactoryContextImpl : public Server::Configuration::CommonFactoryContext {
// other contexts taken from TransportSocketFactoryContext.
FactoryContextImpl(Stats::Scope& stats_scope, Envoy::Runtime::Loader& runtime,
Server::Configuration::TransportSocketFactoryContext& c)
: admin_(c.admin()), stats_scope_(stats_scope), cluster_manager_(c.clusterManager()),
local_info_(c.localInfo()), dispatcher_(c.mainThreadDispatcher()), runtime_(runtime),
: admin_(c.admin()), server_scope_(c.stats()), stats_scope_(stats_scope),
cluster_manager_(c.clusterManager()), local_info_(c.localInfo()),
dispatcher_(c.mainThreadDispatcher()), runtime_(runtime),
singleton_manager_(c.singletonManager()), tls_(c.threadLocal()), api_(c.api()),
options_(c.options()), message_validation_visitor_(c.messageValidationVisitor()) {}

Expand All @@ -687,6 +688,7 @@ class FactoryContextImpl : public Server::Configuration::CommonFactoryContext {
const LocalInfo::LocalInfo& localInfo() const override { return local_info_; }
Envoy::Runtime::Loader& runtime() override { return runtime_; }
Stats::Scope& scope() override { return stats_scope_; }
Stats::Scope& serverScope() override { return server_scope_; }
Singleton::Manager& singletonManager() override { return singleton_manager_; }
ThreadLocal::SlotAllocator& threadLocal() override { return tls_; }
Server::Admin& admin() override { return admin_; }
Expand Down Expand Up @@ -719,6 +721,7 @@ class FactoryContextImpl : public Server::Configuration::CommonFactoryContext {

private:
Server::Admin& admin_;
Stats::Scope& server_scope_;
Stats::Scope& stats_scope_;
Upstream::ClusterManager& cluster_manager_;
const LocalInfo::LocalInfo& local_info_;
Expand Down
21 changes: 10 additions & 11 deletions source/extensions/access_loggers/common/grpc_access_logger.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,8 @@ template <typename GrpcAccessLogger, typename ConfigProto> class GrpcAccessLogge
* @param config supplies the configuration for the logger.
* @return GrpcAccessLoggerSharedPtr ready for logging requests.
*/
virtual typename GrpcAccessLogger::SharedPtr getOrCreateLogger(const ConfigProto& config,
GrpcAccessLoggerType logger_type,
Stats::Scope& scope) PURE;
virtual typename GrpcAccessLogger::SharedPtr
getOrCreateLogger(const ConfigProto& config, GrpcAccessLoggerType logger_type) PURE;
};

template <typename LogRequest, typename LogResponse> class GrpcAccessLogClient {
Expand Down Expand Up @@ -252,15 +251,14 @@ class GrpcAccessLoggerCache : public Singleton::Instance,

GrpcAccessLoggerCache(Grpc::AsyncClientManager& async_client_manager, Stats::Scope& scope,
ThreadLocal::SlotAllocator& tls)
: async_client_manager_(async_client_manager), scope_(scope), tls_slot_(tls.allocateSlot()) {
: scope_(scope), async_client_manager_(async_client_manager), tls_slot_(tls.allocateSlot()) {
tls_slot_->set([](Event::Dispatcher& dispatcher) {
return std::make_shared<ThreadLocalCache>(dispatcher);
});
}

typename GrpcAccessLogger::SharedPtr getOrCreateLogger(const ConfigProto& config,
GrpcAccessLoggerType logger_type,
Stats::Scope& scope) override {
typename GrpcAccessLogger::SharedPtr
getOrCreateLogger(const ConfigProto& config, GrpcAccessLoggerType logger_type) override {
// TODO(euroelessar): Consider cleaning up loggers.
auto& cache = tls_slot_->getTyped<ThreadLocalCache>();
const auto cache_key = std::make_pair(MessageUtil::hash(config), logger_type);
Expand All @@ -277,12 +275,14 @@ class GrpcAccessLoggerCache : public Singleton::Instance,
const auto logger = createLogger(
config, std::move(client),
std::chrono::milliseconds(PROTOBUF_GET_MS_OR_DEFAULT(config, buffer_flush_interval, 1000)),
PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, buffer_size_bytes, 16384), cache.dispatcher_,
scope);
PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, buffer_size_bytes, 16384), cache.dispatcher_);
cache.access_loggers_.emplace(cache_key, logger);
return logger;
}

protected:
Stats::Scope& scope_;

private:
/**
* Per-thread cache.
Expand All @@ -301,10 +301,9 @@ class GrpcAccessLoggerCache : public Singleton::Instance,
virtual typename GrpcAccessLogger::SharedPtr
createLogger(const ConfigProto& config, const Grpc::RawAsyncClientSharedPtr& client,
std::chrono::milliseconds buffer_flush_interval_msec, uint64_t max_buffer_size_bytes,
Event::Dispatcher& dispatcher, Stats::Scope& scope) PURE;
Event::Dispatcher& dispatcher) PURE;

Grpc::AsyncClientManager& async_client_manager_;
Stats::Scope& scope_;
ThreadLocal::SlotPtr tls_slot_;
};

Expand Down
2 changes: 1 addition & 1 deletion source/extensions/access_loggers/grpc/config_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ getGrpcAccessLoggerCacheSingleton(Server::Configuration::CommonFactoryContext& c
return context.singletonManager().getTyped<GrpcCommon::GrpcAccessLoggerCacheImpl>(
SINGLETON_MANAGER_REGISTERED_NAME(grpc_access_logger_cache), [&context] {
return std::make_shared<GrpcCommon::GrpcAccessLoggerCacheImpl>(
context.clusterManager().grpcAsyncClientManager(), context.scope(),
context.clusterManager().grpcAsyncClientManager(), context.serverScope(),
context.threadLocal(), context.localInfo());
});
}
Expand Down
4 changes: 2 additions & 2 deletions source/extensions/access_loggers/grpc/grpc_access_log_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ GrpcAccessLoggerImpl::SharedPtr GrpcAccessLoggerCacheImpl::createLogger(
const envoy::extensions::access_loggers::grpc::v3::CommonGrpcAccessLogConfig& config,
const Grpc::RawAsyncClientSharedPtr& client,
std::chrono::milliseconds buffer_flush_interval_msec, uint64_t max_buffer_size_bytes,
Event::Dispatcher& dispatcher, Stats::Scope& scope) {
Event::Dispatcher& dispatcher) {
return std::make_shared<GrpcAccessLoggerImpl>(client, config.log_name(),
buffer_flush_interval_msec, max_buffer_size_bytes,
dispatcher, local_info_, scope);
dispatcher, local_info_, scope_);
}

} // namespace GrpcCommon
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class GrpcAccessLoggerCacheImpl
createLogger(const envoy::extensions::access_loggers::grpc::v3::CommonGrpcAccessLogConfig& config,
const Grpc::RawAsyncClientSharedPtr& client,
std::chrono::milliseconds buffer_flush_interval_msec, uint64_t max_buffer_size_bytes,
Event::Dispatcher& dispatcher, Stats::Scope& scope) override;
Event::Dispatcher& dispatcher) override;

const LocalInfo::LocalInfo& local_info_;
};
Expand Down
6 changes: 3 additions & 3 deletions source/extensions/access_loggers/grpc/http_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ AccessLog::InstanceSharedPtr HttpGrpcAccessLogFactory::createAccessLogInstance(
if (service_config.has_envoy_grpc()) {
context.clusterManager().checkActiveStaticCluster(service_config.envoy_grpc().cluster_name());
}
return std::make_shared<HttpGrpcAccessLog>(std::move(filter), proto_config, context.threadLocal(),
GrpcCommon::getGrpcAccessLoggerCacheSingleton(context),
context.scope());
return std::make_shared<HttpGrpcAccessLog>(
std::move(filter), proto_config, context.threadLocal(),
GrpcCommon::getGrpcAccessLoggerCacheSingleton(context));
}

ProtobufTypes::MessagePtr HttpGrpcAccessLogFactory::createEmptyConfigProto() {
Expand Down
17 changes: 7 additions & 10 deletions source/extensions/access_loggers/grpc/http_grpc_access_log_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,8 @@ HttpGrpcAccessLog::ThreadLocalLogger::ThreadLocalLogger(
HttpGrpcAccessLog::HttpGrpcAccessLog(AccessLog::FilterPtr&& filter,
const HttpGrpcAccessLogConfig config,
ThreadLocal::SlotAllocator& tls,
GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache,
Stats::Scope& scope)
: Common::ImplBase(std::move(filter)), scope_(scope),
GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache)
: Common::ImplBase(std::move(filter)),
config_(std::make_shared<const HttpGrpcAccessLogConfig>(std::move(config))),
tls_slot_(tls.allocateSlot()), access_logger_cache_(std::move(access_logger_cache)) {
for (const auto& header : config_->additional_request_headers_to_log()) {
Expand All @@ -43,13 +42,11 @@ HttpGrpcAccessLog::HttpGrpcAccessLog(AccessLog::FilterPtr&& filter,
response_trailers_to_log_.emplace_back(header);
}
Envoy::Config::Utility::checkTransportVersion(config_->common_config());
// Note that &scope might have died by the time when this callback is called on each thread.
// This is supposed to be fixed by https://github.com/envoyproxy/envoy/issues/18066.
tls_slot_->set([config = config_, access_logger_cache = access_logger_cache_,
&scope = scope_](Event::Dispatcher&) {
return std::make_shared<ThreadLocalLogger>(access_logger_cache->getOrCreateLogger(
config->common_config(), Common::GrpcAccessLoggerType::HTTP, scope));
});
tls_slot_->set(
[config = config_, access_logger_cache = access_logger_cache_](Event::Dispatcher&) {
return std::make_shared<ThreadLocalLogger>(access_logger_cache->getOrCreateLogger(
config->common_config(), Common::GrpcAccessLoggerType::HTTP));
});
}

void HttpGrpcAccessLog::emitLog(const Http::RequestHeaderMap& request_headers,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ class HttpGrpcAccessLog : public Common::ImplBase {
public:
HttpGrpcAccessLog(AccessLog::FilterPtr&& filter, const HttpGrpcAccessLogConfig config,
ThreadLocal::SlotAllocator& tls,
GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache,
Stats::Scope& scope);
GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache);

private:
/**
Expand All @@ -50,7 +49,6 @@ class HttpGrpcAccessLog : public Common::ImplBase {
const Http::ResponseTrailerMap& response_trailers,
const StreamInfo::StreamInfo& stream_info) override;

Stats::Scope& scope_;
const HttpGrpcAccessLogConfigConstSharedPtr config_;
const ThreadLocal::SlotPtr tls_slot_;
const GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache_;
Expand Down
3 changes: 1 addition & 2 deletions source/extensions/access_loggers/grpc/tcp_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ AccessLog::InstanceSharedPtr TcpGrpcAccessLogFactory::createAccessLogInstance(
context.clusterManager().checkActiveStaticCluster(service_config.envoy_grpc().cluster_name());
}
return std::make_shared<TcpGrpcAccessLog>(std::move(filter), proto_config, context.threadLocal(),
GrpcCommon::getGrpcAccessLoggerCacheSingleton(context),
context.scope());
GrpcCommon::getGrpcAccessLoggerCacheSingleton(context));
}

ProtobufTypes::MessagePtr TcpGrpcAccessLogFactory::createEmptyConfigProto() {
Expand Down
17 changes: 7 additions & 10 deletions source/extensions/access_loggers/grpc/tcp_grpc_access_log_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,16 @@ TcpGrpcAccessLog::ThreadLocalLogger::ThreadLocalLogger(GrpcCommon::GrpcAccessLog
TcpGrpcAccessLog::TcpGrpcAccessLog(AccessLog::FilterPtr&& filter,
const TcpGrpcAccessLogConfig config,
ThreadLocal::SlotAllocator& tls,
GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache,
Stats::Scope& scope)
: Common::ImplBase(std::move(filter)), scope_(scope),
GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache)
: Common::ImplBase(std::move(filter)),
config_(std::make_shared<const TcpGrpcAccessLogConfig>(std::move(config))),
tls_slot_(tls.allocateSlot()), access_logger_cache_(std::move(access_logger_cache)) {
Config::Utility::checkTransportVersion(config_->common_config());
// Note that &scope might have died by the time when this callback is called on each thread.
// This is supposed to be fixed by https://github.com/envoyproxy/envoy/issues/18066.
tls_slot_->set([config = config_, access_logger_cache = access_logger_cache_,
&scope = scope_](Event::Dispatcher&) {
return std::make_shared<ThreadLocalLogger>(access_logger_cache->getOrCreateLogger(
config->common_config(), Common::GrpcAccessLoggerType::TCP, scope));
});
tls_slot_->set(
[config = config_, access_logger_cache = access_logger_cache_](Event::Dispatcher&) {
return std::make_shared<ThreadLocalLogger>(access_logger_cache->getOrCreateLogger(
config->common_config(), Common::GrpcAccessLoggerType::TCP));
});
}

void TcpGrpcAccessLog::emitLog(const Http::RequestHeaderMap&, const Http::ResponseHeaderMap&,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ class TcpGrpcAccessLog : public Common::ImplBase {
public:
TcpGrpcAccessLog(AccessLog::FilterPtr&& filter, const TcpGrpcAccessLogConfig config,
ThreadLocal::SlotAllocator& tls,
GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache,
Stats::Scope& scope);
GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache);

private:
/**
Expand All @@ -49,7 +48,6 @@ class TcpGrpcAccessLog : public Common::ImplBase {
const Http::ResponseTrailerMap& response_trailers,
const StreamInfo::StreamInfo& stream_info) override;

Stats::Scope& scope_;
const TcpGrpcAccessLogConfigConstSharedPtr config_;
const ThreadLocal::SlotPtr tls_slot_;
const GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,14 @@ AccessLog::ThreadLocalLogger::ThreadLocalLogger(GrpcAccessLoggerSharedPtr logger
AccessLog::AccessLog(
::Envoy::AccessLog::FilterPtr&& filter,
envoy::extensions::access_loggers::open_telemetry::v3alpha::OpenTelemetryAccessLogConfig config,
ThreadLocal::SlotAllocator& tls, GrpcAccessLoggerCacheSharedPtr access_logger_cache,
Stats::Scope& scope)
: Common::ImplBase(std::move(filter)), scope_(scope), tls_slot_(tls.allocateSlot()),
ThreadLocal::SlotAllocator& tls, GrpcAccessLoggerCacheSharedPtr access_logger_cache)
: Common::ImplBase(std::move(filter)), tls_slot_(tls.allocateSlot()),
access_logger_cache_(std::move(access_logger_cache)) {

Envoy::Config::Utility::checkTransportVersion(config.common_config());
tls_slot_->set([this, config](Event::Dispatcher&) {
return std::make_shared<ThreadLocalLogger>(access_logger_cache_->getOrCreateLogger(
config.common_config(), Common::GrpcAccessLoggerType::HTTP, scope_));
config.common_config(), Common::GrpcAccessLoggerType::HTTP));
});

ProtobufWkt::Struct body_format;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ class AccessLog : public Common::ImplBase {
AccessLog(::Envoy::AccessLog::FilterPtr&& filter,
envoy::extensions::access_loggers::open_telemetry::v3alpha::OpenTelemetryAccessLogConfig
config,
ThreadLocal::SlotAllocator& tls, GrpcAccessLoggerCacheSharedPtr access_logger_cache,
Stats::Scope& scope);
ThreadLocal::SlotAllocator& tls, GrpcAccessLoggerCacheSharedPtr access_logger_cache);

private:
/**
Expand All @@ -55,7 +54,6 @@ class AccessLog : public Common::ImplBase {
const Http::ResponseTrailerMap& response_trailers,
const StreamInfo::StreamInfo& stream_info) override;

Stats::Scope& scope_;
const ThreadLocal::SlotPtr tls_slot_;
const GrpcAccessLoggerCacheSharedPtr access_logger_cache_;
std::unique_ptr<Formatter::StructFormatter> body_formatter_;
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/access_loggers/open_telemetry/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ AccessLogFactory::createAccessLogInstance(const Protobuf::Message& config,
config, context.messageValidationVisitor());

return std::make_shared<AccessLog>(std::move(filter), proto_config, context.threadLocal(),
getAccessLoggerCacheSingleton(context), context.scope());
getAccessLoggerCacheSingleton(context));
}

ProtobufTypes::MessagePtr AccessLogFactory::createEmptyConfigProto() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@ GrpcAccessLoggerImpl::SharedPtr GrpcAccessLoggerCacheImpl::createLogger(
const envoy::extensions::access_loggers::grpc::v3::CommonGrpcAccessLogConfig& config,
const Grpc::RawAsyncClientSharedPtr& client,
std::chrono::milliseconds buffer_flush_interval_msec, uint64_t max_buffer_size_bytes,
Event::Dispatcher& dispatcher, Stats::Scope& scope) {
Event::Dispatcher& dispatcher) {
return std::make_shared<GrpcAccessLoggerImpl>(client, config.log_name(),
buffer_flush_interval_msec, max_buffer_size_bytes,
dispatcher, local_info_, scope);
dispatcher, local_info_, scope_);
}

} // namespace OpenTelemetry
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class GrpcAccessLoggerCacheImpl
createLogger(const envoy::extensions::access_loggers::grpc::v3::CommonGrpcAccessLogConfig& config,
const Grpc::RawAsyncClientSharedPtr& client,
std::chrono::milliseconds buffer_flush_interval_msec, uint64_t max_buffer_size_bytes,
Event::Dispatcher& dispatcher, Stats::Scope& scope) override;
Event::Dispatcher& dispatcher) override;

const LocalInfo::LocalInfo& local_info_;
};
Expand Down
1 change: 1 addition & 0 deletions source/server/factory_context_base_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class FactoryContextBaseImpl : public Configuration::FactoryContextBase {
return validation_visitor_;
};
Stats::Scope& scope() override { return scope_; };
Stats::Scope& serverScope() override { return scope_; }
ThreadLocal::SlotAllocator& threadLocal() override { return thread_local_; };

private:
Expand Down
2 changes: 2 additions & 0 deletions source/server/filter_chain_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor
const LocalInfo::LocalInfo& localInfo() const override;
Envoy::Runtime::Loader& runtime() override;
Stats::Scope& scope() override;
Stats::Scope& serverScope() override { return parent_context_.serverScope(); }
Singleton::Manager& singletonManager() override;
OverloadManager& overloadManager() override;
ThreadLocal::SlotAllocator& threadLocal() override;
Expand Down Expand Up @@ -151,6 +152,7 @@ class FactoryContextImpl : public Configuration::FactoryContext {
const LocalInfo::LocalInfo& localInfo() const override;
Envoy::Runtime::Loader& runtime() override;
Stats::Scope& scope() override;
Stats::Scope& serverScope() override { return server_.stats(); }
Singleton::Manager& singletonManager() override;
OverloadManager& overloadManager() override;
ThreadLocal::SlotAllocator& threadLocal() override;
Expand Down
2 changes: 2 additions & 0 deletions source/server/listener_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ class ListenerFactoryContextBaseImpl final : public Configuration::FactoryContex
Init::Manager& initManager() override;
const LocalInfo::LocalInfo& localInfo() const override;
Envoy::Runtime::Loader& runtime() override;
Stats::Scope& serverScope() override { return server_.stats(); }
Stats::Scope& scope() override;
Singleton::Manager& singletonManager() override;
OverloadManager& overloadManager() override;
Expand Down Expand Up @@ -188,6 +189,7 @@ class PerListenerFactoryContextImpl : public Configuration::ListenerFactoryConte
const LocalInfo::LocalInfo& localInfo() const override;
Envoy::Runtime::Loader& runtime() override;
Stats::Scope& scope() override;
Stats::Scope& serverScope() override { return listener_factory_context_base_->serverScope(); }
Singleton::Manager& singletonManager() override;
OverloadManager& overloadManager() override;
ThreadLocal::Instance& threadLocal() override;
Expand Down
Loading

0 comments on commit 34edbca

Please sign in to comment.