From 92efcbe94a792f80b97037b400e95705029ce58d Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Tue, 9 Jan 2024 18:21:38 -0600 Subject: [PATCH] =?UTF-8?q?Revert=20"fix!:=20change=20the=20instrumentatio?= =?UTF-8?q?n=20config=20to=20circumvent=20viper=20b=E2=80=A6=20(#1167)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …ug (#1143)" This reverts commit f1dc8f7da7a8fc1e2fd1409c4200e095ac9ae547. which is a breaking change to the config --- config/config.go | 16 ++++++---------- config/toml.go | 10 ++++------ node/tracing.go | 5 ++--- pkg/trace/client.go | 32 +++----------------------------- pkg/trace/schema/tables.go | 8 ++------ test/e2e/pkg/infrastructure.go | 2 +- test/e2e/pkg/testnet.go | 2 +- 7 files changed, 19 insertions(+), 56 deletions(-) diff --git a/config/config.go b/config/config.go index 5e5af9af41..15e7f35925 100644 --- a/config/config.go +++ b/config/config.go @@ -7,7 +7,6 @@ import ( "net/http" "os" "path/filepath" - "strings" "time" ) @@ -67,7 +66,7 @@ var ( // This global var is filled by an init function in the schema package. This // allows for the schema package to contain all the relevant logic while // avoiding import cycles. - DefaultInfluxTables = "" + DefaultInfluxTables = []string{} ) // Config defines the top level configuration for a CometBFT node @@ -1199,9 +1198,8 @@ type InstrumentationConfig struct { InfluxBatchSize int `mapstructure:"influx_batch_size"` // InfluxTables is the list of tables that will be traced. See the - // pkg/trace/schema for a complete list of tables. It is represented as a - // comma separate string. For example: "consensus_round_state,mempool_tx". - InfluxTables string `mapstructure:"influx_tables"` + // pkg/trace/schema for a complete list of tables. + InfluxTables []string `mapstructure:"influx_tables"` // PyroscopeURL is the pyroscope url used to establish a connection with a // pyroscope continuous profiling server. @@ -1213,9 +1211,8 @@ type InstrumentationConfig struct { // PyroscopeProfileTypes is a list of profile types to be traced with // pyroscope. Available profile types are: cpu, alloc_objects, alloc_space, // inuse_objects, inuse_space, goroutines, mutex_count, mutex_duration, - // block_count, block_duration. It is represented as a comma separate - // string. For example: "goroutines,alloc_objects". - PyroscopeProfileTypes string `mapstructure:"pyroscope_profile_types"` + // block_count, block_duration. + PyroscopeProfileTypes []string `mapstructure:"pyroscope_profile_types"` } // DefaultInstrumentationConfig returns a default configuration for metrics @@ -1233,7 +1230,7 @@ func DefaultInstrumentationConfig() *InstrumentationConfig { InfluxTables: DefaultInfluxTables, PyroscopeURL: "", PyroscopeTrace: false, - PyroscopeProfileTypes: strings.Join([]string{ + PyroscopeProfileTypes: []string{ "cpu", "alloc_objects", "inuse_objects", @@ -1243,7 +1240,6 @@ func DefaultInstrumentationConfig() *InstrumentationConfig { "block_count", "block_duration", }, - ","), } } diff --git a/config/toml.go b/config/toml.go index 7c9abcc7a8..3a7d8492fd 100644 --- a/config/toml.go +++ b/config/toml.go @@ -558,9 +558,8 @@ influx_org = "{{ .Instrumentation.InfluxOrg }}" influx_batch_size = {{ .Instrumentation.InfluxBatchSize }} # The list of tables that are updated when tracing. All available tables and -# their schema can be found in the pkg/trace/schema package. It is represented as a -# comma separate string. For example: "consensus_round_state,mempool_tx". -influx_tables = "{{ .Instrumentation.InfluxTables }}" +# their schema can be found in the pkg/trace/schema package. +influx_tables = [{{ range .Instrumentation.InfluxTables }}{{ printf "%q, " . }}{{end}}] # The URL of the pyroscope instance to use for continuous profiling. # If empty, continuous profiling is disabled. @@ -573,9 +572,8 @@ pyroscope_trace = {{ .Instrumentation.PyroscopeTrace }} # pyroscope_profile_types is a list of profile types to be traced with # pyroscope. Available profile types are: cpu, alloc_objects, alloc_space, # inuse_objects, inuse_space, goroutines, mutex_count, mutex_duration, -# block_count, block_duration. It is represented as a comma separate -# string. For example: "goroutines,alloc_objects". -pyroscope_profile_types = "{{ .Instrumentation.PyroscopeProfileTypes }}" +# block_count, block_duration. +pyroscope_profile_types = [{{ range .Instrumentation.PyroscopeProfileTypes }}{{ printf "%q, " . }}{{end}}] ` diff --git a/node/tracing.go b/node/tracing.go index 9332602733..4e2e00f76e 100644 --- a/node/tracing.go +++ b/node/tracing.go @@ -76,9 +76,8 @@ func tracerProviderDebug() (*sdktrace.TracerProvider, error) { return sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(sdktrace.NewBatchSpanProcessor(exp))), nil } -func toPyroscopeProfiles(profiles string) []pyroscope.ProfileType { - parsedProfiles := splitAndTrimEmpty(profiles, ",", " ") - pts := make([]pyroscope.ProfileType, 0, len(parsedProfiles)) +func toPyroscopeProfiles(profiles []string) []pyroscope.ProfileType { + pts := make([]pyroscope.ProfileType, 0, len(profiles)) for _, p := range profiles { pts = append(pts, pyroscope.ProfileType(p)) } diff --git a/pkg/trace/client.go b/pkg/trace/client.go index 74f93850fd..0ee70aebed 100644 --- a/pkg/trace/client.go +++ b/pkg/trace/client.go @@ -3,7 +3,6 @@ package trace import ( "context" "fmt" - "strings" "time" influxdb2 "github.com/influxdata/influxdb-client-go/v2" @@ -82,7 +81,7 @@ func NewClient(cfg *config.InstrumentationConfig, logger log.Logger, chainID, no cancel: cancel, chainID: chainID, nodeID: nodeID, - tables: stringToMap(cfg.InfluxTables), + tables: sliceToMap(cfg.InfluxTables), } if cfg.InfluxURL == "" { return cli, nil @@ -147,35 +146,10 @@ func (c *Client) WritePoint(table string, fields map[string]interface{}) { writeAPI.WritePoint(p) } -func stringToMap(tables string) map[string]struct{} { - parsedTables := splitAndTrimEmpty(tables, ",", " ") +func sliceToMap(tables []string) map[string]struct{} { m := make(map[string]struct{}) - for _, s := range parsedTables { + for _, s := range tables { m[s] = struct{}{} } return m } - -// splitAndTrimEmpty slices s into all subslices separated by sep and returns a -// slice of the string s with all leading and trailing Unicode code points -// contained in cutset removed. If sep is empty, SplitAndTrim splits after each -// UTF-8 sequence. First part is equivalent to strings.SplitN with a count of -// -1. also filter out empty strings, only return non-empty strings. -// -// NOTE: this is copy pasted from the config pacakage to avoid a circular -// dependency. See the function of the same name for tests. -func splitAndTrimEmpty(s, sep, cutset string) []string { - if s == "" { - return []string{} - } - - spl := strings.Split(s, sep) - nonEmptyStrings := make([]string, 0, len(spl)) - for i := 0; i < len(spl); i++ { - element := strings.Trim(spl[i], cutset) - if element != "" { - nonEmptyStrings = append(nonEmptyStrings, element) - } - } - return nonEmptyStrings -} diff --git a/pkg/trace/schema/tables.go b/pkg/trace/schema/tables.go index b07488ca9b..2c8c9ef97d 100644 --- a/pkg/trace/schema/tables.go +++ b/pkg/trace/schema/tables.go @@ -1,13 +1,9 @@ package schema -import ( - "strings" - - "github.com/tendermint/tendermint/config" -) +import "github.com/tendermint/tendermint/config" func init() { - config.DefaultInfluxTables = strings.Join(AllTables(), ",") + config.DefaultInfluxTables = AllTables() } func AllTables() []string { diff --git a/test/e2e/pkg/infrastructure.go b/test/e2e/pkg/infrastructure.go index fe4b7eb866..a1bec6fbf2 100644 --- a/test/e2e/pkg/infrastructure.go +++ b/test/e2e/pkg/infrastructure.go @@ -49,7 +49,7 @@ type InfrastructureData struct { PyroscopeTrace bool `json:"pyroscope_trace,omitempty"` // PyroscopeProfileTypes is the list of profile types to collect. - PyroscopeProfileTypes string `json:"pyroscope_profile_types,omitempty"` + PyroscopeProfileTypes []string `json:"pyroscope_profile_types,omitempty"` } // InstanceData contains the relevant information for a machine instance backing diff --git a/test/e2e/pkg/testnet.go b/test/e2e/pkg/testnet.go index c8d2859aef..2cb9a58f4e 100644 --- a/test/e2e/pkg/testnet.go +++ b/test/e2e/pkg/testnet.go @@ -110,7 +110,7 @@ type Node struct { InfluxDBToken string PyroscopeURL string PyroscopeTrace bool - PyroscopeProfileTypes string + PyroscopeProfileTypes []string } // LoadTestnet loads a testnet from a manifest file, using the filename to