tsdump: histograms are missing

[tbg]

In the tsdump in #106140 (comment), the histogram quantiles are not recorded. For example, exec.latency is not there. I remember this working at some point; unclear if it's an issue with this particular tsdump or a regression.

tsdump.gob.yaml (July 5-6)

1: 1
2: 2
3: 3
4: 4
5: 5
6: 6
7: 7
8: 8

Jira issue: CRDB-30167

[blathers-crl]

Hi @tbg, please add branch-* labels to identify which branch(es) this release-blocker affects.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[ericharmeling]

We're not actually constructing the histogram names for most histograms in the tsdump code path.

GetInternalTimeseriesNamesFromServer checks the meta.MetricType of the metadata for each metric here:

cockroach/pkg/server/serverpb/admin.go

Line 83 in 62e59ec

func GetInternalTimeseriesNamesFromServer(

The zero value for the MetricType in the prometheus go client is COUNTER: https://github.com/prometheus/client_model/blob/62e59ec852bda61ad6d00519f9e81595c41f865b/go/metrics.pb.go#L41

The histogram mentioned in this issue (exec.latency) does not specify a MetricType:

cockroach/pkg/server/node.go

Lines 93 to 108 in 62e59ec

	metaExecLatency = metric.Metadata{
	Name: "exec.latency",
	Help: `Latency of batch KV requests (including errors) executed on this node.
	This measures requests already addressed to a single replica, from the moment
	at which they arrive at the internal gRPC endpoint to the moment at which the
	response (or an error) is returned.
	This latency includes in particular commit waits, conflict resolution and replication,
	and end-users can easily produce high measurements via long-running transactions that
	conflict with foreground traffic. This metric thus does not provide a good signal for
	understanding the health of the KV layer.
	`,
	Measurement: "Latency",
	Unit: metric.Unit_NANOSECONDS,
	}

In fact, the only metrics for which we do specify a MetricType are ttljob_metrics:

cockroach/pkg/sql/ttl/ttljob/ttljob_metrics.go

Lines 102 to 134 in 62e59ec

	SpanTotalDuration: b.Histogram(metric.HistogramOptions{
	Metadata: metric.Metadata{
	Name: "jobs.row_level_ttl.span_total_duration",
	Help: "Duration for processing a span during row level TTL.",
	Measurement: "nanoseconds",
	Unit: metric.Unit_NANOSECONDS,
	MetricType: io_prometheus_client.MetricType_HISTOGRAM,
	},
	MaxVal: time.Hour.Nanoseconds(),
	SigFigs: sigFigs,
	Duration: histogramWindowInterval,
	BucketConfig: metric.LongRunning60mLatencyBuckets,
	}),
	SelectDuration: b.Histogram(metric.HistogramOptions{
	Metadata: metric.Metadata{
	Name: "jobs.row_level_ttl.select_duration",
	Help: "Duration for select requests during row level TTL.",
	Measurement: "nanoseconds",
	Unit: metric.Unit_NANOSECONDS,
	MetricType: io_prometheus_client.MetricType_HISTOGRAM,
	},
	MaxVal: time.Minute.Nanoseconds(),
	SigFigs: sigFigs,
	Duration: histogramWindowInterval,
	BucketConfig: metric.BatchProcessLatencyBuckets,
	}),
	DeleteDuration: b.Histogram(metric.HistogramOptions{
	Metadata: metric.Metadata{
	Name: "jobs.row_level_ttl.delete_duration",
	Help: "Duration for delete requests during row level TTL.",
	Measurement: "nanoseconds",
	Unit: metric.Unit_NANOSECONDS,
	MetricType: io_prometheus_client.MetricType_HISTOGRAM,

And these are the only histogram metrics that actually show up in the tsdump.

Rather than explicitly specify a metric type for each histogram, we should be assigning this type on histogram creation, or returning it on metadata introspection. This PR should resolve this issue: #108597