fix!: change the instrumentation config to circumvent viper bug (#1143)

## Description

There's an viper property or potential bug that stops us from overriding
the default values for slices of strings. The "solution" atm is to use a
string and parse it later like animals later. Unfortunately I think this
is breaking since we're changing what a config is doing so tbh I'm not
sure if we can ever include this in v1. If that's the case then we might
want to consider not merging this to v0.34.x-celestia and only to main.

closes #1141

---------

Co-authored-by: Rootul P <rootulp@gmail.com>

config/config.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"strings"
 	"time"
 )
 
@@ -66,7 +67,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 = []string{}
+	DefaultInfluxTables = ""
 )
 
 // Config defines the top level configuration for a CometBFT node
@@ -1198,8 +1199,9 @@ 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.
-	InfluxTables []string `mapstructure:"influx_tables"`
+	// 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"`
 
 	// PyroscopeURL is the pyroscope url used to establish a connection with a
 	// pyroscope continuous profiling server.
@@ -1211,8 +1213,9 @@ 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.
-	PyroscopeProfileTypes []string `mapstructure:"pyroscope_profile_types"`
+	// block_count, block_duration. It is represented as a comma separate
+	// string. For example: "goroutines,alloc_objects".
+	PyroscopeProfileTypes string `mapstructure:"pyroscope_profile_types"`
 }
 
 // DefaultInstrumentationConfig returns a default configuration for metrics
@@ -1230,7 +1233,7 @@ func DefaultInstrumentationConfig() *InstrumentationConfig {
 		InfluxTables:         DefaultInfluxTables,
 		PyroscopeURL:         "",
 		PyroscopeTrace:       false,
-		PyroscopeProfileTypes: []string{
+		PyroscopeProfileTypes: strings.Join([]string{
 			"cpu",
 			"alloc_objects",
 			"inuse_objects",
@@ -1240,6 +1243,7 @@ func DefaultInstrumentationConfig() *InstrumentationConfig {
 			"block_count",
 			"block_duration",
 		},
+			","),
 	}
 }
 

config/toml.go

@@ -558,8 +558,9 @@ 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.
-influx_tables = [{{ range .Instrumentation.InfluxTables }}{{ printf "%q, " . }}{{end}}]
+# 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 }}"
 
 # The URL of the pyroscope instance to use for continuous profiling.
 # If empty, continuous profiling is disabled.
@@ -572,8 +573,9 @@ 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.
-pyroscope_profile_types = [{{ range .Instrumentation.PyroscopeProfileTypes }}{{ printf "%q, " . }}{{end}}]
+# block_count, block_duration. It is represented as a comma separate
+# string. For example: "goroutines,alloc_objects".
+pyroscope_profile_types = "{{ .Instrumentation.PyroscopeProfileTypes }}"
 
 `
 

node/tracing.go

@@ -76,8 +76,9 @@ func tracerProviderDebug() (*sdktrace.TracerProvider, error) {
 	return sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(sdktrace.NewBatchSpanProcessor(exp))), nil
 }
 
-func toPyroscopeProfiles(profiles []string) []pyroscope.ProfileType {
-	pts := make([]pyroscope.ProfileType, 0, len(profiles))
+func toPyroscopeProfiles(profiles string) []pyroscope.ProfileType {
+	parsedProfiles := splitAndTrimEmpty(profiles, ",", " ")
+	pts := make([]pyroscope.ProfileType, 0, len(parsedProfiles))
 	for _, p := range profiles {
 		pts = append(pts, pyroscope.ProfileType(p))
 	}

pkg/trace/client.go

@@ -3,6 +3,7 @@ package trace
 import (
 	"context"
 	"fmt"
+	"strings"
 	"time"
 
 	influxdb2 "github.com/influxdata/influxdb-client-go/v2"
@@ -81,7 +82,7 @@ func NewClient(cfg *config.InstrumentationConfig, logger log.Logger, chainID, no
 		cancel:  cancel,
 		chainID: chainID,
 		nodeID:  nodeID,
-		tables:  sliceToMap(cfg.InfluxTables),
+		tables:  stringToMap(cfg.InfluxTables),
 	}
 	if cfg.InfluxURL == "" {
 		return cli, nil
@@ -146,10 +147,35 @@ func (c *Client) WritePoint(table string, fields map[string]interface{}) {
 	writeAPI.WritePoint(p)
 }
 
-func sliceToMap(tables []string) map[string]struct{} {
+func stringToMap(tables string) map[string]struct{} {
+	parsedTables := splitAndTrimEmpty(tables, ",", " ")
 	m := make(map[string]struct{})
-	for _, s := range tables {
+	for _, s := range parsedTables {
 		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
+}

pkg/trace/schema/tables.go

@@ -1,9 +1,13 @@
 package schema
 
-import "github.com/tendermint/tendermint/config"
+import (
+	"strings"
+
+	"github.com/tendermint/tendermint/config"
+)
 
 func init() {
-	config.DefaultInfluxTables = AllTables()
+	config.DefaultInfluxTables = strings.Join(AllTables(), ",")
 }
 
 func AllTables() []string {

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

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