Skip to content

Commit 588f079

Browse files
metric: embed source file in Metadata for CODEOWNERS resolution
Previously, metric ownership was determined by a separate gen-metric-owners tool that scanned Go source files via AST to find metric.Metadata definitions, mapped each to its owning team via CODEOWNERS, and produced a metric_owners.yaml file. This file was checked into the tree but had no Bazel genrule, and required a bespoke step in both ./dev generate and check-generated-code CI. This commit eliminates that pipeline by embedding source file information directly in metric.Metadata at definition time. A new source_file proto field is populated by metric.NewMetadata(), which captures the caller's file via runtime.Caller. At generation time, cockroach gen metric-list resolves ownership by calling codeowners.Match on each metric's source file — no separate AST scanning needed. To enforce adoption, a new metadatanew nogo analyzer flags bare metric.Metadata{} composite literals that aren't wrapped in NewMetadata(). The metric registry also logs a warning when metrics are registered without a source file set. Removed: - pkg/cmd/gen-metric-owners and its Bazel target - pkg/internal/metricscan/metric_owners.yaml and its go:embed wrapper - The generateMetricOwners step from ./dev generate - The bazel run //pkg/cmd/gen-metric-owners step from check-generated-code.sh - The --metric-owners flag from cockroach gen metric-list Epic: none Release note: None Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
1 parent 001a9ae commit 588f079

153 files changed

Lines changed: 3282 additions & 4600 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ nogo(
205205
"//pkg/testutils/lint/passes/grpcstatuswithdetails",
206206
"//pkg/testutils/lint/passes/hash",
207207
"//pkg/testutils/lint/passes/leaktestcall",
208+
"//pkg/testutils/lint/passes/metadatanew",
208209
"//pkg/testutils/lint/passes/nilness",
209210
"//pkg/testutils/lint/passes/nocopy",
210211
"//pkg/testutils/lint/passes/redactcheck",

build/bazelutil/nogo_config.json

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1654,5 +1654,16 @@
16541654
"external/": "exclude all third-party code for all analyzers",
16551655
"pkg/sql/colexec/colexechash/hash\\.go$": "re-implements runtime.noescape for efficient hashing"
16561656
}
1657+
},
1658+
"metadatanew": {
1659+
"exclude_files": {
1660+
"external/": "exclude all third-party code",
1661+
".*\\.pb\\.go$": "generated code",
1662+
".*\\.pb\\.gw\\.go$": "generated code",
1663+
"pkg/.*\\.eg\\.go$": "generated code",
1664+
"pkg/.*_generated\\.go$": "generated code",
1665+
"pkg/util/metric/.*\\.go$": "metric package internal usage is allowed",
1666+
"pkg/testutils/": "test utilities may construct Metadata for testing"
1667+
}
16571668
}
16581669
}

build/github/check-generated-code.sh

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,6 @@ if grep TODO DEPS.bzl; then
6161
fi
6262
check_workspace_clean "Run \`./dev generate bazel\` to fix this error."
6363

64-
# Run gen-metric-owners to ensure metric_owners.yaml is up to date
65-
# before //pkg/gen regenerates metrics.yaml from it.
66-
bazel run //pkg/cmd/gen-metric-owners $ENGFLOW_ARGS -- -out="$(pwd)/pkg/internal/metricscan/metric_owners.yaml"
67-
6864
# Run `bazel run //pkg/gen` and ensure nothing changes. This ensures
6965
# generated documentation and checked-in go code are up to date.
7066
bazel run //pkg/gen $ENGFLOW_ARGS

docs/generated/metrics/BUILD.bazel

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
genrule(
22
name = "metrics",
3-
srcs = [
4-
"//pkg/internal/metricscan:metric_owners.yaml",
5-
],
63
outs = ["metrics.yaml"],
7-
cmd = "$(location //pkg/cmd/cockroach-short) gen metric-list --logtostderr=NONE --metric-owners=$(location //pkg/internal/metricscan:metric_owners.yaml) > $@",
4+
cmd = "$(location //pkg/cmd/cockroach-short) gen metric-list --logtostderr=NONE > $@",
85
tools = ["//pkg/cmd/cockroach-short"],
96
visibility = [
107
":__pkg__",

pkg/BUILD.bazel

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1208,8 +1208,6 @@ GO_TARGETS = [
12081208
"//pkg/cmd/flaky-test-notifier:flaky-test-notifier_lib",
12091209
"//pkg/cmd/fuzz:fuzz",
12101210
"//pkg/cmd/fuzz:fuzz_lib",
1211-
"//pkg/cmd/gen-metric-owners:gen-metric-owners",
1212-
"//pkg/cmd/gen-metric-owners:gen-metric-owners_lib",
12131211
"//pkg/cmd/generate-acceptance-tests:generate-acceptance-tests",
12141212
"//pkg/cmd/generate-acceptance-tests:generate-acceptance-tests_lib",
12151213
"//pkg/cmd/generate-ash-inventory:generate-ash-inventory",
@@ -2761,6 +2759,7 @@ GO_TARGETS = [
27612759
"//pkg/testutils/lint/passes/hash:hash_test",
27622760
"//pkg/testutils/lint/passes/leaktestcall:leaktestcall",
27632761
"//pkg/testutils/lint/passes/leaktestcall:leaktestcall_test",
2762+
"//pkg/testutils/lint/passes/metadatanew:metadatanew",
27642763
"//pkg/testutils/lint/passes/nilness:nilness",
27652764
"//pkg/testutils/lint/passes/nilness:nilness_test",
27662765
"//pkg/testutils/lint/passes/nocopy:nocopy",

pkg/backup/backup_metrics.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,12 @@ func (b BackupMetrics) MetricStruct() {}
2222
// MakeBackupMetrics instantiates the metrics for backup.
2323
func MakeBackupMetrics(time.Duration) metric.Struct {
2424
m := &BackupMetrics{
25-
LastKMSInaccessibleErrorTime: metric.NewGauge(metric.Metadata{
25+
LastKMSInaccessibleErrorTime: metric.NewGauge(metric.NewMetadata(metric.Metadata{
2626
Name: "backup.last-failed-time.kms-inaccessible",
2727
Help: "The unix timestamp of the most recent failure of backup due to errKMSInaccessible by a backup specified as maintaining this metric",
2828
Measurement: "Jobs",
2929
Unit: metric.Unit_TIMESTAMP_SEC,
30-
}),
30+
})),
3131
}
3232
return m
3333
}

pkg/backup/schedule_exec.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,7 @@ func init() {
606606
metrics: backupMetrics{
607607
ExecutorMetrics: &m,
608608
ExecutorPTSMetrics: &pm,
609-
RpoMetric: metric.NewGauge(metric.Metadata{
609+
RpoMetric: metric.NewGauge(metric.NewMetadata(metric.Metadata{
610610
Name: "schedules.BACKUP.last-completed-time",
611611
Help: crstrings.UnwrapText(`
612612
The unix timestamp of the most recently completed backup by a
@@ -629,16 +629,16 @@ func init() {
629629
For example with a backup frequency of 60 minutes, monitor
630630
time() - max_across_nodes(max_over_time(schedules_BACKUP_last_completed_time, 60min)).
631631
`),
632-
}),
633-
RpoTenantMetric: metric.NewExportedGaugeVec(metric.Metadata{
632+
})),
633+
RpoTenantMetric: metric.NewExportedGaugeVec(metric.NewMetadata(metric.Metadata{
634634
Name: "schedules.BACKUP.last-completed-time-by-virtual_cluster",
635635
Help: crstrings.UnwrapText(`
636636
The unix timestamp of the most recently completed host scheduled
637637
backup by virtual cluster specified as maintaining this metric
638638
`),
639639
Measurement: "Jobs",
640640
Unit: metric.Unit_TIMESTAMP_SEC,
641-
}, []string{"tenant_id"}),
641+
}), []string{"tenant_id"}),
642642
},
643643
}, nil
644644
})

pkg/backup/schedule_exec_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ func TestBackupSucceededUpdatesMetrics(t *testing.T) {
2727
ctx := context.Background()
2828
executor := &scheduledBackupExecutor{
2929
metrics: backupMetrics{
30-
RpoMetric: metric.NewGauge(metric.Metadata{}),
31-
RpoTenantMetric: metric.NewExportedGaugeVec(metric.Metadata{}, []string{"tenant_id"}),
30+
RpoMetric: metric.NewGauge(metric.NewMetadata(metric.Metadata{})),
31+
RpoTenantMetric: metric.NewExportedGaugeVec(metric.NewMetadata(metric.Metadata{}), []string{"tenant_id"}),
3232
},
3333
}
3434

@@ -63,8 +63,8 @@ func TestBackupSucceededUpdatesMetrics(t *testing.T) {
6363
// Use a fresh executor to ensure RpoMetric starts at zero.
6464
freshExecutor := &scheduledBackupExecutor{
6565
metrics: backupMetrics{
66-
RpoMetric: metric.NewGauge(metric.Metadata{}),
67-
RpoTenantMetric: metric.NewExportedGaugeVec(metric.Metadata{}, []string{"tenant_id"}),
66+
RpoMetric: metric.NewGauge(metric.NewMetadata(metric.Metadata{})),
67+
RpoTenantMetric: metric.NewExportedGaugeVec(metric.NewMetadata(metric.Metadata{}), []string{"tenant_id"}),
6868
},
6969
}
7070
schedule := createSchedule(t, true)

pkg/ccl/changefeedccl/cdcutils/throttle.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,13 +145,13 @@ type Metrics struct {
145145
// MakeMetrics constructs a Metrics struct with the provided histogram window.
146146
func MakeMetrics(histogramWindow time.Duration) Metrics {
147147
makeMetric := func(n string) metric.Metadata {
148-
return metric.Metadata{
148+
return metric.NewMetadata(metric.Metadata{
149149
Name: fmt.Sprintf("changefeed.%s.messages_pushback_nanos", n),
150150
Help: fmt.Sprintf("Total time spent throttled for %s quota", n),
151151
Measurement: "Nanoseconds",
152152
Unit: metric.Unit_NANOSECONDS,
153153
Category: metric.Metadata_CHANGEFEEDS,
154-
}
154+
})
155155
}
156156

157157
return Metrics{

pkg/ccl/changefeedccl/checkpoint/metrics.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,37 +12,37 @@ import (
1212
)
1313

1414
var (
15-
metaCreateNanos = metric.Metadata{
15+
metaCreateNanos = metric.NewMetadata(metric.Metadata{
1616
Name: "changefeed.checkpoint.create_nanos",
1717
Help: "Time it takes to create a changefeed checkpoint",
1818
Unit: metric.Unit_NANOSECONDS,
1919
Measurement: "Nanoseconds",
2020
Category: metric.Metadata_CHANGEFEEDS,
21-
}
21+
})
2222

23-
metaTotalBytes = metric.Metadata{
23+
metaTotalBytes = metric.NewMetadata(metric.Metadata{
2424
Name: "changefeed.checkpoint.total_bytes",
2525
Help: "Total size of a changefeed checkpoint",
2626
Unit: metric.Unit_BYTES,
2727
Measurement: "Bytes",
2828
Category: metric.Metadata_CHANGEFEEDS,
29-
}
29+
})
3030

31-
metaTimestampCount = metric.Metadata{
31+
metaTimestampCount = metric.NewMetadata(metric.Metadata{
3232
Name: "changefeed.checkpoint.timestamp_count",
3333
Help: "Number of unique timestamps in a changefeed checkpoint",
3434
Unit: metric.Unit_COUNT,
3535
Measurement: "Timestamps",
3636
Category: metric.Metadata_CHANGEFEEDS,
37-
}
37+
})
3838

39-
metaSpanCount = metric.Metadata{
39+
metaSpanCount = metric.NewMetadata(metric.Metadata{
4040
Name: "changefeed.checkpoint.span_count",
4141
Help: "Number of spans in a changefeed checkpoint",
4242
Unit: metric.Unit_COUNT,
4343
Measurement: "Spans",
4444
Category: metric.Metadata_CHANGEFEEDS,
45-
}
45+
})
4646
)
4747

4848
type AggMetrics struct {

0 commit comments

Comments
 (0)