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

## Description

There's an viper [property or potential
bug](spf13/viper#1366) 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.

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"
 
 	"github.com/cometbft/cometbft/p2p/conn"
@@ -68,7 +69,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
@@ -1201,8 +1202,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.
@@ -1214,8 +1216,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
@@ -1233,7 +1236,7 @@ func DefaultInstrumentationConfig() *InstrumentationConfig {
 		InfluxTables:         DefaultInfluxTables,
 		PyroscopeURL:         "",
 		PyroscopeTrace:       false,
-		PyroscopeProfileTypes: []string{
+		PyroscopeProfileTypes: strings.Join([]string{
 			"cpu",
 			"alloc_objects",
 			"inuse_objects",
@@ -1243,6 +1246,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"
 
 	"github.com/cometbft/cometbft/config"
@@ -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/cometbft/cometbft/config"
+import (
+	"strings"
+
+	"github.com/cometbft/cometbft/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