Skip to content

Commit

Permalink
changefeedccl: give notice when metrics_label set without child_metrics
Browse files Browse the repository at this point in the history
Resolves #75682

Surfaces a notice of
```
server.child_metrics.enabled is set to false, metrics will only be published to
the '<scope>' label when it is set to true"
```
When child_metrics setting isn't enabled during changefeed creation

Release note (enterprise change): Changefeeds created/altered with a
metrics_label set while server.child_metrics.enabled is false will now provide
the user a notice upon creation.

<what was there before: Previously, ...>
<why it needed to change: This was inadequate because ...>
<what you did about it: To address this, this patch ...>
  • Loading branch information
samiskin committed Jan 9, 2023
1 parent 01032c2 commit f5a5b4b
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 2 deletions.
1 change: 1 addition & 0 deletions pkg/ccl/changefeedccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ go_library(
"//pkg/scheduledjobs",
"//pkg/scheduledjobs/schedulebase",
"//pkg/security/username",
"//pkg/server/status",
"//pkg/server/telemetry",
"//pkg/settings",
"//pkg/settings/cluster",
Expand Down
9 changes: 9 additions & 0 deletions pkg/ccl/changefeedccl/changefeed_stmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/protectedts"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/protectedts/ptpb"
"github.com/cockroachdb/cockroach/pkg/server/status"
"github.com/cockroachdb/cockroach/pkg/server/telemetry"
"github.com/cockroachdb/cockroach/pkg/settings"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
Expand Down Expand Up @@ -581,6 +582,14 @@ func createChangefeedJobRecord(
"Otherwise, please re-run with a different %[1]q value.",
changefeedbase.OptMetricsScope, defaultSLIScope)
}

if !status.ChildMetricsEnabled.Get(&p.ExecCfg().Settings.SV) {
p.BufferClientNotice(ctx, pgnotice.Newf(
"%s is set to false, metrics will only be published to the '%s' label when it is set to true",
status.ChildMetricsEnabled.Key(),
scope,
))
}
}

details.Opts = opts.AsMap()
Expand Down
25 changes: 25 additions & 0 deletions pkg/ccl/changefeedccl/changefeed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7658,6 +7658,31 @@ func TestPartitionSpans(t *testing.T) {
}
}

func TestChangefeedMetricsScopeNotice(t *testing.T) {
defer leaktest.AfterTest(t)()

s, stopServer := makeServer(t)
defer stopServer()
sqlDB := sqlutils.MakeSQLRunner(s.DB)
sqlDB.Exec(t, "CREATE table foo (i int)")
sqlDB.Exec(t, `SET CLUSTER SETTING server.child_metrics.enabled = false`)

sqlCreate := "CREATE CHANGEFEED FOR d.foo INTO 'null://' WITH metrics_label='scope'"
expectNotice(t, s.Server, sqlCreate, `server.child_metrics.enabled is set to false, metrics will only be published to the 'scope' label when it is set to true`)

var jobID string
sqlDB.QueryRow(t, `SELECT job_id FROM [SHOW JOBS] where job_type='CHANGEFEED'`).Scan(&jobID)
sqlDB.Exec(t, "PAUSE JOB $1", jobID)
sqlDB.CheckQueryResultsRetry(
t,
fmt.Sprintf(`SELECT count(*) FROM [SHOW JOBS] WHERE job_type='CHANGEFEED' AND status='%s'`, jobs.StatusPaused),
[][]string{{"1"}},
)

sqlAlter := fmt.Sprintf("ALTER CHANGEFEED %s SET metrics_label='other'", jobID)
expectNotice(t, s.Server, sqlAlter, `server.child_metrics.enabled is set to false, metrics will only be published to the 'other' label when it is set to true`)
}

// TestPubsubValidationErrors tests error messages during pubsub sink URI validations.
func TestPubsubValidationErrors(t *testing.T) {
defer leaktest.AfterTest(t)()
Expand Down
5 changes: 3 additions & 2 deletions pkg/server/status/recorder.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ type storeMetrics interface {
Registry() *metric.Registry
}

var childMetricsEnabled = settings.RegisterBoolSetting(
// ChildMetricsEnabled enables exporting of additional prometheus time series with extra labels
var ChildMetricsEnabled = settings.RegisterBoolSetting(
settings.TenantWritable, "server.child_metrics.enabled",
"enables the exporting of child metrics, additional prometheus time series with extra labels",
false).WithPublic()
Expand Down Expand Up @@ -245,7 +246,7 @@ func (mr *MetricsRecorder) ScrapeIntoPrometheus(pm *metric.PrometheusExporter) {
log.Warning(context.TODO(), "MetricsRecorder asked to scrape metrics before NodeID allocation")
}
}
includeChildMetrics := childMetricsEnabled.Get(&mr.settings.SV)
includeChildMetrics := ChildMetricsEnabled.Get(&mr.settings.SV)
pm.ScrapeRegistry(mr.mu.nodeRegistry, includeChildMetrics)
for _, reg := range mr.mu.storeRegistries {
pm.ScrapeRegistry(reg, includeChildMetrics)
Expand Down

0 comments on commit f5a5b4b

Please sign in to comment.