Skip to content

fix(config): disable default Prometheus telemetry sink (CON-335) #4032

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
amir-deris wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(config): disable default Prometheus telemetry sink (CON-335) #4032

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 11 additions & 12 deletions config/cosmosbase/agreement_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,17 @@ import (
// Held as text because the two sides carry different Go types for the same key often enough that comparing
// values would be comparing shapes. What matters here is which keys disagree and what a node gets instead.
var legacyConfigManagerDefaults = map[string]string{
"api.address": "",
"api.max-open-connections": "0",
"api.rpc-max-body-bytes": "0",
"api.rpc-read-timeout": "0",
"api.swagger": "false",
"grpc.enable": "true",
"minimum-gas-prices": "",
"occ-enabled": "false",
"pruning": "default",
"pruning-keep-every": "",
"telemetry.enabled": "false",
"telemetry.prometheus-retention-time": "0",
"api.address": "",
"api.max-open-connections": "0",
"api.rpc-max-body-bytes": "0",
"api.rpc-read-timeout": "0",
"api.swagger": "false",
"grpc.enable": "true",
"minimum-gas-prices": "",
"occ-enabled": "false",
"pruning": "default",
"pruning-keep-every": "",
"telemetry.enabled": "false",
}

// reasoning says what a node gets under that manager, for the keys where it is worth stating.
Expand Down
2 changes: 1 addition & 1 deletion sei-cosmos/server/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ func DefaultConfig() *Config {
},
Telemetry: telemetry.Config{
Enabled: true,
PrometheusRetentionTime: 7200,
PrometheusRetentionTime: 0,

@seidroid seidroid Bot Aug 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[blocker] This only changes one of the two app.toml-generation pipelines. cmd/seid/cmd/root.go:421 still sets srvCfg.Telemetry.PrometheusRetentionTime = 60 on the config returned by initAppConfig, and that config is what PersistentPreRunE → mgr.Apply → InterceptConfigsPreRunHandler (sei-cosmos/server/util.go:296-313) writes whenever app.toml is missing for any command other than init. So a node whose app.toml is auto-created by seid start (or any other non-init subcommand) still gets prometheus-retention-time = 60 and still starts the Prometheus sink with no operator opt-in — the PR description's claim that "the Prometheus sink is only created when an operator explicitly sets a positive retention value" does not hold for that path.

Before this change both pipelines enabled the sink (7200 vs 60) and only the retention window differed; now they disagree about whether the sink exists at all, which is a new and silent inconsistency. Either drop the = 60 override in root.go too, or state explicitly why the self-written file should keep Prometheus on.

@bdchatham bdchatham Aug 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth taking a look at this one Amir. This is one of the challenges of existing config generation. It's done in multiple places. Ideally we won't need to deal with this in the near future

GlobalLabels: nil,
},
API: APIConfig{
Expand Down
9 changes: 3 additions & 6 deletions sei-cosmos/server/config/config_fuzz_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -951,8 +951,7 @@ var telemetryKeys = []configtest.KeySpec{
{
Key: "telemetry.prometheus-retention-time", Path: "PrometheusRetentionTime",
Cast: configtest.CastInt64, Unguarded: true,
Why: "the declared default is 7200 seconds and an absent key resolves 0, which telemetry " +
"reads as retaining nothing, so a scrape finds an empty store",
Why: "zero disables the Prometheus sink and an absent key resolves 0 the same way",
},
}

Expand Down Expand Up @@ -1123,10 +1122,8 @@ func TestGetConfigAbsentSectionDivergences(t *testing.T) {
{"grpc.enable", cfg.GRPC.Enable, def.GRPC.Enable, true},
{"grpc.address", cfg.GRPC.Address, def.GRPC.Address, true},
{"telemetry.enabled", cfg.Telemetry.Enabled, def.Telemetry.Enabled, true},
{
"telemetry.prometheus-retention-time",
cfg.Telemetry.PrometheusRetentionTime, def.Telemetry.PrometheusRetentionTime, true,
},
{"telemetry.prometheus-retention-time",
cfg.Telemetry.PrometheusRetentionTime, def.Telemetry.PrometheusRetentionTime, false},

// [api]. Five diverge. The three set false have a declared default that is already the
// getter's zero, so nothing about the resolved value distinguishes a guard from its absence.
Expand Down
2 changes: 1 addition & 1 deletion sei-cosmos/server/config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestDefaultSwaggerConfig(t *testing.T) {
func TestDefaultTelemetryConfig(t *testing.T) {
cfg := DefaultConfig()
require.True(t, cfg.Telemetry.Enabled, "Telemetry should be enabled by default")
require.Equal(t, int64(7200), cfg.Telemetry.PrometheusRetentionTime)
require.Equal(t, int64(0), cfg.Telemetry.PrometheusRetentionTime)
require.Empty(t, cfg.Telemetry.GlobalLabels)
}

Expand Down
2 changes: 1 addition & 1 deletion sei-cosmos/server/config/testdata/server_config.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Telemetry.Enabled = bool(true)
Telemetry.EnableHostname = bool(false)
Telemetry.EnableHostnameLabel = bool(false)
Telemetry.EnableServiceLabel = bool(false)
Telemetry.PrometheusRetentionTime = int64(7200)
Telemetry.PrometheusRetentionTime = int64(0)
Telemetry.GlobalLabels = <nil-slice>
API.Enable = bool(false)
API.Swagger = bool(true)
Expand Down
12 changes: 12 additions & 0 deletions sei-cosmos/telemetry/metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,18 @@ func TestMetrics_InMem(t *testing.T) {
require.Equal(t, 10, jsonMetrics.Counters[0].Count)
}

func TestMetrics_PromDisabledByDefaultRetention(t *testing.T) {
m, err := New(Config{
Enabled: true,
EnableHostname: false,
ServiceName: "test",
PrometheusRetentionTime: 0,
})
require.NoError(t, err)
require.NotNil(t, m)
require.False(t, m.prometheusEnabled)
}

func TestMetrics_Prom(t *testing.T) {
m, err := New(Config{
Enabled: true,
Expand Down
Loading