Skip to content

Commit

Permalink
Revert "fix!: change the instrumentation config to circumvent viper b… (
Browse files Browse the repository at this point in the history
#1167)

…ug (#1143)"

This reverts commit f1dc8f7.

which is a breaking change to the config
  • Loading branch information
evan-forbes authored Jan 10, 2024
1 parent f2d6da5 commit 92efcbe
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 56 deletions.
16 changes: 6 additions & 10 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"net/http"
"os"
"path/filepath"
"strings"
"time"
)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -1233,7 +1230,7 @@ func DefaultInstrumentationConfig() *InstrumentationConfig {
InfluxTables: DefaultInfluxTables,
PyroscopeURL: "",
PyroscopeTrace: false,
PyroscopeProfileTypes: strings.Join([]string{
PyroscopeProfileTypes: []string{
"cpu",
"alloc_objects",
"inuse_objects",
Expand All @@ -1243,7 +1240,6 @@ func DefaultInstrumentationConfig() *InstrumentationConfig {
"block_count",
"block_duration",
},
","),
}
}

Expand Down
10 changes: 4 additions & 6 deletions config/toml.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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}}]
`

Expand Down
5 changes: 2 additions & 3 deletions node/tracing.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
32 changes: 3 additions & 29 deletions pkg/trace/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package trace
import (
"context"
"fmt"
"strings"
"time"

influxdb2 "github.com/influxdata/influxdb-client-go/v2"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
8 changes: 2 additions & 6 deletions pkg/trace/schema/tables.go
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/pkg/infrastructure.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/pkg/testnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 92efcbe

Please sign in to comment.