Skip to content
Open
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions google/export/series_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,14 @@ func (c *seriesCache) populate(ref storage.SeriesRef, entry *seriesCacheEntry, e
metric_pb.MetricDescriptor_DOUBLE,
)

case model.MetricTypeInfo:
protos.gauge = newSeries(
c.getMetricType(metricName, gcmMetricSuffixGauge, gcmMetricSuffixNone),
Copy link
Collaborator

Choose a reason for hiding this comment

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

One question to answer before we move forward: Should we reuse gauge suffixgcmMetricSuffixGauge or add some /info suffix and if that decision even matter now (e.g. we can easily change later). Gauge would totally work for now. The only reason to do info is to be forward compatible with potential special handling of info in PromQL later on. Similar for stateset.

FYI: the definitions of those rare types are here: https://prometheus.io/docs/specs/om/open_metrics_spec/#info

I think it's fine to have gauge for both now, because:

  • There are no clear plans how to have special PromQL handling for info and stateset. There are small discussions on some storage efficiency gains for native info/stateset samples, but that's not impacting us.
  • If, ever, we will make breaking changes to Prometheus to suddenly treat info and stateset specially on PromQL it will be a big thing and in 4.0 Prometheus, soo.. YAGNI.
  • Otel collector is already assuming a "sum" (gauge) AFAIK

@lyanco @realschwa @dashpole to double check my thinking (:

FYI: There's some discussions to add info and stateset natively to SDK (https://cloud-native.slack.com/archives/CC6CPDEJV/p1762847796024439), so they will be more popular, but again, will be treated as a gauge on consumption.

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1 to YAGNI - gauge seems fine for now and it matches other info-like metrics that already exist (e.g. the ones from ksm).

Do these metrics typically end in _info? If so we can always special case metrics that end in _info/gauge if we ever need to. Or later on if /info becomes necessary we could always change how we write these metrics to include a /info suffix, or double-write (cardinality is not high for these, so cost isn't either), or something.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The info suffix is specified for OpenMetrics. Not sure how well that translates to real-world usage.

metadata.Help,
metric_pb.MetricDescriptor_GAUGE,
metric_pb.MetricDescriptor_DOUBLE,
)

case model.MetricTypeUnknown:
protos.gauge = newSeries(
c.getMetricType(metricName, gcmMetricSuffixUnknown, gcmMetricSuffixNone),
Expand Down