Skip to content

Commit

Permalink
lua: add stats (#22623)
Browse files Browse the repository at this point in the history
Currently stats are not available in lua filter, which makes it hard to
track/alert for the script errors. This change adds error stat.

Signed-off-by: Suresh Kumar <sureshkumar.pp@gmail.com>
Signed-off-by: Suresh Kumar <suresh.ponnusamy@freshworks.com>
  • Loading branch information
surki committed Aug 12, 2022
1 parent 073ed39 commit 8c88943
Show file tree
Hide file tree
Showing 8 changed files with 222 additions and 18 deletions.
20 changes: 20 additions & 0 deletions api/envoy/extensions/filters/http/lua/v3/lua.proto
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,26 @@ message Lua {
// The default Lua code that Envoy will execute. If no per route config is provided
// for the request, this Lua code will be applied.
config.core.v3.DataSource default_source_code = 3;

// Optional additional prefix to use when emitting statistics. By default
// metrics are emitted in *.lua.* namespace. If multiple lua filters are
// configured in a filter chain, the stats from each filter instance can
// be emitted using custom stat prefix to distinguish emitted
// statistics. For example:
//
// .. code-block:: yaml
//
// http_filters:
// - name: envoy.filters.http.lua
// typed_config:
// "@type": type.googleapis.com/envoy.extensions.filters.http.lua.v3.Lua
// stat_prefix: foo_script # This emits lua.foo_script.errors etc.
// - name: envoy.filters.http.lua
// typed_config:
// "@type": type.googleapis.com/envoy.extensions.filters.http.lua.v3.Lua
// stat_prefix: bar_script # This emits lua.bar_script.errors etc.
//
string stat_prefix = 4;
}

message LuaPerRoute {
Expand Down
3 changes: 3 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -104,5 +104,8 @@ new_features:
- area: access_log
change: |
added support for number values in substitution format string in json_format.
- area: lua
change: |
added stats for lua filter, please see :ref:`lua filter stats <config_http_filters_lua_stats>`.
deprecated:
15 changes: 15 additions & 0 deletions docs/root/configuration/http/http_filters/lua_filter.rst
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,21 @@ Lua script as follows:
response_handle:logInfo("Goodbye.")
end
Statistics
----------
.. _config_http_filters_lua_stats:

The lua filter outputs statistics in the *.lua.* namespace by default. When
there are multiple lua filters configured in a filter chain, stats from
individual filter instance/script can be tracked by providing a per filter
:ref:`stat prefix
<envoy_v3_api_field_extensions.filters.http.lua.v3.Lua.stat_prefix>`.

.. csv-table::
:header: Name, Type, Description
:widths: 1, 1, 2

error, Counter, Total script execution errors.

Script examples
---------------
Expand Down
9 changes: 5 additions & 4 deletions source/extensions/filters/http/lua/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@ namespace HttpFilters {
namespace Lua {

Http::FilterFactoryCb LuaFilterConfig::createFilterFactoryFromProtoTyped(
const envoy::extensions::filters::http::lua::v3::Lua& proto_config, const std::string&,
Server::Configuration::FactoryContext& context) {
FilterConfigConstSharedPtr filter_config(new FilterConfig{
proto_config, context.threadLocal(), context.clusterManager(), context.api()});
const envoy::extensions::filters::http::lua::v3::Lua& proto_config,
const std::string& stat_prefix, Server::Configuration::FactoryContext& context) {
FilterConfigConstSharedPtr filter_config(new FilterConfig{proto_config, context.threadLocal(),
context.clusterManager(), context.api(),
context.scope(), stat_prefix});
auto& time_source = context.mainThreadDispatcher().timeSource();
return [filter_config, &time_source](Http::FilterChainFactoryCallbacks& callbacks) -> void {
callbacks.addStreamFilter(std::make_shared<Filter>(filter_config, time_source));
Expand Down
4 changes: 2 additions & 2 deletions source/extensions/filters/http/lua/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ class LuaFilterConfig

private:
Http::FilterFactoryCb createFilterFactoryFromProtoTyped(
const envoy::extensions::filters::http::lua::v3::Lua& proto_config, const std::string&,
Server::Configuration::FactoryContext& context) override;
const envoy::extensions::filters::http::lua::v3::Lua& proto_config,
const std::string& stats_prefix, Server::Configuration::FactoryContext& context) override;

Router::RouteSpecificFilterConfigConstSharedPtr createRouteSpecificFilterConfigTyped(
const envoy::extensions::filters::http::lua::v3::LuaPerRoute& proto_config,
Expand Down
7 changes: 5 additions & 2 deletions source/extensions/filters/http/lua/lua_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -688,8 +688,10 @@ StreamHandleWrapper::getTimestampResolution(absl::string_view unit_parameter) {

FilterConfig::FilterConfig(const envoy::extensions::filters::http::lua::v3::Lua& proto_config,
ThreadLocal::SlotAllocator& tls,
Upstream::ClusterManager& cluster_manager, Api::Api& api)
: cluster_manager_(cluster_manager) {
Upstream::ClusterManager& cluster_manager, Api::Api& api,
Stats::Scope& scope, const std::string& stats_prefix)
: cluster_manager_(cluster_manager),
stats_(generateStats(stats_prefix, proto_config.stat_prefix(), scope)) {
if (proto_config.has_default_source_code()) {
if (!proto_config.inline_code().empty()) {
throw EnvoyException("Error: Only one of `inline_code` or `default_source_code` can be set "
Expand Down Expand Up @@ -793,6 +795,7 @@ Http::FilterTrailersStatus Filter::doTrailers(StreamHandleRef& handle, Http::Hea
}

void Filter::scriptError(const Filters::Common::Lua::LuaException& e) {
stats_.errors_.inc();
scriptLog(spdlog::level::err, e.what());
request_stream_wrapper_.reset();
response_stream_wrapper_.reset();
Expand Down
27 changes: 25 additions & 2 deletions source/extensions/filters/http/lua/lua_filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include "envoy/extensions/filters/http/lua/v3/lua.pb.h"
#include "envoy/http/filter.h"
#include "envoy/stats/stats_macros.h"
#include "envoy/upstream/cluster_manager.h"

#include "source/common/crypto/utility.h"
Expand All @@ -15,6 +16,18 @@ namespace Extensions {
namespace HttpFilters {
namespace Lua {

/**
* All lua stats. @see stats_macros.h
*/
#define ALL_LUA_FILTER_STATS(COUNTER) COUNTER(errors)

/**
* Struct definition for all Lua stats. @see stats_macros.h
*/
struct LuaFilterStats {
ALL_LUA_FILTER_STATS(GENERATE_COUNTER_STRUCT)
};

class PerLuaCodeSetup : Logger::Loggable<Logger::Id::lua> {
public:
PerLuaCodeSetup(const std::string& lua_code, ThreadLocal::SlotAllocator& tls);
Expand Down Expand Up @@ -357,7 +370,7 @@ class FilterConfig : Logger::Loggable<Logger::Id::lua> {
public:
FilterConfig(const envoy::extensions::filters::http::lua::v3::Lua& proto_config,
ThreadLocal::SlotAllocator& tls, Upstream::ClusterManager& cluster_manager,
Api::Api& api);
Api::Api& api, Stats::Scope& scope, const std::string& stat_prefix);

PerLuaCodeSetup* perLuaCodeSetup(absl::optional<absl::string_view> name = absl::nullopt) const {
if (!name.has_value()) {
Expand All @@ -371,11 +384,20 @@ class FilterConfig : Logger::Loggable<Logger::Id::lua> {
return nullptr;
}

const LuaFilterStats& stats() const { return stats_; }

Upstream::ClusterManager& cluster_manager_;

private:
LuaFilterStats generateStats(const std::string& prefix, const std::string& filter_stats_prefix,
Stats::Scope& scope) {
const std::string final_prefix = absl::StrCat(prefix, "lua.", filter_stats_prefix);
return {ALL_LUA_FILTER_STATS(POOL_COUNTER_PREFIX(scope, final_prefix))};
}

PerLuaCodeSetupPtr default_lua_code_setup_;
absl::flat_hash_map<std::string, PerLuaCodeSetupPtr> per_lua_code_setups_map_;
LuaFilterStats stats_;
};

using FilterConfigConstSharedPtr = std::shared_ptr<FilterConfig>;
Expand Down Expand Up @@ -448,7 +470,7 @@ PerLuaCodeSetup* getPerLuaCodeSetup(const FilterConfig* filter_config,
class Filter : public Http::StreamFilter, Logger::Loggable<Logger::Id::lua> {
public:
Filter(FilterConfigConstSharedPtr config, TimeSource& time_source)
: config_(config), time_source_(time_source) {}
: config_(config), time_source_(time_source), stats_(config->stats()) {}

Upstream::ClusterManager& clusterManager() { return config_->cluster_manager_; }
void scriptError(const Filters::Common::Lua::LuaException& e);
Expand Down Expand Up @@ -561,6 +583,7 @@ class Filter : public Http::StreamFilter, Logger::Loggable<Logger::Id::lua> {
StreamHandleRef response_stream_wrapper_;
bool destroyed_{};
TimeSource& time_source_;
LuaFilterStats stats_;

// These coroutines used to be owned by the stream handles. After investigating #3570, it
// became clear that there is a circular memory reference when a coroutine yields. Basically,
Expand Down

0 comments on commit 8c88943

Please sign in to comment.