From 8019e0c2c7b4a142ced0537467663b844030ab65 Mon Sep 17 00:00:00 2001 From: Jianbo Sun Date: Wed, 18 Dec 2024 10:52:06 +0800 Subject: [PATCH 01/10] support expose metrics for plugins --- api/pkg/filtermanager/config.go | 6 ++++++ api/pkg/plugins/plugins.go | 21 +++++++++++++++++++++ api/pkg/plugins/plugins_test.go | 6 ++++++ api/tests/integration/test_plugins.go | 18 +++++++++++++++++- 4 files changed, 50 insertions(+), 1 deletion(-) diff --git a/api/pkg/filtermanager/config.go b/api/pkg/filtermanager/config.go index 6cd411fe..594f54b8 100644 --- a/api/pkg/filtermanager/config.go +++ b/api/pkg/filtermanager/config.go @@ -216,6 +216,12 @@ func (p *FilterManagerConfigParser) Parse(any *anypb.Any, callbacks capi.ConfigC for _, proto := range plugins { name := proto.Name + + if registerMetrics := pkgPlugins.LoadMetricsCallback(name); registerMetrics != nil { + registerMetrics(callbacks) + api.LogInfof("loaded metrics definition for plugin %s", name) + } + if plugin := pkgPlugins.LoadHTTPFilterFactoryAndParser(name); plugin != nil { config, err := plugin.ConfigParser.Parse(proto.Config) if err != nil { diff --git a/api/pkg/plugins/plugins.go b/api/pkg/plugins/plugins.go index 5ab2742a..c69f40f5 100644 --- a/api/pkg/plugins/plugins.go +++ b/api/pkg/plugins/plugins.go @@ -20,6 +20,8 @@ import ( "errors" "runtime/debug" + capi "github.com/envoyproxy/envoy/contrib/golang/common/go/api" + "mosn.io/htnn/api/internal/proto" "mosn.io/htnn/api/pkg/filtermanager/api" "mosn.io/htnn/api/pkg/log" @@ -31,6 +33,7 @@ var ( pluginTypes = map[string]Plugin{} plugins = map[string]Plugin{} httpFilterFactoryAndParser = map[string]*FilterFactoryAndParser{} + metricsRegister = map[string]func(capi.ConfigCallbacks){} ) // Here we introduce extra struct to avoid cyclic import between pkg/filtermanager and pkg/plugins @@ -188,6 +191,24 @@ func (cp *PluginConfigParser) Parse(any interface{}) (res interface{}, err error return conf, nil } +func RegisterMetricsCallback(pluginName string, registerMetricFunc func(capi.ConfigCallbacks)) { + if registerMetricFunc == nil { + panic("registerMetricFunc should not be nil") + } + if pluginName == "" { + panic("pluginName should not be empty") + } + if _, ok := metricsRegister[pluginName]; ok { + logger.Error(errors.New("metrics for plugin already registered, overriding"), "name", pluginName) + } + metricsRegister[pluginName] = registerMetricFunc + logger.Info("registered metrics for plugin", "name", pluginName) +} + +func LoadMetricsCallback(pluginName string) func(capi.ConfigCallbacks) { + return metricsRegister[pluginName] +} + // PluginMethodDefaultImpl provides reasonable implementation for optional methods type PluginMethodDefaultImpl struct{} diff --git a/api/pkg/plugins/plugins_test.go b/api/pkg/plugins/plugins_test.go index 103e6572..235d6868 100644 --- a/api/pkg/plugins/plugins_test.go +++ b/api/pkg/plugins/plugins_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/agiledragon/gomonkey/v2" + capi "github.com/envoyproxy/envoy/contrib/golang/common/go/api" "github.com/stretchr/testify/assert" "google.golang.org/protobuf/types/known/anypb" @@ -285,3 +286,8 @@ func TestRegisterPluginWithType(t *testing.T) { assert.NotNil(t, LoadPlugin("mock")) assert.NotNil(t, LoadPluginType("mock")) } + +func TestRegisterPluginMetrics(t *testing.T) { + RegisterMetricsCallback("mock", func(cc capi.ConfigCallbacks) {}) + assert.NotNil(t, LoadMetricsCallback("mock")) +} diff --git a/api/tests/integration/test_plugins.go b/api/tests/integration/test_plugins.go index dac15c4c..6e5239a1 100644 --- a/api/tests/integration/test_plugins.go +++ b/api/tests/integration/test_plugins.go @@ -21,6 +21,8 @@ import ( "strconv" "strings" + capi "github.com/envoyproxy/envoy/contrib/golang/common/go/api" + "mosn.io/htnn/api/pkg/filtermanager/api" "mosn.io/htnn/api/pkg/plugins" ) @@ -200,6 +202,7 @@ func (p *bufferPlugin) Factory() api.FilterFactory { type localReplyPlugin struct { plugins.PluginMethodDefaultImpl basePlugin + usageCounter capi.CounterMetric } func localReplyFactory(c interface{}, callbacks api.FilterCallbackHandler) api.Filter { @@ -242,6 +245,9 @@ func (f *localReplyFilter) DecodeRequest(headers api.RequestHeaderMap, buf api.B f.reqHdr = headers f.runFilters = headers.Values("run") if f.config.Decode { + if lrp.usageCounter != nil { + lrp.usageCounter.Increment(1) + } return f.NewLocalResponse("reply", true) } return api.Continue @@ -309,6 +315,11 @@ func (p *localReplyPlugin) Factory() api.FilterFactory { return localReplyFactory } +func (p *localReplyPlugin) MetricsDefinition(c capi.ConfigCallbacks) { + p.usageCounter = c.DefineCounterMetric("localreply.usage.counter") + // Define more metrics here +} + type badPlugin struct { plugins.PluginMethodDefaultImpl } @@ -619,10 +630,15 @@ func (f *onLogFilter) OnLog(reqHeaders api.RequestHeaderMap, reqTrailers api.Req api.LogWarnf("receive request trailers: %+v", trailers) } +var lrp = &localReplyPlugin{} + func init() { plugins.RegisterPlugin("stream", &streamPlugin{}) plugins.RegisterPlugin("buffer", &bufferPlugin{}) - plugins.RegisterPlugin("localReply", &localReplyPlugin{}) + + plugins.RegisterPlugin("localReply", lrp) + plugins.RegisterMetricsCallback("localReply", lrp.MetricsDefinition) + plugins.RegisterPlugin("bad", &badPlugin{}) plugins.RegisterPlugin("consumer", &consumerPlugin{}) plugins.RegisterPlugin("init", &initPlugin{}) From fdbcdffa3b068a516308d19de1f7888112eedf84 Mon Sep 17 00:00:00 2001 From: Jianbo Sun Date: Thu, 19 Dec 2024 19:37:26 +0800 Subject: [PATCH 02/10] address comments and add integration test --- api/pkg/filtermanager/config.go | 13 ++-- api/pkg/plugins/plugins.go | 18 ----- api/pkg/plugins/plugins_test.go | 6 -- api/pkg/plugins/type.go | 6 ++ .../tests/integration/dataplane/data_plane.go | 21 ++++++ .../integration/filtermanager_latest_test.go | 41 ++++++++++ api/tests/integration/test_plugins.go | 75 +++++++++++++++---- 7 files changed, 137 insertions(+), 43 deletions(-) diff --git a/api/pkg/filtermanager/config.go b/api/pkg/filtermanager/config.go index 594f54b8..c6a15e39 100644 --- a/api/pkg/filtermanager/config.go +++ b/api/pkg/filtermanager/config.go @@ -177,6 +177,10 @@ func (conf *filterManagerConfig) InitOnce() { } func (p *FilterManagerConfigParser) Parse(any *anypb.Any, callbacks capi.ConfigCallbackHandler) (interface{}, error) { + if callbacks == nil { + api.LogErrorf("no config callback handler provided") + // the call back handler to be nil only affects plugin metrics, so we can continue + } configStruct := &xds.TypedStruct{} // No configuration @@ -217,11 +221,6 @@ func (p *FilterManagerConfigParser) Parse(any *anypb.Any, callbacks capi.ConfigC for _, proto := range plugins { name := proto.Name - if registerMetrics := pkgPlugins.LoadMetricsCallback(name); registerMetrics != nil { - registerMetrics(callbacks) - api.LogInfof("loaded metrics definition for plugin %s", name) - } - if plugin := pkgPlugins.LoadHTTPFilterFactoryAndParser(name); plugin != nil { config, err := plugin.ConfigParser.Parse(proto.Config) if err != nil { @@ -252,6 +251,10 @@ func (p *FilterManagerConfigParser) Parse(any *anypb.Any, callbacks capi.ConfigC if _, ok := config.(pkgPlugins.Initer); ok { needInit = true } + if register, ok := config.(pkgPlugins.MetricsRegister); ok { + register.MetricsDefinition(callbacks) + api.LogInfof("loaded metrics definition for plugin: %s", name) + } if name == "debugMode" { // we handle this plugin differently, so we can have debug behavior before diff --git a/api/pkg/plugins/plugins.go b/api/pkg/plugins/plugins.go index c69f40f5..7cae28a7 100644 --- a/api/pkg/plugins/plugins.go +++ b/api/pkg/plugins/plugins.go @@ -191,24 +191,6 @@ func (cp *PluginConfigParser) Parse(any interface{}) (res interface{}, err error return conf, nil } -func RegisterMetricsCallback(pluginName string, registerMetricFunc func(capi.ConfigCallbacks)) { - if registerMetricFunc == nil { - panic("registerMetricFunc should not be nil") - } - if pluginName == "" { - panic("pluginName should not be empty") - } - if _, ok := metricsRegister[pluginName]; ok { - logger.Error(errors.New("metrics for plugin already registered, overriding"), "name", pluginName) - } - metricsRegister[pluginName] = registerMetricFunc - logger.Info("registered metrics for plugin", "name", pluginName) -} - -func LoadMetricsCallback(pluginName string) func(capi.ConfigCallbacks) { - return metricsRegister[pluginName] -} - // PluginMethodDefaultImpl provides reasonable implementation for optional methods type PluginMethodDefaultImpl struct{} diff --git a/api/pkg/plugins/plugins_test.go b/api/pkg/plugins/plugins_test.go index 235d6868..103e6572 100644 --- a/api/pkg/plugins/plugins_test.go +++ b/api/pkg/plugins/plugins_test.go @@ -20,7 +20,6 @@ import ( "testing" "github.com/agiledragon/gomonkey/v2" - capi "github.com/envoyproxy/envoy/contrib/golang/common/go/api" "github.com/stretchr/testify/assert" "google.golang.org/protobuf/types/known/anypb" @@ -286,8 +285,3 @@ func TestRegisterPluginWithType(t *testing.T) { assert.NotNil(t, LoadPlugin("mock")) assert.NotNil(t, LoadPluginType("mock")) } - -func TestRegisterPluginMetrics(t *testing.T) { - RegisterMetricsCallback("mock", func(cc capi.ConfigCallbacks) {}) - assert.NotNil(t, LoadMetricsCallback("mock")) -} diff --git a/api/pkg/plugins/type.go b/api/pkg/plugins/type.go index 1c258586..5642da87 100644 --- a/api/pkg/plugins/type.go +++ b/api/pkg/plugins/type.go @@ -15,6 +15,8 @@ package plugins import ( + capi "github.com/envoyproxy/envoy/contrib/golang/common/go/api" + "mosn.io/htnn/api/pkg/filtermanager/api" ) @@ -149,6 +151,10 @@ type Initer interface { Init(cb api.ConfigCallbackHandler) error } +type MetricsRegister interface { + MetricsDefinition(capi.ConfigCallbacks) +} + type NativePlugin interface { Plugin diff --git a/api/plugins/tests/integration/dataplane/data_plane.go b/api/plugins/tests/integration/dataplane/data_plane.go index 4e29b0e0..ef178477 100644 --- a/api/plugins/tests/integration/dataplane/data_plane.go +++ b/api/plugins/tests/integration/dataplane/data_plane.go @@ -543,6 +543,27 @@ func (dp *DataPlane) do(method string, path string, header http.Header, body io. return resp, err } +func (dp *DataPlane) GetAdmin(path string) (*http.Response, error) { + req, err := http.NewRequest("GET", "http://localhost:"+dp.adminAPIPort+path, nil) + if err != nil { + return nil, err + } + tr := &http.Transport{ + DialContext: func(ctx context.Context, proto, addr string) (conn net.Conn, err error) { + return net.DialTimeout("tcp", ":"+dp.adminAPIPort, 1*time.Second) + }, + } + + client := &http.Client{Transport: tr, + Timeout: 10 * time.Second, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, + } + resp, err := client.Do(req) + return resp, err +} + func (dp *DataPlane) doWithTrailer(method string, path string, header http.Header, body io.Reader, trailer http.Header) (*http.Response, error) { req, err := http.NewRequest(method, "http://localhost:"+dp.dataPlanePort+path, body) if err != nil { diff --git a/api/tests/integration/filtermanager_latest_test.go b/api/tests/integration/filtermanager_latest_test.go index 308433e5..74ab4dc8 100644 --- a/api/tests/integration/filtermanager_latest_test.go +++ b/api/tests/integration/filtermanager_latest_test.go @@ -19,6 +19,7 @@ package integration import ( "bytes" _ "embed" + "io" "net/http" "os" "path/filepath" @@ -339,3 +340,43 @@ func TestFilterManagerLogWithTrailers(t *testing.T) { require.Nil(t, err) assert.Equal(t, 200, resp.StatusCode) } + +func TestMetricsEnabledPlugin(t *testing.T) { + + dp, err := dataplane.StartDataPlane(t, &dataplane.Option{ + LogLevel: "debug", + Bootstrap: dataplane.Bootstrap(), + }) + if err != nil { + t.Fatalf("failed to start data plane: %v", err) + return + } + defer dp.Stop() + + lp := &filtermanager.FilterManagerConfig{ + Plugins: []*model.FilterConfig{ + { + Name: "metrics", + Config: &Config{}, + }, + }, + } + + controlPlane.UseGoPluginConfig(t, lp, dp) + hdr := http.Header{} + trailer := http.Header{} + trailer.Add("Expires", "Wed, 21 Oct 2015 07:28:00 GMT") + resp, err := dp.Get("/", hdr) + require.Nil(t, err) + body, err := io.ReadAll(resp.Body) + require.Nil(t, err) + assert.Equal(t, 200, resp.StatusCode, "response: %s", string(body)) + resp.Body.Close() + + resp, err = dp.GetAdmin("/stats") + require.Nil(t, err) + body, err = io.ReadAll(resp.Body) + require.Nil(t, err) + assert.Contains(t, string(body), "metrics-test.usage.counter 1") + assert.Contains(t, string(body), "metrics-test.usage.gauge 2") +} diff --git a/api/tests/integration/test_plugins.go b/api/tests/integration/test_plugins.go index 6e5239a1..6d5ec988 100644 --- a/api/tests/integration/test_plugins.go +++ b/api/tests/integration/test_plugins.go @@ -202,7 +202,6 @@ func (p *bufferPlugin) Factory() api.FilterFactory { type localReplyPlugin struct { plugins.PluginMethodDefaultImpl basePlugin - usageCounter capi.CounterMetric } func localReplyFactory(c interface{}, callbacks api.FilterCallbackHandler) api.Filter { @@ -245,9 +244,6 @@ func (f *localReplyFilter) DecodeRequest(headers api.RequestHeaderMap, buf api.B f.reqHdr = headers f.runFilters = headers.Values("run") if f.config.Decode { - if lrp.usageCounter != nil { - lrp.usageCounter.Increment(1) - } return f.NewLocalResponse("reply", true) } return api.Continue @@ -315,11 +311,6 @@ func (p *localReplyPlugin) Factory() api.FilterFactory { return localReplyFactory } -func (p *localReplyPlugin) MetricsDefinition(c capi.ConfigCallbacks) { - p.usageCounter = c.DefineCounterMetric("localreply.usage.counter") - // Define more metrics here -} - type badPlugin struct { plugins.PluginMethodDefaultImpl } @@ -630,15 +621,70 @@ func (f *onLogFilter) OnLog(reqHeaders api.RequestHeaderMap, reqTrailers api.Req api.LogWarnf("receive request trailers: %+v", trailers) } -var lrp = &localReplyPlugin{} +type metricsConfig struct { + Config + + usageCounter capi.CounterMetric + gauge capi.GaugeMetric +} + +func (m *metricsConfig) MetricsDefinition(c capi.ConfigCallbacks) { + if c == nil { + api.LogErrorf("metrics config callback is nil") + return + } + m.usageCounter = c.DefineCounterMetric("metrics-test.usage.counter") + m.gauge = c.DefineGaugeMetric("metrics-test.usage.gauge") + api.LogInfo("metrics config loaded for metrics-test") + // Define more metrics here +} + +var _ plugins.MetricsRegister = &metricsConfig{} + +type metricsPlugin struct { + plugins.PluginMethodDefaultImpl +} + +func (p *metricsPlugin) Config() api.PluginConfig { + return &metricsConfig{} +} + +func (p *metricsPlugin) Factory() api.FilterFactory { + return metricsFactory +} + +func metricsFactory(c interface{}, callbacks api.FilterCallbackHandler) api.Filter { + return &metricsFilter{ + callbacks: callbacks, + config: c.(*metricsConfig), + } +} + +type metricsFilter struct { + api.PassThroughFilter + + callbacks api.FilterCallbackHandler + config *metricsConfig +} + +func (f *metricsFilter) DecodeHeaders(headers api.RequestHeaderMap, endStream bool) api.ResultAction { + if f.config.usageCounter != nil { + f.config.usageCounter.Increment(1) + } else { + return &api.LocalResponse{Code: 500, Msg: "metrics config counter is nil"} + } + if f.config.gauge != nil { + f.config.gauge.Record(2) + } else { + return &api.LocalResponse{Code: 500, Msg: "metrics config gauge is nil"} + } + return &api.LocalResponse{Code: 200, Msg: "metrics works"} +} func init() { plugins.RegisterPlugin("stream", &streamPlugin{}) plugins.RegisterPlugin("buffer", &bufferPlugin{}) - - plugins.RegisterPlugin("localReply", lrp) - plugins.RegisterMetricsCallback("localReply", lrp.MetricsDefinition) - + plugins.RegisterPlugin("localReply", &localReplyPlugin{}) plugins.RegisterPlugin("bad", &badPlugin{}) plugins.RegisterPlugin("consumer", &consumerPlugin{}) plugins.RegisterPlugin("init", &initPlugin{}) @@ -647,4 +693,5 @@ func init() { plugins.RegisterPlugin("beforeConsumerAndHasOtherMethod", &beforeConsumerAndHasOtherMethodPlugin{}) plugins.RegisterPlugin("beforeConsumerAndHasDecodeRequest", &beforeConsumerAndHasDecodeRequestPlugin{}) plugins.RegisterPlugin("onLog", &onLogPlugin{}) + plugins.RegisterPlugin("metrics", &metricsPlugin{}) } From 9dbe46c9d6c87ee3ec79c57beb477b3ee790d4e9 Mon Sep 17 00:00:00 2001 From: Jianbo Sun Date: Thu, 19 Dec 2024 22:52:27 +0800 Subject: [PATCH 03/10] add additional filter --- .../integration/controlplane/control_plane.go | 6 ++- .../tests/integration/dataplane/bootstrap.go | 43 +++++++++++++++++++ .../integration/filtermanager_latest_test.go | 15 +++++-- 3 files changed, 59 insertions(+), 5 deletions(-) diff --git a/api/plugins/tests/integration/controlplane/control_plane.go b/api/plugins/tests/integration/controlplane/control_plane.go index 1434e54b..60ba1aee 100644 --- a/api/plugins/tests/integration/controlplane/control_plane.go +++ b/api/plugins/tests/integration/controlplane/control_plane.go @@ -165,10 +165,14 @@ func (cp *ControlPlane) UseGoPluginConfig(t *testing.T, config *filtermanager.Fi }, } if config != nil { + pluginName := os.Getenv("plugin_name_for_test") + if pluginName == "" { + pluginName = "fm" + } testRoute.TypedPerFilterConfig = map[string]*any1.Any{ "htnn.filters.http.golang": proto.MessageToAny(&golang.ConfigsPerRoute{ PluginsConfig: map[string]*golang.RouterPlugin{ - "fm": { + pluginName: { Override: &golang.RouterPlugin_Config{ Config: proto.MessageToAny( FilterManagerConfigToTypedStruct(config)), diff --git a/api/plugins/tests/integration/dataplane/bootstrap.go b/api/plugins/tests/integration/dataplane/bootstrap.go index 9fe22e57..952bda19 100644 --- a/api/plugins/tests/integration/dataplane/bootstrap.go +++ b/api/plugins/tests/integration/dataplane/bootstrap.go @@ -17,6 +17,7 @@ package dataplane import ( _ "embed" "encoding/json" + "fmt" "math/rand" "os" "strconv" @@ -36,6 +37,8 @@ type bootstrap struct { accessLogFormat string clusters []map[string]interface{} + additionalFilterGolang map[string]map[string]interface{} + dp *DataPlane } @@ -44,6 +47,8 @@ func Bootstrap() *bootstrap { backendRoutes: []map[string]interface{}{}, consumers: map[string]map[string]interface{}{}, clusters: []map[string]interface{}{}, + + additionalFilterGolang: map[string]map[string]interface{}{}, } } @@ -80,6 +85,11 @@ func (b *bootstrap) AddConsumer(name string, c map[string]interface{}) *bootstra return b } +func (b *bootstrap) AddAdditionalFilterGolang(name string, c map[string]interface{}) *bootstrap { + b.additionalFilterGolang[name] = c + return b +} + func (b *bootstrap) SetFilterGolang(cfg map[string]interface{}) *bootstrap { b.httpFilterGolang = cfg return b @@ -144,6 +154,39 @@ func (b *bootstrap) buildConfiguration() (map[string]interface{}, error) { } } } + if b.additionalFilterGolang != nil { + fmt.Println("XXXXXXXXXXXX add additional filter") + var additionalFilters []interface{} + for name, cfg := range b.additionalFilterGolang { + var found = false + wrapper := map[string]interface{}{ + "@type": "type.googleapis.com/xds.type.v3.TypedStruct", + "value": cfg, + } + for _, hf := range httpFilters { + if hf.(map[string]interface{})["name"] == name { + hf.(map[string]interface{})["disabled"] = false + hf.(map[string]interface{})["typed_config"].(map[string]interface{})["plugin_config"] = wrapper + found = true + } + } + if !found { + additionalFilters = append(additionalFilters, map[string]interface{}{ + "name": name, + "disabled": false, + "typed_config": map[string]interface{}{ + "@type": "type.googleapis.com/envoy.extensions.filters.http.golang.v3alpha.Config", + "library_path": "/etc/libgolang.so", + "library_id": "fm", + "plugin_name": name, + "plugin_config": wrapper, + }, + }) + } + } + httpFilters = append(additionalFilters, httpFilters...) + hcm["http_filters"] = httpFilters + } if b.accessLogFormat != "" { accessLog := hcm["access_log"].([]interface{})[0].(map[string]interface{})["typed_config"].(map[string]interface{}) diff --git a/api/tests/integration/filtermanager_latest_test.go b/api/tests/integration/filtermanager_latest_test.go index 74ab4dc8..673563a9 100644 --- a/api/tests/integration/filtermanager_latest_test.go +++ b/api/tests/integration/filtermanager_latest_test.go @@ -344,8 +344,15 @@ func TestFilterManagerLogWithTrailers(t *testing.T) { func TestMetricsEnabledPlugin(t *testing.T) { dp, err := dataplane.StartDataPlane(t, &dataplane.Option{ - LogLevel: "debug", - Bootstrap: dataplane.Bootstrap(), + LogLevel: "debug", + Bootstrap: dataplane.Bootstrap().AddAdditionalFilterGolang("lds.metrics", map[string]interface{}{ + "plugins": []interface{}{ + map[string]interface{}{ + "name": "metrics", + "config": map[string]interface{}{}, + }, + }, + }), }) if err != nil { t.Fatalf("failed to start data plane: %v", err) @@ -362,10 +369,9 @@ func TestMetricsEnabledPlugin(t *testing.T) { }, } + t.Setenv("plugin_name_for_test", "lds.metrics") controlPlane.UseGoPluginConfig(t, lp, dp) hdr := http.Header{} - trailer := http.Header{} - trailer.Add("Expires", "Wed, 21 Oct 2015 07:28:00 GMT") resp, err := dp.Get("/", hdr) require.Nil(t, err) body, err := io.ReadAll(resp.Body) @@ -379,4 +385,5 @@ func TestMetricsEnabledPlugin(t *testing.T) { require.Nil(t, err) assert.Contains(t, string(body), "metrics-test.usage.counter 1") assert.Contains(t, string(body), "metrics-test.usage.gauge 2") + //time.Sleep(5 * time.Minute) } From 0ab9ff4e78c299f52e9833eb14ba708852dd7f32 Mon Sep 17 00:00:00 2001 From: Jianbo Sun Date: Fri, 20 Dec 2024 09:47:05 +0800 Subject: [PATCH 04/10] fix lint --- api/pkg/plugins/plugins.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/api/pkg/plugins/plugins.go b/api/pkg/plugins/plugins.go index 7cae28a7..5ab2742a 100644 --- a/api/pkg/plugins/plugins.go +++ b/api/pkg/plugins/plugins.go @@ -20,8 +20,6 @@ import ( "errors" "runtime/debug" - capi "github.com/envoyproxy/envoy/contrib/golang/common/go/api" - "mosn.io/htnn/api/internal/proto" "mosn.io/htnn/api/pkg/filtermanager/api" "mosn.io/htnn/api/pkg/log" @@ -33,7 +31,6 @@ var ( pluginTypes = map[string]Plugin{} plugins = map[string]Plugin{} httpFilterFactoryAndParser = map[string]*FilterFactoryAndParser{} - metricsRegister = map[string]func(capi.ConfigCallbacks){} ) // Here we introduce extra struct to avoid cyclic import between pkg/filtermanager and pkg/plugins From cd2ce7afbcc4b0fa001f4999bd6ef82a32a3ead6 Mon Sep 17 00:00:00 2001 From: Jianbo Sun Date: Fri, 20 Dec 2024 21:53:52 +0800 Subject: [PATCH 05/10] use separate metricsconfigparser for metrics --- api/pkg/filtermanager/metricsconfigparser.go | 97 +++++++++++++++++++ .../tests/integration/dataplane/bootstrap.go | 55 ++++------- .../integration/filtermanager_latest_test.go | 26 +++-- api/tests/integration/libgolang/main.go | 1 + 4 files changed, 139 insertions(+), 40 deletions(-) create mode 100644 api/pkg/filtermanager/metricsconfigparser.go diff --git a/api/pkg/filtermanager/metricsconfigparser.go b/api/pkg/filtermanager/metricsconfigparser.go new file mode 100644 index 00000000..821a05ed --- /dev/null +++ b/api/pkg/filtermanager/metricsconfigparser.go @@ -0,0 +1,97 @@ +// Copyright The HTNN Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package filtermanager + +import ( + "encoding/json" + "errors" + + xds "github.com/cncf/xds/go/xds/type/v3" + capi "github.com/envoyproxy/envoy/contrib/golang/common/go/api" + "google.golang.org/protobuf/types/known/anypb" + + "mosn.io/htnn/api/pkg/filtermanager/api" + "mosn.io/htnn/api/pkg/filtermanager/model" + pkgPlugins "mosn.io/htnn/api/pkg/plugins" +) + +type MetricsConfigParser struct { +} + +type MetricsConfig struct { + Plugins []*model.FilterConfig `json:"plugins"` +} + +// MetricsConfigParser is the parser to register metrics only, no real parsing +func (p *MetricsConfigParser) Parse(any *anypb.Any, callbacks capi.ConfigCallbackHandler) (interface{}, error) { + if callbacks == nil { + api.LogErrorf("no config callback handler provided") + // the call back handler to be nil only affects plugin metrics, so we can continue + } + configStruct := &xds.TypedStruct{} + + // No configuration + if any.GetTypeUrl() == "" { + conf := initFilterManagerConfig("") + return conf, nil + } + + if err := any.UnmarshalTo(configStruct); err != nil { + return nil, err + } + + if configStruct.Value == nil { + return nil, errors.New("bad TypedStruct format") + } + + data, err := configStruct.Value.MarshalJSON() + if err != nil { + return nil, err + } + + mConfig := &MetricsConfig{} + if err := json.Unmarshal(data, mConfig); err != nil { + return nil, err + } + + plugins := mConfig.Plugins + for _, proto := range plugins { + name := proto.Name + + plugin := pkgPlugins.LoadHTTPFilterFactoryAndParser(name) + if plugin == nil { + api.LogErrorf("plugin %s not found, ignored", name) + continue + } + config, err := plugin.ConfigParser.Parse(proto.Config) + if err != nil { + api.LogErrorf("%s during parsing plugin %s in metrics manager", err, name) + + continue + } + if register, ok := config.(pkgPlugins.MetricsRegister); ok { + register.MetricsDefinition(callbacks) + api.LogInfof("loaded metrics definition for plugin: %s", name) + } + + } + + return nil, nil +} + +// just to satisfy the interface, no real merge +func (p *MetricsConfigParser) Merge(parent interface{}, child interface{}) interface{} { + return parent +} diff --git a/api/plugins/tests/integration/dataplane/bootstrap.go b/api/plugins/tests/integration/dataplane/bootstrap.go index 952bda19..cdcf2f78 100644 --- a/api/plugins/tests/integration/dataplane/bootstrap.go +++ b/api/plugins/tests/integration/dataplane/bootstrap.go @@ -17,7 +17,6 @@ package dataplane import ( _ "embed" "encoding/json" - "fmt" "math/rand" "os" "strconv" @@ -37,7 +36,7 @@ type bootstrap struct { accessLogFormat string clusters []map[string]interface{} - additionalFilterGolang map[string]map[string]interface{} + filterGolangForMetrics map[string]interface{} dp *DataPlane } @@ -48,7 +47,7 @@ func Bootstrap() *bootstrap { consumers: map[string]map[string]interface{}{}, clusters: []map[string]interface{}{}, - additionalFilterGolang: map[string]map[string]interface{}{}, + filterGolangForMetrics: map[string]interface{}{}, } } @@ -85,8 +84,8 @@ func (b *bootstrap) AddConsumer(name string, c map[string]interface{}) *bootstra return b } -func (b *bootstrap) AddAdditionalFilterGolang(name string, c map[string]interface{}) *bootstrap { - b.additionalFilterGolang[name] = c +func (b *bootstrap) AddFilterForGoMetrics(c map[string]interface{}) *bootstrap { + b.filterGolangForMetrics = c return b } @@ -154,35 +153,23 @@ func (b *bootstrap) buildConfiguration() (map[string]interface{}, error) { } } } - if b.additionalFilterGolang != nil { - fmt.Println("XXXXXXXXXXXX add additional filter") - var additionalFilters []interface{} - for name, cfg := range b.additionalFilterGolang { - var found = false - wrapper := map[string]interface{}{ - "@type": "type.googleapis.com/xds.type.v3.TypedStruct", - "value": cfg, - } - for _, hf := range httpFilters { - if hf.(map[string]interface{})["name"] == name { - hf.(map[string]interface{})["disabled"] = false - hf.(map[string]interface{})["typed_config"].(map[string]interface{})["plugin_config"] = wrapper - found = true - } - } - if !found { - additionalFilters = append(additionalFilters, map[string]interface{}{ - "name": name, - "disabled": false, - "typed_config": map[string]interface{}{ - "@type": "type.googleapis.com/envoy.extensions.filters.http.golang.v3alpha.Config", - "library_path": "/etc/libgolang.so", - "library_id": "fm", - "plugin_name": name, - "plugin_config": wrapper, - }, - }) - } + if b.filterGolangForMetrics != nil { + wrapper := map[string]interface{}{ + "@type": "type.googleapis.com/xds.type.v3.TypedStruct", + "value": b.filterGolangForMetrics, + } + var additionalFilters []interface{} = []interface{}{ + map[string]interface{}{ + "name": "htnn.go.metrics", + "disabled": false, + "typed_config": map[string]interface{}{ + "@type": "type.googleapis.com/envoy.extensions.filters.http.golang.v3alpha.Config", + "library_path": "/etc/libgolang.so", + "library_id": "fm-metrics", + "plugin_name": "fm-metrics", + "plugin_config": wrapper, + }, + }, } httpFilters = append(additionalFilters, httpFilters...) hcm["http_filters"] = httpFilters diff --git a/api/tests/integration/filtermanager_latest_test.go b/api/tests/integration/filtermanager_latest_test.go index 673563a9..a4ac87b8 100644 --- a/api/tests/integration/filtermanager_latest_test.go +++ b/api/tests/integration/filtermanager_latest_test.go @@ -23,6 +23,7 @@ import ( "net/http" "os" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -342,13 +343,12 @@ func TestFilterManagerLogWithTrailers(t *testing.T) { } func TestMetricsEnabledPlugin(t *testing.T) { - dp, err := dataplane.StartDataPlane(t, &dataplane.Option{ LogLevel: "debug", - Bootstrap: dataplane.Bootstrap().AddAdditionalFilterGolang("lds.metrics", map[string]interface{}{ + Bootstrap: dataplane.Bootstrap().AddFilterForGoMetrics(map[string]interface{}{ "plugins": []interface{}{ map[string]interface{}{ - "name": "metrics", + "name": "onLog", "config": map[string]interface{}{}, }, }, @@ -369,7 +369,6 @@ func TestMetricsEnabledPlugin(t *testing.T) { }, } - t.Setenv("plugin_name_for_test", "lds.metrics") controlPlane.UseGoPluginConfig(t, lp, dp) hdr := http.Header{} resp, err := dp.Get("/", hdr) @@ -383,7 +382,22 @@ func TestMetricsEnabledPlugin(t *testing.T) { require.Nil(t, err) body, err = io.ReadAll(resp.Body) require.Nil(t, err) - assert.Contains(t, string(body), "metrics-test.usage.counter 1") - assert.Contains(t, string(body), "metrics-test.usage.gauge 2") + lines := strings.Split(string(body), "\n") + + var found int + for _, l := range lines { + if !strings.Contains(l, "metrics-test") { + continue + } + if strings.Contains(l, "usage.counter") { + found++ + assert.Equal(t, "metrics-test.usage.counter 1", string(body)) + } + if strings.Contains(l, "usage.gauge") { + found++ + assert.Contains(t, "metrics-test.usage.gauge 2", string(body)) + } + } + assert.Equal(t, 2, found, "expect to have metrics usage.counter and usage.gauge") //time.Sleep(5 * time.Minute) } diff --git a/api/tests/integration/libgolang/main.go b/api/tests/integration/libgolang/main.go index 0af71b5b..7efc6fd0 100644 --- a/api/tests/integration/libgolang/main.go +++ b/api/tests/integration/libgolang/main.go @@ -28,6 +28,7 @@ import ( func init() { http.RegisterHttpFilterFactoryAndConfigParser("fm", filtermanager.FilterManagerFactory, &filtermanager.FilterManagerConfigParser{}) + http.RegisterHttpFilterFactoryAndConfigParser("fm-metrics", filtermanager.FilterManagerFactory, &filtermanager.MetricsConfigParser{}) http.RegisterHttpFilterFactoryAndConfigParser("cm", consumer.ConsumerManagerFactory, &consumer.ConsumerManagerConfigParser{}) http.RegisterHttpFilterFactoryAndConfigParser("dc", dynamicconfig.DynamicConfigFactory, &dynamicconfig.DynamicConfigParser{}) } From c88f188a9fe9bf0c2576ba4ac4a9249465a2590b Mon Sep 17 00:00:00 2001 From: Jianbo Sun Date: Fri, 20 Dec 2024 22:09:43 +0800 Subject: [PATCH 06/10] separate factory for metrics --- api/pkg/filtermanager/metricsconfigparser.go | 14 +++++++++++++- api/tests/integration/libgolang/main.go | 2 +- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/api/pkg/filtermanager/metricsconfigparser.go b/api/pkg/filtermanager/metricsconfigparser.go index 821a05ed..db270f57 100644 --- a/api/pkg/filtermanager/metricsconfigparser.go +++ b/api/pkg/filtermanager/metricsconfigparser.go @@ -27,6 +27,18 @@ import ( pkgPlugins "mosn.io/htnn/api/pkg/plugins" ) +type metricsConfigFilter struct { + capi.PassThroughStreamFilter + + callbacks capi.FilterCallbackHandler +} + +func MetricsConfigFactory(_ interface{}, callbacks capi.FilterCallbackHandler) capi.StreamFilter { + return &metricsConfigFilter{ + callbacks: callbacks, + } +} + type MetricsConfigParser struct { } @@ -88,7 +100,7 @@ func (p *MetricsConfigParser) Parse(any *anypb.Any, callbacks capi.ConfigCallbac } - return nil, nil + return any, nil } // just to satisfy the interface, no real merge diff --git a/api/tests/integration/libgolang/main.go b/api/tests/integration/libgolang/main.go index 7efc6fd0..8972b072 100644 --- a/api/tests/integration/libgolang/main.go +++ b/api/tests/integration/libgolang/main.go @@ -28,7 +28,7 @@ import ( func init() { http.RegisterHttpFilterFactoryAndConfigParser("fm", filtermanager.FilterManagerFactory, &filtermanager.FilterManagerConfigParser{}) - http.RegisterHttpFilterFactoryAndConfigParser("fm-metrics", filtermanager.FilterManagerFactory, &filtermanager.MetricsConfigParser{}) + http.RegisterHttpFilterFactoryAndConfigParser("fm-metrics", filtermanager.MetricsConfigFactory, &filtermanager.MetricsConfigParser{}) http.RegisterHttpFilterFactoryAndConfigParser("cm", consumer.ConsumerManagerFactory, &consumer.ConsumerManagerConfigParser{}) http.RegisterHttpFilterFactoryAndConfigParser("dc", dynamicconfig.DynamicConfigFactory, &dynamicconfig.DynamicConfigParser{}) } From a02e9cc70f7fb4fd3928c5cceaf579e02cf7c17d Mon Sep 17 00:00:00 2001 From: Jianbo Sun Date: Fri, 20 Dec 2024 23:30:08 +0800 Subject: [PATCH 07/10] use global var to register metrics --- api/pkg/filtermanager/config.go | 8 --- api/pkg/filtermanager/metricsconfigparser.go | 59 ++----------------- api/pkg/plugins/plugins.go | 15 +++++ .../integration/filtermanager_latest_test.go | 6 +- api/tests/integration/test_plugins.go | 37 ++++-------- 5 files changed, 33 insertions(+), 92 deletions(-) diff --git a/api/pkg/filtermanager/config.go b/api/pkg/filtermanager/config.go index c6a15e39..a85a55a1 100644 --- a/api/pkg/filtermanager/config.go +++ b/api/pkg/filtermanager/config.go @@ -177,10 +177,6 @@ func (conf *filterManagerConfig) InitOnce() { } func (p *FilterManagerConfigParser) Parse(any *anypb.Any, callbacks capi.ConfigCallbackHandler) (interface{}, error) { - if callbacks == nil { - api.LogErrorf("no config callback handler provided") - // the call back handler to be nil only affects plugin metrics, so we can continue - } configStruct := &xds.TypedStruct{} // No configuration @@ -251,10 +247,6 @@ func (p *FilterManagerConfigParser) Parse(any *anypb.Any, callbacks capi.ConfigC if _, ok := config.(pkgPlugins.Initer); ok { needInit = true } - if register, ok := config.(pkgPlugins.MetricsRegister); ok { - register.MetricsDefinition(callbacks) - api.LogInfof("loaded metrics definition for plugin: %s", name) - } if name == "debugMode" { // we handle this plugin differently, so we can have debug behavior before diff --git a/api/pkg/filtermanager/metricsconfigparser.go b/api/pkg/filtermanager/metricsconfigparser.go index db270f57..9e645b76 100644 --- a/api/pkg/filtermanager/metricsconfigparser.go +++ b/api/pkg/filtermanager/metricsconfigparser.go @@ -15,15 +15,10 @@ package filtermanager import ( - "encoding/json" - "errors" - - xds "github.com/cncf/xds/go/xds/type/v3" capi "github.com/envoyproxy/envoy/contrib/golang/common/go/api" "google.golang.org/protobuf/types/known/anypb" "mosn.io/htnn/api/pkg/filtermanager/api" - "mosn.io/htnn/api/pkg/filtermanager/model" pkgPlugins "mosn.io/htnn/api/pkg/plugins" ) @@ -42,62 +37,16 @@ func MetricsConfigFactory(_ interface{}, callbacks capi.FilterCallbackHandler) c type MetricsConfigParser struct { } -type MetricsConfig struct { - Plugins []*model.FilterConfig `json:"plugins"` -} - // MetricsConfigParser is the parser to register metrics only, no real parsing func (p *MetricsConfigParser) Parse(any *anypb.Any, callbacks capi.ConfigCallbackHandler) (interface{}, error) { if callbacks == nil { api.LogErrorf("no config callback handler provided") // the call back handler to be nil only affects plugin metrics, so we can continue } - configStruct := &xds.TypedStruct{} - - // No configuration - if any.GetTypeUrl() == "" { - conf := initFilterManagerConfig("") - return conf, nil - } - - if err := any.UnmarshalTo(configStruct); err != nil { - return nil, err - } - - if configStruct.Value == nil { - return nil, errors.New("bad TypedStruct format") - } - - data, err := configStruct.Value.MarshalJSON() - if err != nil { - return nil, err - } - - mConfig := &MetricsConfig{} - if err := json.Unmarshal(data, mConfig); err != nil { - return nil, err - } - - plugins := mConfig.Plugins - for _, proto := range plugins { - name := proto.Name - - plugin := pkgPlugins.LoadHTTPFilterFactoryAndParser(name) - if plugin == nil { - api.LogErrorf("plugin %s not found, ignored", name) - continue - } - config, err := plugin.ConfigParser.Parse(proto.Config) - if err != nil { - api.LogErrorf("%s during parsing plugin %s in metrics manager", err, name) - - continue - } - if register, ok := config.(pkgPlugins.MetricsRegister); ok { - register.MetricsDefinition(callbacks) - api.LogInfof("loaded metrics definition for plugin: %s", name) - } - + counterMetrics := pkgPlugins.GetCounterMetricsForCallback() + for m := range counterMetrics { + counterMetrics[m] = callbacks.DefineCounterMetric(m) + api.LogInfof("initialized counter metrics for %s", m) } return any, nil diff --git a/api/pkg/plugins/plugins.go b/api/pkg/plugins/plugins.go index 5ab2742a..6b56a081 100644 --- a/api/pkg/plugins/plugins.go +++ b/api/pkg/plugins/plugins.go @@ -20,6 +20,8 @@ import ( "errors" "runtime/debug" + capi "github.com/envoyproxy/envoy/contrib/golang/common/go/api" + "mosn.io/htnn/api/internal/proto" "mosn.io/htnn/api/pkg/filtermanager/api" "mosn.io/htnn/api/pkg/log" @@ -31,6 +33,7 @@ var ( pluginTypes = map[string]Plugin{} plugins = map[string]Plugin{} httpFilterFactoryAndParser = map[string]*FilterFactoryAndParser{} + metricsCounters = map[string]capi.CounterMetric{} ) // Here we introduce extra struct to avoid cyclic import between pkg/filtermanager and pkg/plugins @@ -234,3 +237,15 @@ func ComparePluginOrderInt(a, b string) int { } return cmp.Compare(a, b) } + +func RegisterCounterMetrics(name string) { + metricsCounters[name] = nil +} + +func GetCounterMetricsForCallback() map[string]capi.CounterMetric { + return metricsCounters +} + +func LoadCounterMetric(name string) capi.CounterMetric { + return metricsCounters[name] +} diff --git a/api/tests/integration/filtermanager_latest_test.go b/api/tests/integration/filtermanager_latest_test.go index a4ac87b8..a4ff1a06 100644 --- a/api/tests/integration/filtermanager_latest_test.go +++ b/api/tests/integration/filtermanager_latest_test.go @@ -391,13 +391,13 @@ func TestMetricsEnabledPlugin(t *testing.T) { } if strings.Contains(l, "usage.counter") { found++ - assert.Equal(t, "metrics-test.usage.counter 1", string(body)) + assert.Equal(t, "metrics-test.usage.counter: 1", l) } if strings.Contains(l, "usage.gauge") { found++ - assert.Contains(t, "metrics-test.usage.gauge 2", string(body)) + assert.Contains(t, "metrics-test.usage.gauge: 2", l) } } - assert.Equal(t, 2, found, "expect to have metrics usage.counter and usage.gauge") + assert.Equal(t, 1, found, "expect to have metrics usage.counter and usage.gauge") //time.Sleep(5 * time.Minute) } diff --git a/api/tests/integration/test_plugins.go b/api/tests/integration/test_plugins.go index 6d5ec988..cea847c4 100644 --- a/api/tests/integration/test_plugins.go +++ b/api/tests/integration/test_plugins.go @@ -21,8 +21,6 @@ import ( "strconv" "strings" - capi "github.com/envoyproxy/envoy/contrib/golang/common/go/api" - "mosn.io/htnn/api/pkg/filtermanager/api" "mosn.io/htnn/api/pkg/plugins" ) @@ -623,24 +621,8 @@ func (f *onLogFilter) OnLog(reqHeaders api.RequestHeaderMap, reqTrailers api.Req type metricsConfig struct { Config - - usageCounter capi.CounterMetric - gauge capi.GaugeMetric -} - -func (m *metricsConfig) MetricsDefinition(c capi.ConfigCallbacks) { - if c == nil { - api.LogErrorf("metrics config callback is nil") - return - } - m.usageCounter = c.DefineCounterMetric("metrics-test.usage.counter") - m.gauge = c.DefineGaugeMetric("metrics-test.usage.gauge") - api.LogInfo("metrics config loaded for metrics-test") - // Define more metrics here } -var _ plugins.MetricsRegister = &metricsConfig{} - type metricsPlugin struct { plugins.PluginMethodDefaultImpl } @@ -668,19 +650,17 @@ type metricsFilter struct { } func (f *metricsFilter) DecodeHeaders(headers api.RequestHeaderMap, endStream bool) api.ResultAction { - if f.config.usageCounter != nil { - f.config.usageCounter.Increment(1) + usageCounter := plugins.LoadCounterMetric("metrics-test.usage.counter") + if usageCounter != nil { + usageCounter.Increment(1) } else { return &api.LocalResponse{Code: 500, Msg: "metrics config counter is nil"} } - if f.config.gauge != nil { - f.config.gauge.Record(2) - } else { - return &api.LocalResponse{Code: 500, Msg: "metrics config gauge is nil"} - } return &api.LocalResponse{Code: 200, Msg: "metrics works"} } +var mp = &metricsPlugin{} + func init() { plugins.RegisterPlugin("stream", &streamPlugin{}) plugins.RegisterPlugin("buffer", &bufferPlugin{}) @@ -693,5 +673,10 @@ func init() { plugins.RegisterPlugin("beforeConsumerAndHasOtherMethod", &beforeConsumerAndHasOtherMethodPlugin{}) plugins.RegisterPlugin("beforeConsumerAndHasDecodeRequest", &beforeConsumerAndHasDecodeRequestPlugin{}) plugins.RegisterPlugin("onLog", &onLogPlugin{}) - plugins.RegisterPlugin("metrics", &metricsPlugin{}) + // register plugin "metrics" for plugin execution + plugins.RegisterPlugin("metrics", mp) + // register metrics definition for plugin "metrics" + plugins.RegisterCounterMetrics("metrics-test.usage.counter") + // TODO(wonderflow): support gauge metric + // TODO(wonderflow): allow metrics to contains runtime information especially for listener name } From 5329df61d3d3f3d25fadca13ba882c0b3e78a53f Mon Sep 17 00:00:00 2001 From: Jianbo Sun Date: Mon, 23 Dec 2024 17:47:33 +0800 Subject: [PATCH 08/10] refactor to use filtermanager for metrics registration --- api/pkg/filtermanager/api/api.go | 4 ++ api/pkg/filtermanager/api_impl.go | 41 +++++++++++++++++++ api/pkg/filtermanager/config.go | 23 +++++++++++ api/pkg/filtermanager/filtermanager.go | 2 + api/pkg/plugins/plugins.go | 16 ++++++++ api/pkg/plugins/type.go | 6 --- .../tests/integration/dataplane/bootstrap.go | 4 +- .../integration/filtermanager_latest_test.go | 19 +++++---- api/tests/integration/libgolang/main.go | 2 +- api/tests/integration/test_plugins.go | 27 ++++++++++-- 10 files changed, 124 insertions(+), 20 deletions(-) diff --git a/api/pkg/filtermanager/api/api.go b/api/pkg/filtermanager/api/api.go index 8f310bf2..40c9d16d 100644 --- a/api/pkg/filtermanager/api/api.go +++ b/api/pkg/filtermanager/api/api.go @@ -229,6 +229,10 @@ type StreamFilterCallbacks interface { // PluginState returns the PluginState associated to this request. PluginState() PluginState + // Metrics API for plugins + GetCounterMetrics(pluginName, metricsName string) api.CounterMetric + GetGaugeMetrics(pluginName, metricsName string) api.GaugeMetric + // WithLogArg injectes `key: value` as the suffix of application log created by this // callback's Log* methods. The injected log arguments are only valid in the current request. // This method can be used to inject IDs or other context information into the logs. diff --git a/api/pkg/filtermanager/api_impl.go b/api/pkg/filtermanager/api_impl.go index 24078e62..5d2ed1d1 100644 --- a/api/pkg/filtermanager/api_impl.go +++ b/api/pkg/filtermanager/api_impl.go @@ -30,6 +30,7 @@ import ( "mosn.io/htnn/api/internal/cookie" "mosn.io/htnn/api/internal/pluginstate" "mosn.io/htnn/api/pkg/filtermanager/api" + "mosn.io/htnn/api/pkg/plugins" ) type filterManagerRequestHeaderMap struct { @@ -146,6 +147,7 @@ type filterManagerCallbackHandler struct { namespace string consumer api.Consumer pluginState api.PluginState + metrics map[string]plugins.MetricsWriter streamInfo *filterManagerStreamInfo @@ -164,6 +166,7 @@ func (cb *filterManagerCallbackHandler) Reset() { cb.streamInfo = nil cb.logArgNames = "" cb.logArgs = nil + cb.metrics = nil cb.cacheLock.Unlock() } @@ -207,6 +210,44 @@ func (cb *filterManagerCallbackHandler) PluginState() api.PluginState { return cb.pluginState } +func (cb *filterManagerCallbackHandler) GetCounterMetrics(pluginName, metricName string) capi.CounterMetric { + if cb.metrics == nil { + api.LogErrorf("metrics not exist or not initialized for plugin %s", pluginName) + return nil + } + api.LogInfo("[metrics] printing:") + for k, v := range cb.metrics { + api.LogInfof("[metrics] %s: %v", k, v) + } + writer, ok := cb.metrics[pluginName] + if !ok { + api.LogErrorf("metrics writer for plugin %s not found", pluginName) + return nil + } + if writer.Counters == nil || writer.Counters[metricName] == nil { + api.LogErrorf("counter metric %s not found in plugin %s", metricName, pluginName) + return nil + } + return writer.Counters[metricName] +} + +func (cb *filterManagerCallbackHandler) GetGaugeMetrics(pluginName, metricName string) capi.GaugeMetric { + if cb.metrics == nil { + api.LogErrorf("metrics not exist or not initialized for plugin %s", pluginName) + return nil + } + writer, ok := cb.metrics[pluginName] + if !ok { + api.LogErrorf("metrics writer for plugin %s not found", pluginName) + return nil + } + if writer.Gaugers == nil || writer.Gaugers[metricName] == nil { + api.LogErrorf("gauge metric %s not found in plugin %s", metricName, pluginName) + return nil + } + return writer.Gaugers[metricName] +} + func (cb *filterManagerCallbackHandler) WithLogArg(key string, value any) api.StreamFilterCallbacks { // As the log is embedded into the Envoy's log, it's not so necessary to use structural logging // here. So far the value is just an ID string, introduce complex processions like quoting is diff --git a/api/pkg/filtermanager/config.go b/api/pkg/filtermanager/config.go index a85a55a1..44a150a8 100644 --- a/api/pkg/filtermanager/config.go +++ b/api/pkg/filtermanager/config.go @@ -28,6 +28,7 @@ import ( "mosn.io/htnn/api/pkg/filtermanager/api" "mosn.io/htnn/api/pkg/filtermanager/model" + "mosn.io/htnn/api/pkg/plugins" pkgPlugins "mosn.io/htnn/api/pkg/plugins" ) @@ -57,6 +58,8 @@ type filterManagerConfig struct { parsed []*model.ParsedFilterConfig pool *sync.Pool + metricsWriters map[string]plugins.MetricsWriter + namespace string enableDebugMode bool @@ -92,6 +95,10 @@ func (conf *filterManagerConfig) Merge(another *filterManagerConfig) *filterMana ns = another.namespace } + // Pass LDS metrics writers to the merged config for golang filter to use at route level + capi.LogInfof("[metrics] merging http filter, filtermanager config: %+v", another.metricsWriters) + conf.metricsWriters = another.metricsWriters + // It's tough to do the data plane merge right. We don't use shallow copy, which may share // data structure accidentally. We don't use deep copy all the fields, which may copy unexpected computed data. // Let's copy fields manually. @@ -179,9 +186,23 @@ func (conf *filterManagerConfig) InitOnce() { func (p *FilterManagerConfigParser) Parse(any *anypb.Any, callbacks capi.ConfigCallbackHandler) (interface{}, error) { configStruct := &xds.TypedStruct{} + var metricsWriters = map[string]plugins.MetricsWriter{} + + if callbacks != nil { + // If callbacks is not nil, it means this filter is configured in the LDS level. + // We need to initialize the metrics for all golang plugins here. + registers := plugins.GetMetricsDefinitions() + for pluginName, register := range registers { + api.LogInfof("initializing metrics for plugin %s", pluginName) + metricsWriters[pluginName] = register(callbacks) + } + capi.LogInfof("[metrics] initialized http filter, filtermanager config: %+v", metricsWriters) + } + // No configuration if any.GetTypeUrl() == "" { conf := initFilterManagerConfig("") + conf.metricsWriters = metricsWriters return conf, nil } @@ -265,6 +286,7 @@ func (p *FilterManagerConfigParser) Parse(any *anypb.Any, callbacks capi.ConfigC if needInit { conf.initOnce = &sync.Once{} } + conf.metricsWriters = metricsWriters return conf, nil } @@ -280,6 +302,7 @@ func (p *FilterManagerConfigParser) Merge(parent interface{}, child interface{}) } if httpFilterCfg == nil || len(httpFilterCfg.parsed) == 0 { + routeCfg.metricsWriters = httpFilterCfg.metricsWriters return routeCfg } diff --git a/api/pkg/filtermanager/filtermanager.go b/api/pkg/filtermanager/filtermanager.go index 30c4271b..241be929 100644 --- a/api/pkg/filtermanager/filtermanager.go +++ b/api/pkg/filtermanager/filtermanager.go @@ -152,6 +152,8 @@ func FilterManagerFactory(c interface{}, cb capi.FilterCallbackHandler) (streamF } fm.callbacks.FilterCallbackHandler = cb + capi.LogInfof("[metrics] filter manager metrics Writers %v", conf.metricsWriters) + fm.callbacks.metrics = conf.metricsWriters canSkipMethods := fm.canSkipMethods canSyncRunMethods := fm.canSyncRunMethods diff --git a/api/pkg/plugins/plugins.go b/api/pkg/plugins/plugins.go index 6b56a081..b476d22e 100644 --- a/api/pkg/plugins/plugins.go +++ b/api/pkg/plugins/plugins.go @@ -34,6 +34,7 @@ var ( plugins = map[string]Plugin{} httpFilterFactoryAndParser = map[string]*FilterFactoryAndParser{} metricsCounters = map[string]capi.CounterMetric{} + metricsDefinitions = map[string]MetricsRegister{} ) // Here we introduce extra struct to avoid cyclic import between pkg/filtermanager and pkg/plugins @@ -238,6 +239,21 @@ func ComparePluginOrderInt(a, b string) int { return cmp.Compare(a, b) } +type MetricsWriter struct { + Counters map[string]capi.CounterMetric + Gaugers map[string]capi.GaugeMetric +} + +type MetricsRegister func(capi.ConfigCallbacks) MetricsWriter + +func RegisterMetricsDefinitions(pluginName string, definition MetricsRegister) { + metricsDefinitions[pluginName] = definition +} + +func GetMetricsDefinitions() map[string]MetricsRegister { + return metricsDefinitions +} + func RegisterCounterMetrics(name string) { metricsCounters[name] = nil } diff --git a/api/pkg/plugins/type.go b/api/pkg/plugins/type.go index 5642da87..1c258586 100644 --- a/api/pkg/plugins/type.go +++ b/api/pkg/plugins/type.go @@ -15,8 +15,6 @@ package plugins import ( - capi "github.com/envoyproxy/envoy/contrib/golang/common/go/api" - "mosn.io/htnn/api/pkg/filtermanager/api" ) @@ -151,10 +149,6 @@ type Initer interface { Init(cb api.ConfigCallbackHandler) error } -type MetricsRegister interface { - MetricsDefinition(capi.ConfigCallbacks) -} - type NativePlugin interface { Plugin diff --git a/api/plugins/tests/integration/dataplane/bootstrap.go b/api/plugins/tests/integration/dataplane/bootstrap.go index cdcf2f78..abe036ff 100644 --- a/api/plugins/tests/integration/dataplane/bootstrap.go +++ b/api/plugins/tests/integration/dataplane/bootstrap.go @@ -153,7 +153,7 @@ func (b *bootstrap) buildConfiguration() (map[string]interface{}, error) { } } } - if b.filterGolangForMetrics != nil { + if len(b.filterGolangForMetrics) > 0 { wrapper := map[string]interface{}{ "@type": "type.googleapis.com/xds.type.v3.TypedStruct", "value": b.filterGolangForMetrics, @@ -161,7 +161,7 @@ func (b *bootstrap) buildConfiguration() (map[string]interface{}, error) { var additionalFilters []interface{} = []interface{}{ map[string]interface{}{ "name": "htnn.go.metrics", - "disabled": false, + "disabled": true, "typed_config": map[string]interface{}{ "@type": "type.googleapis.com/envoy.extensions.filters.http.golang.v3alpha.Config", "library_path": "/etc/libgolang.so", diff --git a/api/tests/integration/filtermanager_latest_test.go b/api/tests/integration/filtermanager_latest_test.go index a4ff1a06..ad6b5afe 100644 --- a/api/tests/integration/filtermanager_latest_test.go +++ b/api/tests/integration/filtermanager_latest_test.go @@ -25,6 +25,7 @@ import ( "path/filepath" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -345,14 +346,16 @@ func TestFilterManagerLogWithTrailers(t *testing.T) { func TestMetricsEnabledPlugin(t *testing.T) { dp, err := dataplane.StartDataPlane(t, &dataplane.Option{ LogLevel: "debug", - Bootstrap: dataplane.Bootstrap().AddFilterForGoMetrics(map[string]interface{}{ - "plugins": []interface{}{ - map[string]interface{}{ - "name": "onLog", - "config": map[string]interface{}{}, + /* + Bootstrap: dataplane.Bootstrap().AddFilterForGoMetrics(map[string]interface{}{ + "plugins": []interface{}{ + map[string]interface{}{ + "name": "onLog", + "config": map[string]interface{}{}, + }, }, - }, - }), + }), + */ }) if err != nil { t.Fatalf("failed to start data plane: %v", err) @@ -399,5 +402,5 @@ func TestMetricsEnabledPlugin(t *testing.T) { } } assert.Equal(t, 1, found, "expect to have metrics usage.counter and usage.gauge") - //time.Sleep(5 * time.Minute) + time.Sleep(5 * time.Minute) } diff --git a/api/tests/integration/libgolang/main.go b/api/tests/integration/libgolang/main.go index 8972b072..4928147f 100644 --- a/api/tests/integration/libgolang/main.go +++ b/api/tests/integration/libgolang/main.go @@ -28,7 +28,7 @@ import ( func init() { http.RegisterHttpFilterFactoryAndConfigParser("fm", filtermanager.FilterManagerFactory, &filtermanager.FilterManagerConfigParser{}) - http.RegisterHttpFilterFactoryAndConfigParser("fm-metrics", filtermanager.MetricsConfigFactory, &filtermanager.MetricsConfigParser{}) + //http.RegisterHttpFilterFactoryAndConfigParser("fm-metrics", filtermanager.MetricsConfigFactory, &filtermanager.MetricsConfigParser{}) http.RegisterHttpFilterFactoryAndConfigParser("cm", consumer.ConsumerManagerFactory, &consumer.ConsumerManagerConfigParser{}) http.RegisterHttpFilterFactoryAndConfigParser("dc", dynamicconfig.DynamicConfigFactory, &dynamicconfig.DynamicConfigParser{}) } diff --git a/api/tests/integration/test_plugins.go b/api/tests/integration/test_plugins.go index cea847c4..a105afb6 100644 --- a/api/tests/integration/test_plugins.go +++ b/api/tests/integration/test_plugins.go @@ -21,6 +21,8 @@ import ( "strconv" "strings" + capi "github.com/envoyproxy/envoy/contrib/golang/common/go/api" + "mosn.io/htnn/api/pkg/filtermanager/api" "mosn.io/htnn/api/pkg/plugins" ) @@ -649,13 +651,33 @@ type metricsFilter struct { config *metricsConfig } +const metricsUsageCounter = "metrics-test.usage.counter" +const metricsGauge = "metrics-test.usage.guage" + +func RegisterMetrics(c capi.ConfigCallbacks) plugins.MetricsWriter { + writer := plugins.MetricsWriter{ + Counters: map[string]capi.CounterMetric{}, + Gaugers: map[string]capi.GaugeMetric{}, + } + writer.Counters[metricsUsageCounter] = c.DefineCounterMetric(metricsUsageCounter) + writer.Gaugers[metricsGauge] = c.DefineGaugeMetric(metricsGauge) + return writer +} + func (f *metricsFilter) DecodeHeaders(headers api.RequestHeaderMap, endStream bool) api.ResultAction { - usageCounter := plugins.LoadCounterMetric("metrics-test.usage.counter") + usageCounter := f.callbacks.GetCounterMetrics("metrics", metricsUsageCounter) if usageCounter != nil { usageCounter.Increment(1) } else { return &api.LocalResponse{Code: 500, Msg: "metrics config counter is nil"} } + + gauger := f.callbacks.GetGaugeMetrics("metrics", metricsGauge) + if gauger != nil { + gauger.Record(2) + } else { + return &api.LocalResponse{Code: 500, Msg: "metrics config gauge is nil"} + } return &api.LocalResponse{Code: 200, Msg: "metrics works"} } @@ -676,7 +698,6 @@ func init() { // register plugin "metrics" for plugin execution plugins.RegisterPlugin("metrics", mp) // register metrics definition for plugin "metrics" - plugins.RegisterCounterMetrics("metrics-test.usage.counter") - // TODO(wonderflow): support gauge metric + plugins.RegisterMetricsDefinitions("metrics", RegisterMetrics) // TODO(wonderflow): allow metrics to contains runtime information especially for listener name } From dd128836a093378388975cbf32937326f342f035 Mon Sep 17 00:00:00 2001 From: Jianbo Sun Date: Mon, 23 Dec 2024 17:51:15 +0800 Subject: [PATCH 09/10] remove unused code --- api/pkg/filtermanager/metricsconfigparser.go | 58 ------------------- api/pkg/plugins/plugins.go | 13 ----- .../tests/integration/dataplane/bootstrap.go | 30 ---------- .../integration/filtermanager_latest_test.go | 12 ---- api/tests/integration/libgolang/main.go | 1 - 5 files changed, 114 deletions(-) delete mode 100644 api/pkg/filtermanager/metricsconfigparser.go diff --git a/api/pkg/filtermanager/metricsconfigparser.go b/api/pkg/filtermanager/metricsconfigparser.go deleted file mode 100644 index 9e645b76..00000000 --- a/api/pkg/filtermanager/metricsconfigparser.go +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright The HTNN Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package filtermanager - -import ( - capi "github.com/envoyproxy/envoy/contrib/golang/common/go/api" - "google.golang.org/protobuf/types/known/anypb" - - "mosn.io/htnn/api/pkg/filtermanager/api" - pkgPlugins "mosn.io/htnn/api/pkg/plugins" -) - -type metricsConfigFilter struct { - capi.PassThroughStreamFilter - - callbacks capi.FilterCallbackHandler -} - -func MetricsConfigFactory(_ interface{}, callbacks capi.FilterCallbackHandler) capi.StreamFilter { - return &metricsConfigFilter{ - callbacks: callbacks, - } -} - -type MetricsConfigParser struct { -} - -// MetricsConfigParser is the parser to register metrics only, no real parsing -func (p *MetricsConfigParser) Parse(any *anypb.Any, callbacks capi.ConfigCallbackHandler) (interface{}, error) { - if callbacks == nil { - api.LogErrorf("no config callback handler provided") - // the call back handler to be nil only affects plugin metrics, so we can continue - } - counterMetrics := pkgPlugins.GetCounterMetricsForCallback() - for m := range counterMetrics { - counterMetrics[m] = callbacks.DefineCounterMetric(m) - api.LogInfof("initialized counter metrics for %s", m) - } - - return any, nil -} - -// just to satisfy the interface, no real merge -func (p *MetricsConfigParser) Merge(parent interface{}, child interface{}) interface{} { - return parent -} diff --git a/api/pkg/plugins/plugins.go b/api/pkg/plugins/plugins.go index b476d22e..538dc898 100644 --- a/api/pkg/plugins/plugins.go +++ b/api/pkg/plugins/plugins.go @@ -33,7 +33,6 @@ var ( pluginTypes = map[string]Plugin{} plugins = map[string]Plugin{} httpFilterFactoryAndParser = map[string]*FilterFactoryAndParser{} - metricsCounters = map[string]capi.CounterMetric{} metricsDefinitions = map[string]MetricsRegister{} ) @@ -253,15 +252,3 @@ func RegisterMetricsDefinitions(pluginName string, definition MetricsRegister) { func GetMetricsDefinitions() map[string]MetricsRegister { return metricsDefinitions } - -func RegisterCounterMetrics(name string) { - metricsCounters[name] = nil -} - -func GetCounterMetricsForCallback() map[string]capi.CounterMetric { - return metricsCounters -} - -func LoadCounterMetric(name string) capi.CounterMetric { - return metricsCounters[name] -} diff --git a/api/plugins/tests/integration/dataplane/bootstrap.go b/api/plugins/tests/integration/dataplane/bootstrap.go index abe036ff..9fe22e57 100644 --- a/api/plugins/tests/integration/dataplane/bootstrap.go +++ b/api/plugins/tests/integration/dataplane/bootstrap.go @@ -36,8 +36,6 @@ type bootstrap struct { accessLogFormat string clusters []map[string]interface{} - filterGolangForMetrics map[string]interface{} - dp *DataPlane } @@ -46,8 +44,6 @@ func Bootstrap() *bootstrap { backendRoutes: []map[string]interface{}{}, consumers: map[string]map[string]interface{}{}, clusters: []map[string]interface{}{}, - - filterGolangForMetrics: map[string]interface{}{}, } } @@ -84,11 +80,6 @@ func (b *bootstrap) AddConsumer(name string, c map[string]interface{}) *bootstra return b } -func (b *bootstrap) AddFilterForGoMetrics(c map[string]interface{}) *bootstrap { - b.filterGolangForMetrics = c - return b -} - func (b *bootstrap) SetFilterGolang(cfg map[string]interface{}) *bootstrap { b.httpFilterGolang = cfg return b @@ -153,27 +144,6 @@ func (b *bootstrap) buildConfiguration() (map[string]interface{}, error) { } } } - if len(b.filterGolangForMetrics) > 0 { - wrapper := map[string]interface{}{ - "@type": "type.googleapis.com/xds.type.v3.TypedStruct", - "value": b.filterGolangForMetrics, - } - var additionalFilters []interface{} = []interface{}{ - map[string]interface{}{ - "name": "htnn.go.metrics", - "disabled": true, - "typed_config": map[string]interface{}{ - "@type": "type.googleapis.com/envoy.extensions.filters.http.golang.v3alpha.Config", - "library_path": "/etc/libgolang.so", - "library_id": "fm-metrics", - "plugin_name": "fm-metrics", - "plugin_config": wrapper, - }, - }, - } - httpFilters = append(additionalFilters, httpFilters...) - hcm["http_filters"] = httpFilters - } if b.accessLogFormat != "" { accessLog := hcm["access_log"].([]interface{})[0].(map[string]interface{})["typed_config"].(map[string]interface{}) diff --git a/api/tests/integration/filtermanager_latest_test.go b/api/tests/integration/filtermanager_latest_test.go index ad6b5afe..a3c6ecba 100644 --- a/api/tests/integration/filtermanager_latest_test.go +++ b/api/tests/integration/filtermanager_latest_test.go @@ -25,7 +25,6 @@ import ( "path/filepath" "strings" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -346,16 +345,6 @@ func TestFilterManagerLogWithTrailers(t *testing.T) { func TestMetricsEnabledPlugin(t *testing.T) { dp, err := dataplane.StartDataPlane(t, &dataplane.Option{ LogLevel: "debug", - /* - Bootstrap: dataplane.Bootstrap().AddFilterForGoMetrics(map[string]interface{}{ - "plugins": []interface{}{ - map[string]interface{}{ - "name": "onLog", - "config": map[string]interface{}{}, - }, - }, - }), - */ }) if err != nil { t.Fatalf("failed to start data plane: %v", err) @@ -402,5 +391,4 @@ func TestMetricsEnabledPlugin(t *testing.T) { } } assert.Equal(t, 1, found, "expect to have metrics usage.counter and usage.gauge") - time.Sleep(5 * time.Minute) } diff --git a/api/tests/integration/libgolang/main.go b/api/tests/integration/libgolang/main.go index 4928147f..0af71b5b 100644 --- a/api/tests/integration/libgolang/main.go +++ b/api/tests/integration/libgolang/main.go @@ -28,7 +28,6 @@ import ( func init() { http.RegisterHttpFilterFactoryAndConfigParser("fm", filtermanager.FilterManagerFactory, &filtermanager.FilterManagerConfigParser{}) - //http.RegisterHttpFilterFactoryAndConfigParser("fm-metrics", filtermanager.MetricsConfigFactory, &filtermanager.MetricsConfigParser{}) http.RegisterHttpFilterFactoryAndConfigParser("cm", consumer.ConsumerManagerFactory, &consumer.ConsumerManagerConfigParser{}) http.RegisterHttpFilterFactoryAndConfigParser("dc", dynamicconfig.DynamicConfigFactory, &dynamicconfig.DynamicConfigParser{}) } From 96b4b3d6577a52c70dfb2f9732bbd5b740bbfbe7 Mon Sep 17 00:00:00 2001 From: Jianbo Sun Date: Tue, 24 Dec 2024 15:52:10 +0800 Subject: [PATCH 10/10] remove extra logs and fix test --- api/pkg/filtermanager/api_impl.go | 4 ---- api/pkg/filtermanager/config.go | 8 +++++--- api/pkg/filtermanager/filtermanager.go | 2 +- .../tests/integration/controlplane/control_plane.go | 6 +----- api/plugins/tests/pkg/envoy/capi.go | 7 +++++++ api/tests/integration/filtermanager_latest_test.go | 3 ++- api/tests/integration/test_plugins.go | 4 ++-- 7 files changed, 18 insertions(+), 16 deletions(-) diff --git a/api/pkg/filtermanager/api_impl.go b/api/pkg/filtermanager/api_impl.go index 5d2ed1d1..44ea830d 100644 --- a/api/pkg/filtermanager/api_impl.go +++ b/api/pkg/filtermanager/api_impl.go @@ -215,10 +215,6 @@ func (cb *filterManagerCallbackHandler) GetCounterMetrics(pluginName, metricName api.LogErrorf("metrics not exist or not initialized for plugin %s", pluginName) return nil } - api.LogInfo("[metrics] printing:") - for k, v := range cb.metrics { - api.LogInfof("[metrics] %s: %v", k, v) - } writer, ok := cb.metrics[pluginName] if !ok { api.LogErrorf("metrics writer for plugin %s not found", pluginName) diff --git a/api/pkg/filtermanager/config.go b/api/pkg/filtermanager/config.go index 44a150a8..5baed145 100644 --- a/api/pkg/filtermanager/config.go +++ b/api/pkg/filtermanager/config.go @@ -20,6 +20,7 @@ import ( "fmt" "reflect" "sort" + "strings" "sync" xds "github.com/cncf/xds/go/xds/type/v3" @@ -96,7 +97,6 @@ func (conf *filterManagerConfig) Merge(another *filterManagerConfig) *filterMana } // Pass LDS metrics writers to the merged config for golang filter to use at route level - capi.LogInfof("[metrics] merging http filter, filtermanager config: %+v", another.metricsWriters) conf.metricsWriters = another.metricsWriters // It's tough to do the data plane merge right. We don't use shallow copy, which may share @@ -192,11 +192,13 @@ func (p *FilterManagerConfigParser) Parse(any *anypb.Any, callbacks capi.ConfigC // If callbacks is not nil, it means this filter is configured in the LDS level. // We need to initialize the metrics for all golang plugins here. registers := plugins.GetMetricsDefinitions() + registeredPlugins := []string{} for pluginName, register := range registers { - api.LogInfof("initializing metrics for plugin %s", pluginName) + api.LogInfof("registering metrics for golang plugin %s", pluginName) metricsWriters[pluginName] = register(callbacks) + registeredPlugins = append(registeredPlugins, pluginName) } - capi.LogInfof("[metrics] initialized http filter, filtermanager config: %+v", metricsWriters) + capi.LogInfof("metrics registered for plugins: [%s]", strings.Join(registeredPlugins, ", ")) } // No configuration diff --git a/api/pkg/filtermanager/filtermanager.go b/api/pkg/filtermanager/filtermanager.go index 241be929..69c19540 100644 --- a/api/pkg/filtermanager/filtermanager.go +++ b/api/pkg/filtermanager/filtermanager.go @@ -152,7 +152,7 @@ func FilterManagerFactory(c interface{}, cb capi.FilterCallbackHandler) (streamF } fm.callbacks.FilterCallbackHandler = cb - capi.LogInfof("[metrics] filter manager metrics Writers %v", conf.metricsWriters) + fm.callbacks.metrics = conf.metricsWriters canSkipMethods := fm.canSkipMethods diff --git a/api/plugins/tests/integration/controlplane/control_plane.go b/api/plugins/tests/integration/controlplane/control_plane.go index 60ba1aee..1434e54b 100644 --- a/api/plugins/tests/integration/controlplane/control_plane.go +++ b/api/plugins/tests/integration/controlplane/control_plane.go @@ -165,14 +165,10 @@ func (cp *ControlPlane) UseGoPluginConfig(t *testing.T, config *filtermanager.Fi }, } if config != nil { - pluginName := os.Getenv("plugin_name_for_test") - if pluginName == "" { - pluginName = "fm" - } testRoute.TypedPerFilterConfig = map[string]*any1.Any{ "htnn.filters.http.golang": proto.MessageToAny(&golang.ConfigsPerRoute{ PluginsConfig: map[string]*golang.RouterPlugin{ - pluginName: { + "fm": { Override: &golang.RouterPlugin_Config{ Config: proto.MessageToAny( FilterManagerConfigToTypedStruct(config)), diff --git a/api/plugins/tests/pkg/envoy/capi.go b/api/plugins/tests/pkg/envoy/capi.go index e26d7328..ed10d146 100644 --- a/api/plugins/tests/pkg/envoy/capi.go +++ b/api/plugins/tests/pkg/envoy/capi.go @@ -583,6 +583,13 @@ func (i *filterCallbackHandler) PluginState() api.PluginState { return i.pluginState } +func (i *filterCallbackHandler) GetCounterMetrics(pluginName, metricsName string) capi.CounterMetric { + return nil +} +func (i *filterCallbackHandler) GetGaugeMetrics(pluginName, metricsName string) capi.GaugeMetric { + return nil +} + func (i *filterCallbackHandler) WithLogArg(key string, value any) api.StreamFilterCallbacks { return i } diff --git a/api/tests/integration/filtermanager_latest_test.go b/api/tests/integration/filtermanager_latest_test.go index a3c6ecba..033d2dd3 100644 --- a/api/tests/integration/filtermanager_latest_test.go +++ b/api/tests/integration/filtermanager_latest_test.go @@ -377,6 +377,7 @@ func TestMetricsEnabledPlugin(t *testing.T) { lines := strings.Split(string(body), "\n") var found int + for _, l := range lines { if !strings.Contains(l, "metrics-test") { continue @@ -390,5 +391,5 @@ func TestMetricsEnabledPlugin(t *testing.T) { assert.Contains(t, "metrics-test.usage.gauge: 2", l) } } - assert.Equal(t, 1, found, "expect to have metrics usage.counter and usage.gauge") + assert.Equal(t, 2, found, "expect to have metrics usage.counter and usage.gauge") } diff --git a/api/tests/integration/test_plugins.go b/api/tests/integration/test_plugins.go index a105afb6..9ac0b005 100644 --- a/api/tests/integration/test_plugins.go +++ b/api/tests/integration/test_plugins.go @@ -652,7 +652,7 @@ type metricsFilter struct { } const metricsUsageCounter = "metrics-test.usage.counter" -const metricsGauge = "metrics-test.usage.guage" +const metricsGauge = "metrics-test.usage.gauge" func RegisterMetrics(c capi.ConfigCallbacks) plugins.MetricsWriter { writer := plugins.MetricsWriter{ @@ -699,5 +699,5 @@ func init() { plugins.RegisterPlugin("metrics", mp) // register metrics definition for plugin "metrics" plugins.RegisterMetricsDefinitions("metrics", RegisterMetrics) - // TODO(wonderflow): allow metrics to contains runtime information especially for listener name + // TODO(wonderflow): allow metrics to contains runtime information especially for listener name, this require support from envoy upstream: https://github.com/envoyproxy/envoy/issues/37808 }