Skip to content

Commit

Permalink
fix: proper error when parsing telemetry configuration (backport cosm…
Browse files Browse the repository at this point in the history
…os#12981) (cosmos#12998)

* fix: proper error when parsing telemetry configuration (cosmos#12981)

When parsing `telemetry.global-labels` config the code assumes that the type will be an array.  I saw an issue where someone edited the configuration in the wrong way and got the following error:
![photo_2022-08-21_08-02-21](https://user-images.githubusercontent.com/22855163/185793842-c5759a54-1860-4dd1-bdb4-b94f4dab3c16.jpg)
Instead, I suggest here to print a proper error log to indicate what the issue is.

(cherry picked from commit c24c439)

* add changelog

Co-authored-by: liorbond <liorbond@gmail.com>
Co-authored-by: Julien Robert <julien@rbrt.fr>
  • Loading branch information
3 people authored and JeancarloBarrios committed Sep 28, 2024
1 parent 5301d1f commit 1099934
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements

* [#12981](https://github.com/cosmos/cosmos-sdk/pull/12981) Return proper error when parsing telemetry configuration.
* [#12969](https://github.com/cosmos/cosmos-sdk/pull/12969) Bump Tendermint to `v0.34.21` and IAVL to `v0.19.1`.
* [#12886](https://github.com/cosmos/cosmos-sdk/pull/12886) Amortize cost of processing cache KV store.
* (events) [#12850](https://github.com/cosmos/cosmos-sdk/pull/12850) Add a new `fee_payer` attribute to the `tx` event that is emitted from the `DeductFeeDecorator` AnteHandler decorator.
Expand Down
79 changes: 75 additions & 4 deletions server/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,11 +267,82 @@ func DefaultConfig() *Config {

// GetConfig returns a fully parsed Config object.
func GetConfig(v *viper.Viper) (Config, error) {
conf := DefaultConfig()
if err := v.Unmarshal(conf); err != nil {
return Config{}, fmt.Errorf("error extracting app config: %w", err)
globalLabelsRaw, ok := v.Get("telemetry.global-labels").([]interface{})
if !ok {
return Config{}, fmt.Errorf("failed to parse global-labels config")
}
return *conf, nil

globalLabels := make([][]string, 0, len(globalLabelsRaw))
for idx, glr := range globalLabelsRaw {
labelsRaw, ok := glr.([]interface{})
if !ok {
return Config{}, fmt.Errorf("failed to parse global label number %d from config", idx)
}
if len(labelsRaw) == 2 {
globalLabels = append(globalLabels, []string{labelsRaw[0].(string), labelsRaw[1].(string)})
}
}

return Config{
BaseConfig: BaseConfig{
MinGasPrices: v.GetString("minimum-gas-prices"),
InterBlockCache: v.GetBool("inter-block-cache"),
Pruning: v.GetString("pruning"),
PruningKeepRecent: v.GetString("pruning-keep-recent"),
PruningInterval: v.GetString("pruning-interval"),
HaltHeight: v.GetUint64("halt-height"),
HaltTime: v.GetUint64("halt-time"),
IndexEvents: v.GetStringSlice("index-events"),
MinRetainBlocks: v.GetUint64("min-retain-blocks"),
IAVLCacheSize: v.GetUint64("iavl-cache-size"),
AppDBBackend: v.GetString("app-db-backend"),
},
Telemetry: telemetry.Config{
ServiceName: v.GetString("telemetry.service-name"),
Enabled: v.GetBool("telemetry.enabled"),
EnableHostname: v.GetBool("telemetry.enable-hostname"),
EnableHostnameLabel: v.GetBool("telemetry.enable-hostname-label"),
EnableServiceLabel: v.GetBool("telemetry.enable-service-label"),
PrometheusRetentionTime: v.GetInt64("telemetry.prometheus-retention-time"),
GlobalLabels: globalLabels,
},
API: APIConfig{
Enable: v.GetBool("api.enable"),
Swagger: v.GetBool("api.swagger"),
Address: v.GetString("api.address"),
MaxOpenConnections: v.GetUint("api.max-open-connections"),
RPCReadTimeout: v.GetUint("api.rpc-read-timeout"),
RPCWriteTimeout: v.GetUint("api.rpc-write-timeout"),
RPCMaxBodyBytes: v.GetUint("api.rpc-max-body-bytes"),
EnableUnsafeCORS: v.GetBool("api.enabled-unsafe-cors"),
},
Rosetta: RosettaConfig{
Enable: v.GetBool("rosetta.enable"),
Address: v.GetString("rosetta.address"),
Blockchain: v.GetString("rosetta.blockchain"),
Network: v.GetString("rosetta.network"),
Retries: v.GetInt("rosetta.retries"),
Offline: v.GetBool("rosetta.offline"),
EnableFeeSuggestion: v.GetBool("rosetta.enable-fee-suggestion"),
GasToSuggest: v.GetInt("rosetta.gas-to-suggest"),
DenomToSuggest: v.GetString("rosetta.denom-to-suggest"),
},
GRPC: GRPCConfig{
Enable: v.GetBool("grpc.enable"),
Address: v.GetString("grpc.address"),
MaxRecvMsgSize: v.GetInt("grpc.max-recv-msg-size"),
MaxSendMsgSize: v.GetInt("grpc.max-send-msg-size"),
},
GRPCWeb: GRPCWebConfig{
Enable: v.GetBool("grpc-web.enable"),
Address: v.GetString("grpc-web.address"),
EnableUnsafeCORS: v.GetBool("grpc-web.enable-unsafe-cors"),
},
StateSync: StateSyncConfig{
SnapshotInterval: v.GetUint64("state-sync.snapshot-interval"),
SnapshotKeepRecent: v.GetUint32("state-sync.snapshot-keep-recent"),
},
}, nil
}

// ValidateBasic returns an error if min-gas-prices field is empty in BaseConfig. Otherwise, it returns nil.
Expand Down
14 changes: 12 additions & 2 deletions server/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,13 @@ func start[T types.Application](svrCtx *Context, clientCtx client.Context, appCr
}

app := appCreator(ctx.Logger, db, traceWriter, ctx.Viper)
_, err = startTelemetry(serverconfig.GetConfig(ctx.Viper))

config, err := serverconfig.GetConfig(ctx.Viper)
if err != nil {
return err
}

_, err = startTelemetry(config)
if err != nil {
return err
}
Expand Down Expand Up @@ -338,11 +344,15 @@ func startInProcess[T types.Application](svrCtx *Context, svrCfg serverconfig.Co
return err
}

config, err := config.GetConfig(ctx.Viper)
config, err := serverconfig.GetConfig(ctx.Viper)
if err != nil {
return err
}

if err := config.ValidateBasic(); err != nil {
return err
}

if err := config.ValidateBasic(); err != nil {
ctx.Logger.Error("WARNING: The minimum-gas-prices config in app.toml is set to the empty string. " +
"This defaults to 0 in the current version, but will error in the next version " +
Expand Down

0 comments on commit 1099934

Please sign in to comment.