From f095c33da12958b5d92a7d857264fafb8d4c549a Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 20 Nov 2023 19:28:05 +0000 Subject: [PATCH 01/23] scrape: simplify TargetsActive function Since everything was serialized on a single mutex, it's exactly the same if we process targets in sequence without starting goroutines. Signed-off-by: Bryan Boreham --- scrape/manager.go | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/scrape/manager.go b/scrape/manager.go index a0ac38f6b..96cad00c9 100644 --- a/scrape/manager.go +++ b/scrape/manager.go @@ -279,24 +279,10 @@ func (m *Manager) TargetsActive() map[string][]*Target { m.mtxScrape.Lock() defer m.mtxScrape.Unlock() - var ( - wg sync.WaitGroup - mtx sync.Mutex - ) - targets := make(map[string][]*Target, len(m.scrapePools)) - wg.Add(len(m.scrapePools)) for tset, sp := range m.scrapePools { - // Running in parallel limits the blocking time of scrapePool to scrape - // interval when there's an update from SD. - go func(tset string, sp *scrapePool) { - mtx.Lock() - targets[tset] = sp.ActiveTargets() - mtx.Unlock() - wg.Done() - }(tset, sp) + targets[tset] = sp.ActiveTargets() } - wg.Wait() return targets } From 34676a240e21705e8832e7be0b40df785bc06eb8 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 22 Nov 2023 18:50:57 +0000 Subject: [PATCH 02/23] scrape: consistent function names for metadata Too confusing to have `MetadataList` and `ListMetadata`, etc. I standardised on the ones which are in an interface. Signed-off-by: Bryan Boreham --- scrape/metrics.go | 4 ++-- scrape/target.go | 10 +++++----- storage/remote/metadata_watcher.go | 2 +- web/api/v1/api.go | 8 ++++---- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/scrape/metrics.go b/scrape/metrics.go index d74143185..7082bc743 100644 --- a/scrape/metrics.go +++ b/scrape/metrics.go @@ -286,8 +286,8 @@ func (mc *MetadataMetricsCollector) Collect(ch chan<- prometheus.Metric) { for tset, targets := range mc.TargetsGatherer.TargetsActive() { var size, length int for _, t := range targets { - size += t.MetadataSize() - length += t.MetadataLength() + size += t.SizeMetadata() + length += t.LengthMetadata() } ch <- prometheus.MustNewConstMetric( diff --git a/scrape/target.go b/scrape/target.go index 8cc8597a4..c100e1bee 100644 --- a/scrape/target.go +++ b/scrape/target.go @@ -92,7 +92,7 @@ type MetricMetadata struct { Unit string } -func (t *Target) MetadataList() []MetricMetadata { +func (t *Target) ListMetadata() []MetricMetadata { t.mtx.RLock() defer t.mtx.RUnlock() @@ -102,7 +102,7 @@ func (t *Target) MetadataList() []MetricMetadata { return t.metadata.ListMetadata() } -func (t *Target) MetadataSize() int { +func (t *Target) SizeMetadata() int { t.mtx.RLock() defer t.mtx.RUnlock() @@ -113,7 +113,7 @@ func (t *Target) MetadataSize() int { return t.metadata.SizeMetadata() } -func (t *Target) MetadataLength() int { +func (t *Target) LengthMetadata() int { t.mtx.RLock() defer t.mtx.RUnlock() @@ -124,8 +124,8 @@ func (t *Target) MetadataLength() int { return t.metadata.LengthMetadata() } -// Metadata returns type and help metadata for the given metric. -func (t *Target) Metadata(metric string) (MetricMetadata, bool) { +// GetMetadata returns type and help metadata for the given metric. +func (t *Target) GetMetadata(metric string) (MetricMetadata, bool) { t.mtx.RLock() defer t.mtx.RUnlock() diff --git a/storage/remote/metadata_watcher.go b/storage/remote/metadata_watcher.go index 21de565ed..abfea3c7b 100644 --- a/storage/remote/metadata_watcher.go +++ b/storage/remote/metadata_watcher.go @@ -136,7 +136,7 @@ func (mw *MetadataWatcher) collect() { metadata := []scrape.MetricMetadata{} for _, tset := range mw.manager.TargetsActive() { for _, target := range tset { - for _, entry := range target.MetadataList() { + for _, entry := range target.ListMetadata() { if _, ok := metadataSet[entry]; !ok { metadata = append(metadata, entry) metadataSet[entry] = struct{}{} diff --git a/web/api/v1/api.go b/web/api/v1/api.go index 671df7887..8fa7ce14a 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -1114,7 +1114,7 @@ func (api *API) targetMetadata(r *http.Request) apiFuncResult { } // If no metric is specified, get the full list for the target. if metric == "" { - for _, md := range t.MetadataList() { + for _, md := range t.ListMetadata() { res = append(res, metricMetadata{ Target: t.Labels(), Metric: md.Metric, @@ -1126,7 +1126,7 @@ func (api *API) targetMetadata(r *http.Request) apiFuncResult { continue } // Get metadata for the specified metric. - if md, ok := t.Metadata(metric); ok { + if md, ok := t.GetMetadata(metric); ok { res = append(res, metricMetadata{ Target: t.Labels(), Type: md.Type, @@ -1249,7 +1249,7 @@ func (api *API) metricMetadata(r *http.Request) apiFuncResult { for _, tt := range api.targetRetriever(r.Context()).TargetsActive() { for _, t := range tt { if metric == "" { - for _, mm := range t.MetadataList() { + for _, mm := range t.ListMetadata() { m := metadata{Type: mm.Type, Help: mm.Help, Unit: mm.Unit} ms, ok := metrics[mm.Metric] @@ -1266,7 +1266,7 @@ func (api *API) metricMetadata(r *http.Request) apiFuncResult { continue } - if md, ok := t.Metadata(metric); ok { + if md, ok := t.GetMetadata(metric); ok { m := metadata{Type: md.Type, Help: md.Help, Unit: md.Unit} ms, ok := metrics[md.Metric] From d12ccf9fa29dbc2d0f40013f04dbe7adb8a1113e Mon Sep 17 00:00:00 2001 From: Marcin Skalski Date: Mon, 11 Dec 2023 16:33:42 +0100 Subject: [PATCH 03/23] kuma_sd: Extend Kuma SD configuration to allow users to specify ClientId Signed-off-by: Marcin Skalski --- config/config_test.go | 1 + config/testdata/conf.good.yml | 1 + config/testdata/roundtrip.good.yml | 1 + discovery/xds/kuma.go | 18 ++++++++++++++---- discovery/xds/kuma_test.go | 2 +- discovery/xds/xds.go | 1 + discovery/xds/xds_test.go | 1 + docs/configuration/configuration.md | 5 +++++ 8 files changed, 25 insertions(+), 5 deletions(-) diff --git a/config/config_test.go b/config/config_test.go index 5d753a0f7..7c061fd54 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -568,6 +568,7 @@ var expectedConf = &Config{ ServiceDiscoveryConfigs: discovery.Configs{ &xds.KumaSDConfig{ Server: "http://kuma-control-plane.kuma-system.svc:5676", + ClientId: "main-prometheus", HTTPClientConfig: config.DefaultHTTPClientConfig, RefreshInterval: model.Duration(15 * time.Second), FetchTimeout: model.Duration(2 * time.Minute), diff --git a/config/testdata/conf.good.yml b/config/testdata/conf.good.yml index e034eff43..972099800 100644 --- a/config/testdata/conf.good.yml +++ b/config/testdata/conf.good.yml @@ -221,6 +221,7 @@ scrape_configs: kuma_sd_configs: - server: http://kuma-control-plane.kuma-system.svc:5676 + clientId: main-prometheus - job_name: service-marathon marathon_sd_configs: diff --git a/config/testdata/roundtrip.good.yml b/config/testdata/roundtrip.good.yml index f2634d257..26589ad1b 100644 --- a/config/testdata/roundtrip.good.yml +++ b/config/testdata/roundtrip.good.yml @@ -108,6 +108,7 @@ scrape_configs: kuma_sd_configs: - server: http://kuma-control-plane.kuma-system.svc:5676 + clientId: main-prometheus marathon_sd_configs: - servers: diff --git a/discovery/xds/kuma.go b/discovery/xds/kuma.go index bc88ba554..1cec053e4 100644 --- a/discovery/xds/kuma.go +++ b/discovery/xds/kuma.go @@ -178,10 +178,11 @@ func kumaMadsV1ResourceParser(resources []*anypb.Any, typeURL string) ([]model.L func NewKumaHTTPDiscovery(conf *KumaSDConfig, logger log.Logger) (discovery.Discoverer, error) { // Default to "prometheus" if hostname is unavailable. - clientID, err := osutil.GetFQDN() - if err != nil { - level.Debug(logger).Log("msg", "error getting FQDN", "err", err) - clientID = "prometheus" + var clientID string + if conf.ClientId == "" { + clientID = defaultClientId(logger) + } else { + clientID = conf.ClientId } clientConfig := &HTTPResourceClientConfig{ @@ -215,3 +216,12 @@ func NewKumaHTTPDiscovery(conf *KumaSDConfig, logger log.Logger) (discovery.Disc return d, nil } + +func defaultClientId(logger log.Logger) string { + clientID, err := osutil.GetFQDN() + if err != nil { + level.Debug(logger).Log("msg", "error getting FQDN", "err", err) + clientID = "prometheus" + } + return clientID +} diff --git a/discovery/xds/kuma_test.go b/discovery/xds/kuma_test.go index 581be9fb1..7f2b0ce3b 100644 --- a/discovery/xds/kuma_test.go +++ b/discovery/xds/kuma_test.go @@ -204,7 +204,7 @@ func TestNewKumaHTTPDiscovery(t *testing.T) { require.True(t, ok) require.Equal(t, kumaConf.Server, resClient.Server()) require.Equal(t, KumaMadsV1ResourceTypeURL, resClient.ResourceTypeURL()) - require.NotEmpty(t, resClient.ID()) + require.Equal(t, kumaConf.ClientId, resClient.ID()) require.Equal(t, KumaMadsV1ResourceType, resClient.config.ResourceType) } diff --git a/discovery/xds/xds.go b/discovery/xds/xds.go index 48bdbab02..47baece78 100644 --- a/discovery/xds/xds.go +++ b/discovery/xds/xds.go @@ -55,6 +55,7 @@ type SDConfig struct { RefreshInterval model.Duration `yaml:"refresh_interval,omitempty"` FetchTimeout model.Duration `yaml:"fetch_timeout,omitempty"` Server string `yaml:"server,omitempty"` + ClientId string `yaml:"clientId,omitempty"` } // mustRegisterMessage registers the provided message type in the typeRegistry, and panics diff --git a/discovery/xds/xds_test.go b/discovery/xds/xds_test.go index 974a47342..2e0f24e19 100644 --- a/discovery/xds/xds_test.go +++ b/discovery/xds/xds_test.go @@ -36,6 +36,7 @@ var ( sdConf = SDConfig{ Server: "http://127.0.0.1", RefreshInterval: model.Duration(10 * time.Second), + ClientId: "test-id", } testFetchFailuresCount = prometheus.NewCounter( diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index f05925d2b..d3ee459be 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -2230,6 +2230,11 @@ See below for the configuration options for Kuma MonitoringAssignment discovery: # Address of the Kuma Control Plane's MADS xDS server. server: +# Client id is used by Kuma Control Plane to compute Monitoring Assignment for specific Prometheus backend. +# This is useful when migrating between multiple Prometheus backends, or having separate backend for each Mesh +# When not specified, system hostname/fqdn will be used if available, if not `prometheus` will be used. +clientId: + # The time to wait between polling update requests. [ refresh_interval: | default = 30s ] From 0af810aa718a9cb9a9466785715103ad6117e959 Mon Sep 17 00:00:00 2001 From: Marcin Skalski Date: Mon, 11 Dec 2023 17:01:52 +0100 Subject: [PATCH 04/23] fix go lint Signed-off-by: Marcin Skalski --- config/testdata/conf.good.yml | 2 +- config/testdata/roundtrip.good.yml | 2 +- discovery/xds/kuma.go | 8 ++++---- discovery/xds/kuma_test.go | 2 +- discovery/xds/xds.go | 2 +- discovery/xds/xds_test.go | 2 +- docs/configuration/configuration.md | 2 +- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/config/testdata/conf.good.yml b/config/testdata/conf.good.yml index 972099800..b58430164 100644 --- a/config/testdata/conf.good.yml +++ b/config/testdata/conf.good.yml @@ -221,7 +221,7 @@ scrape_configs: kuma_sd_configs: - server: http://kuma-control-plane.kuma-system.svc:5676 - clientId: main-prometheus + client_id: main-prometheus - job_name: service-marathon marathon_sd_configs: diff --git a/config/testdata/roundtrip.good.yml b/config/testdata/roundtrip.good.yml index 26589ad1b..24ab7d259 100644 --- a/config/testdata/roundtrip.good.yml +++ b/config/testdata/roundtrip.good.yml @@ -108,7 +108,7 @@ scrape_configs: kuma_sd_configs: - server: http://kuma-control-plane.kuma-system.svc:5676 - clientId: main-prometheus + client_id: main-prometheus marathon_sd_configs: - servers: diff --git a/discovery/xds/kuma.go b/discovery/xds/kuma.go index 1cec053e4..e79195bb2 100644 --- a/discovery/xds/kuma.go +++ b/discovery/xds/kuma.go @@ -179,10 +179,10 @@ func kumaMadsV1ResourceParser(resources []*anypb.Any, typeURL string) ([]model.L func NewKumaHTTPDiscovery(conf *KumaSDConfig, logger log.Logger) (discovery.Discoverer, error) { // Default to "prometheus" if hostname is unavailable. var clientID string - if conf.ClientId == "" { - clientID = defaultClientId(logger) + if conf.ClientID == "" { + clientID = defaultClientID(logger) } else { - clientID = conf.ClientId + clientID = conf.ClientID } clientConfig := &HTTPResourceClientConfig{ @@ -217,7 +217,7 @@ func NewKumaHTTPDiscovery(conf *KumaSDConfig, logger log.Logger) (discovery.Disc return d, nil } -func defaultClientId(logger log.Logger) string { +func defaultClientID(logger log.Logger) string { clientID, err := osutil.GetFQDN() if err != nil { level.Debug(logger).Log("msg", "error getting FQDN", "err", err) diff --git a/discovery/xds/kuma_test.go b/discovery/xds/kuma_test.go index 7f2b0ce3b..6b4bb8784 100644 --- a/discovery/xds/kuma_test.go +++ b/discovery/xds/kuma_test.go @@ -204,7 +204,7 @@ func TestNewKumaHTTPDiscovery(t *testing.T) { require.True(t, ok) require.Equal(t, kumaConf.Server, resClient.Server()) require.Equal(t, KumaMadsV1ResourceTypeURL, resClient.ResourceTypeURL()) - require.Equal(t, kumaConf.ClientId, resClient.ID()) + require.Equal(t, kumaConf.ClientID, resClient.ID()) require.Equal(t, KumaMadsV1ResourceType, resClient.config.ResourceType) } diff --git a/discovery/xds/xds.go b/discovery/xds/xds.go index 47baece78..16aa3f148 100644 --- a/discovery/xds/xds.go +++ b/discovery/xds/xds.go @@ -55,7 +55,7 @@ type SDConfig struct { RefreshInterval model.Duration `yaml:"refresh_interval,omitempty"` FetchTimeout model.Duration `yaml:"fetch_timeout,omitempty"` Server string `yaml:"server,omitempty"` - ClientId string `yaml:"clientId,omitempty"` + ClientID string `yaml:"client_id,omitempty"` } // mustRegisterMessage registers the provided message type in the typeRegistry, and panics diff --git a/discovery/xds/xds_test.go b/discovery/xds/xds_test.go index 2e0f24e19..f57fff996 100644 --- a/discovery/xds/xds_test.go +++ b/discovery/xds/xds_test.go @@ -36,7 +36,7 @@ var ( sdConf = SDConfig{ Server: "http://127.0.0.1", RefreshInterval: model.Duration(10 * time.Second), - ClientId: "test-id", + ClientID: "test-id", } testFetchFailuresCount = prometheus.NewCounter( diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index d3ee459be..41f54dbd2 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -2233,7 +2233,7 @@ server: # Client id is used by Kuma Control Plane to compute Monitoring Assignment for specific Prometheus backend. # This is useful when migrating between multiple Prometheus backends, or having separate backend for each Mesh # When not specified, system hostname/fqdn will be used if available, if not `prometheus` will be used. -clientId: +client_id: # The time to wait between polling update requests. [ refresh_interval: | default = 30s ] From 48934aaef3c8e18b91d232a436c00c802c99a77a Mon Sep 17 00:00:00 2001 From: Marcin Skalski Date: Mon, 11 Dec 2023 17:04:56 +0100 Subject: [PATCH 05/23] fix go lint Signed-off-by: Marcin Skalski --- config/config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/config_test.go b/config/config_test.go index 7c061fd54..e614a4463 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -568,7 +568,7 @@ var expectedConf = &Config{ ServiceDiscoveryConfigs: discovery.Configs{ &xds.KumaSDConfig{ Server: "http://kuma-control-plane.kuma-system.svc:5676", - ClientId: "main-prometheus", + ClientID: "main-prometheus", HTTPClientConfig: config.DefaultHTTPClientConfig, RefreshInterval: model.Duration(15 * time.Second), FetchTimeout: model.Duration(2 * time.Minute), From e27232614a37edbc40f7778c659806a23a3dd61b Mon Sep 17 00:00:00 2001 From: Marcin Skalski Date: Tue, 12 Dec 2023 08:32:46 +0100 Subject: [PATCH 06/23] code review Signed-off-by: Marcin Skalski --- discovery/xds/kuma.go | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/discovery/xds/kuma.go b/discovery/xds/kuma.go index e79195bb2..567e5ab7c 100644 --- a/discovery/xds/kuma.go +++ b/discovery/xds/kuma.go @@ -178,11 +178,14 @@ func kumaMadsV1ResourceParser(resources []*anypb.Any, typeURL string) ([]model.L func NewKumaHTTPDiscovery(conf *KumaSDConfig, logger log.Logger) (discovery.Discoverer, error) { // Default to "prometheus" if hostname is unavailable. - var clientID string - if conf.ClientID == "" { - clientID = defaultClientID(logger) - } else { - clientID = conf.ClientID + clientID := conf.ClientID + if clientID == "" { + var err error + clientID, err = osutil.GetFQDN() + if err != nil { + level.Debug(logger).Log("msg", "error getting FQDN", "err", err) + clientID = "prometheus" + } } clientConfig := &HTTPResourceClientConfig{ @@ -216,12 +219,3 @@ func NewKumaHTTPDiscovery(conf *KumaSDConfig, logger log.Logger) (discovery.Disc return d, nil } - -func defaultClientID(logger log.Logger) string { - clientID, err := osutil.GetFQDN() - if err != nil { - level.Debug(logger).Log("msg", "error getting FQDN", "err", err) - clientID = "prometheus" - } - return clientID -} From 19709f75d05deb8f24c69788fa7b5a4939394734 Mon Sep 17 00:00:00 2001 From: Marcin Skalski Date: Tue, 12 Dec 2023 14:49:43 +0100 Subject: [PATCH 07/23] fix kuma_sd docs Signed-off-by: Marcin Skalski --- docs/configuration/configuration.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index 41f54dbd2..5e2f31c1c 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -2231,9 +2231,9 @@ See below for the configuration options for Kuma MonitoringAssignment discovery: server: # Client id is used by Kuma Control Plane to compute Monitoring Assignment for specific Prometheus backend. -# This is useful when migrating between multiple Prometheus backends, or having separate backend for each Mesh +# This is useful when migrating between multiple Prometheus backends, or having separate backend for each Mesh. # When not specified, system hostname/fqdn will be used if available, if not `prometheus` will be used. -client_id: +[ client_id: ] # The time to wait between polling update requests. [ refresh_interval: | default = 30s ] From 0704c7254876fefbdde85fecb8ad389106312af2 Mon Sep 17 00:00:00 2001 From: Daniel Nicholls Date: Wed, 13 Dec 2023 14:14:01 +0000 Subject: [PATCH 08/23] Dedup code handling getting network interface Signed-off-by: Daniel Nicholls --- discovery/azure/azure.go | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/discovery/azure/azure.go b/discovery/azure/azure.go index 4a85db232..2a0f8380b 100644 --- a/discovery/azure/azure.go +++ b/discovery/azure/azure.go @@ -411,29 +411,19 @@ func (d *Discovery) refresh(ctx context.Context) ([]*targetgroup.Group, error) { } else { if vm.ScaleSet == "" { networkInterface, err = client.getVMNetworkInterfaceByID(ctx, nicID) - if err != nil { - if errors.Is(err, errorNotFound) { - level.Warn(d.logger).Log("msg", "Network interface does not exist", "name", nicID, "err", err) - } else { - ch <- target{labelSet: nil, err: err} - } - // Get out of this routine because we cannot continue without a network interface. - return - } - d.addToCache(nicID, networkInterface) } else { networkInterface, err = client.getVMScaleSetVMNetworkInterfaceByID(ctx, nicID, vm.ScaleSet, vm.InstanceID) - if err != nil { - if errors.Is(err, errorNotFound) { - level.Warn(d.logger).Log("msg", "Network interface does not exist", "name", nicID, "err", err) - } else { - ch <- target{labelSet: nil, err: err} - } - // Get out of this routine because we cannot continue without a network interface. - return - } - d.addToCache(nicID, networkInterface) } + if err != nil { + if errors.Is(err, errorNotFound) { + level.Warn(d.logger).Log("msg", "Network interface does not exist", "name", nicID, "err", err) + } else { + ch <- target{labelSet: nil, err: err} + } + // Get out of this routine because we cannot continue without a network interface. + return + } + d.addToCache(nicID, networkInterface) } if networkInterface.Properties == nil { From 1a8381a5011e17328d059ab80d3963cac3e507b4 Mon Sep 17 00:00:00 2001 From: Diogo Teles Sant'Anna Date: Wed, 13 Dec 2023 16:30:06 -0300 Subject: [PATCH 09/23] Add minimal permissions to script golangci-lint.yml Signed-off-by: Diogo Teles Sant'Anna --- scripts/golangci-lint.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/scripts/golangci-lint.yml b/scripts/golangci-lint.yml index 805c59fb7..4b292229d 100644 --- a/scripts/golangci-lint.yml +++ b/scripts/golangci-lint.yml @@ -12,8 +12,14 @@ on: - ".golangci.yml" pull_request: +permissions: # added using https://github.com/step-security/secure-repo + contents: read + jobs: golangci: + permissions: + contents: read # for actions/checkout to fetch code + pull-requests: read # for golangci/golangci-lint-action to fetch pull requests name: lint runs-on: ubuntu-latest steps: From 1f69dcfa6bfb5c53dacbe33b8aca45a6b7540cc8 Mon Sep 17 00:00:00 2001 From: Filip Petkovski Date: Thu, 14 Dec 2023 11:28:15 +0100 Subject: [PATCH 10/23] Fix reusing float histograms In https://github.com/prometheus/prometheus/pull/13276 we started reusing float histogram objects to reduce allocations in PromQL. That PR introduces a bug where histogram pointers gets copied to the beginning of the histograms slice, but are still kept in the end of the slice. When a new histogram is read into the last element, it can overwrite a previous element because the pointer is the same. This commit fixes the issue by moving outdated points to the end of the slice so that we don't end up with duplicate pointers in the same buffer. In other words, the slice gets rotated so that old objects can get reused. Signed-off-by: Filip Petkovski --- promql/engine.go | 5 +++ promql/engine_test.go | 91 +++++++++++++++++++++++++++++--------- tsdb/tsdbutil/histogram.go | 8 ++++ 3 files changed, 82 insertions(+), 22 deletions(-) diff --git a/promql/engine.go b/promql/engine.go index 16b8ee500..12755663d 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -2052,7 +2052,12 @@ func (ev *evaluator) matrixIterSlice( var drop int for drop = 0; histograms[drop].T < mint; drop++ { } + // Rotate the buffer around the drop index so that points before mint can be + // reused to store new histograms. + tail := make([]HPoint, drop) + copy(tail, histograms[:drop]) copy(histograms, histograms[drop:]) + copy(histograms[len(histograms)-drop:], tail) histograms = histograms[:len(histograms)-drop] ev.currentSamples -= totalHPointSize(histograms) // Only append points with timestamps after the last timestamp we have. diff --git a/promql/engine_test.go b/promql/engine_test.go index 9ab54dd16..105cdc10d 100644 --- a/promql/engine_test.go +++ b/promql/engine_test.go @@ -3169,28 +3169,75 @@ func TestNativeHistogramRate(t *testing.T) { } require.NoError(t, app.Commit()) - queryString := fmt.Sprintf("rate(%s[1m])", seriesName) - qry, err := engine.NewInstantQuery(context.Background(), storage, nil, queryString, timestamp.Time(int64(5*time.Minute/time.Millisecond))) - require.NoError(t, err) - res := qry.Exec(context.Background()) - require.NoError(t, res.Err) - vector, err := res.Vector() - require.NoError(t, err) - require.Len(t, vector, 1) - actualHistogram := vector[0].H - expectedHistogram := &histogram.FloatHistogram{ - CounterResetHint: histogram.GaugeType, - Schema: 1, - ZeroThreshold: 0.001, - ZeroCount: 1. / 15., - Count: 9. / 15., - Sum: 1.226666666666667, - PositiveSpans: []histogram.Span{{Offset: 0, Length: 2}, {Offset: 1, Length: 2}}, - PositiveBuckets: []float64{1. / 15., 1. / 15., 1. / 15., 1. / 15.}, - NegativeSpans: []histogram.Span{{Offset: 0, Length: 2}, {Offset: 1, Length: 2}}, - NegativeBuckets: []float64{1. / 15., 1. / 15., 1. / 15., 1. / 15.}, - } - require.Equal(t, expectedHistogram, actualHistogram) + queryString := fmt.Sprintf("rate(%s[45s])", seriesName) + t.Run("instant_query", func(t *testing.T) { + qry, err := engine.NewInstantQuery(context.Background(), storage, nil, queryString, timestamp.Time(int64(5*time.Minute/time.Millisecond))) + require.NoError(t, err) + res := qry.Exec(context.Background()) + require.NoError(t, res.Err) + vector, err := res.Vector() + require.NoError(t, err) + require.Len(t, vector, 1) + actualHistogram := vector[0].H + expectedHistogram := &histogram.FloatHistogram{ + CounterResetHint: histogram.GaugeType, + Schema: 1, + ZeroThreshold: 0.001, + ZeroCount: 1. / 15., + Count: 9. / 15., + Sum: 1.2266666666666663, + PositiveSpans: []histogram.Span{{Offset: 0, Length: 2}, {Offset: 1, Length: 2}}, + PositiveBuckets: []float64{1. / 15., 1. / 15., 1. / 15., 1. / 15.}, + NegativeSpans: []histogram.Span{{Offset: 0, Length: 2}, {Offset: 1, Length: 2}}, + NegativeBuckets: []float64{1. / 15., 1. / 15., 1. / 15., 1. / 15.}, + } + require.Equal(t, expectedHistogram, actualHistogram) + }) + + t.Run("range_query", func(t *testing.T) { + step := 30 * time.Second + start := timestamp.Time(int64(5 * time.Minute / time.Millisecond)) + end := start.Add(step) + qry, err := engine.NewRangeQuery(context.Background(), storage, nil, queryString, start, end, step) + require.NoError(t, err) + res := qry.Exec(context.Background()) + require.NoError(t, res.Err) + matrix, err := res.Matrix() + require.NoError(t, err) + require.Len(t, matrix, 1) + require.Len(t, matrix[0].Histograms, 2) + actualHistograms := matrix[0].Histograms + expectedHistograms := []HPoint{{ + T: 300000, + H: &histogram.FloatHistogram{ + CounterResetHint: histogram.GaugeType, + Schema: 1, + ZeroThreshold: 0.001, + ZeroCount: 1. / 15., + Count: 9. / 15., + Sum: 1.2266666666666663, + PositiveSpans: []histogram.Span{{Offset: 0, Length: 2}, {Offset: 1, Length: 2}}, + PositiveBuckets: []float64{1. / 15., 1. / 15., 1. / 15., 1. / 15.}, + NegativeSpans: []histogram.Span{{Offset: 0, Length: 2}, {Offset: 1, Length: 2}}, + NegativeBuckets: []float64{1. / 15., 1. / 15., 1. / 15., 1. / 15.}, + }, + }, { + T: 330000, + H: &histogram.FloatHistogram{ + CounterResetHint: histogram.GaugeType, + Schema: 1, + ZeroThreshold: 0.001, + ZeroCount: 1. / 15., + Count: 9. / 15., + Sum: 1.2266666666666663, + PositiveSpans: []histogram.Span{{Offset: 0, Length: 2}, {Offset: 1, Length: 2}}, + PositiveBuckets: []float64{1. / 15., 1. / 15., 1. / 15., 1. / 15.}, + NegativeSpans: []histogram.Span{{Offset: 0, Length: 2}, {Offset: 1, Length: 2}}, + NegativeBuckets: []float64{1. / 15., 1. / 15., 1. / 15., 1. / 15.}, + }, + }} + require.Equal(t, expectedHistograms, actualHistograms) + }) } func TestNativeFloatHistogramRate(t *testing.T) { diff --git a/tsdb/tsdbutil/histogram.go b/tsdb/tsdbutil/histogram.go index 0847f81a8..bb8d49b20 100644 --- a/tsdb/tsdbutil/histogram.go +++ b/tsdb/tsdbutil/histogram.go @@ -30,6 +30,14 @@ func GenerateTestHistograms(n int) (r []*histogram.Histogram) { return r } +func GenerateTestHistogramsWithUnknownResetHint(n int) []*histogram.Histogram { + hs := GenerateTestHistograms(n) + for i := range hs { + hs[i].CounterResetHint = histogram.UnknownCounterReset + } + return hs +} + // GenerateTestHistogram but it is up to the user to set any known counter reset hint. func GenerateTestHistogram(i int) *histogram.Histogram { return &histogram.Histogram{ From 952cb41373c00a46c060ade00e8ecc85603ef758 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Fri, 15 Dec 2023 10:21:18 +0000 Subject: [PATCH 11/23] build(deps): bump github.com/Azure/azure-sdk-for-go/sdk/resourcemanager Signed-off-by: Matthieu MOREL --- discovery/azure/azure.go | 4 ++-- discovery/azure/azure_test.go | 2 +- go.mod | 7 +++---- go.sum | 14 ++++++-------- 4 files changed, 12 insertions(+), 15 deletions(-) diff --git a/discovery/azure/azure.go b/discovery/azure/azure.go index 4a85db232..6637e9800 100644 --- a/discovery/azure/azure.go +++ b/discovery/azure/azure.go @@ -30,8 +30,8 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" - "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v4" - "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v2" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v4" cache "github.com/Code-Hex/go-generics-cache" "github.com/Code-Hex/go-generics-cache/policy/lru" "github.com/go-kit/log" diff --git a/discovery/azure/azure_test.go b/discovery/azure/azure_test.go index 024cf7591..4ff937e0b 100644 --- a/discovery/azure/azure_test.go +++ b/discovery/azure/azure_test.go @@ -17,7 +17,7 @@ import ( "testing" "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" - "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v4" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5" "github.com/stretchr/testify/require" "go.uber.org/goleak" ) diff --git a/go.mod b/go.mod index 770607e9c..9ea078083 100644 --- a/go.mod +++ b/go.mod @@ -5,8 +5,9 @@ go 1.20 require ( github.com/Azure/azure-sdk-for-go/sdk/azcore v1.9.0 github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.4.0 - github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v4 v4.2.1 - github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v2 v2.2.1 + github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5 v5.3.0 + github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v4 v4.3.0 + github.com/Code-Hex/go-generics-cache v1.3.1 github.com/alecthomas/kingpin/v2 v2.4.0 github.com/alecthomas/units v0.0.0-20231202071711-9a357b53e9c9 github.com/aws/aws-sdk-go v1.48.14 @@ -93,9 +94,7 @@ require ( cloud.google.com/go/compute v1.23.3 // indirect cloud.google.com/go/compute/metadata v0.2.3 // indirect github.com/Azure/azure-sdk-for-go/sdk/internal v1.5.0 // indirect - github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork v1.1.0 // indirect github.com/AzureAD/microsoft-authentication-library-for-go v1.1.1 // indirect - github.com/Code-Hex/go-generics-cache v1.3.1 github.com/Microsoft/go-winio v0.6.1 // indirect github.com/armon/go-metrics v0.4.1 // indirect github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 // indirect diff --git a/go.sum b/go.sum index ae367bb90..a6f535dd7 100644 --- a/go.sum +++ b/go.sum @@ -40,14 +40,12 @@ github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.4.0 h1:BMAjVKJM0U/CYF27gA0ZM github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.4.0/go.mod h1:1fXstnBMas5kzG+S3q8UoJcmyU6nUeunJcMDHcRYHhs= github.com/Azure/azure-sdk-for-go/sdk/internal v1.5.0 h1:d81/ng9rET2YqdVkVwkb6EXeRrLJIwyGnJcAlAWKwhs= github.com/Azure/azure-sdk-for-go/sdk/internal v1.5.0/go.mod h1:s4kgfzA0covAXNicZHDMN58jExvcng2mC/DepXiF1EI= -github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v4 v4.2.1 h1:UPeCRD+XY7QlaGQte2EVI2iOcWvUYA2XY8w5T/8v0NQ= -github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v4 v4.2.1/go.mod h1:oGV6NlB0cvi1ZbYRR2UN44QHxWFyGk+iylgD0qaMXjA= -github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/internal v1.1.2 h1:mLY+pNLjCUeKhgnAJWAKhEUQM+RJQo2H1fuGSw1Ky1E= -github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork v1.1.0 h1:QM6sE5k2ZT/vI5BEe0r7mqjsUSnhVBFbOsVkEuaEfiA= -github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork v1.1.0/go.mod h1:243D9iHbcQXoFUtgHJwL7gl2zx1aDuDMjvBZVGr2uW0= -github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v2 v2.2.1 h1:bWh0Z2rOEDfB/ywv/l0iHN1JgyazE6kW/aIA89+CEK0= -github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v2 v2.2.1/go.mod h1:Bzf34hhAE9NSxailk8xVeLEZbUjOXcC+GnU1mMKdhLw= -github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources v1.0.0 h1:ECsQtyERDVz3NP3kvDOTLvbQhqWp/x9EsGKtb4ogUr8= +github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5 v5.3.0 h1:qgs/VAMSR+9qFhwTw4OwF2NbVuw+2m83pVZJjqkKQMw= +github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5 v5.3.0/go.mod h1:uYt4CfhkJA9o0FN7jfE5minm/i4nUE4MjGUJkzB6Zs8= +github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/internal/v2 v2.0.0 h1:PTFGRSlMKCQelWwxUyYVEUqseBJVemLyqWJjvMyt0do= +github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v4 v4.3.0 h1:bXwSugBiSbgtz7rOtbfGf+woewp4f06orW9OP5BjHLA= +github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v4 v4.3.0/go.mod h1:Y/HgrePTmGy9HjdSGTqZNa+apUpTVIEVKXJyARP2lrk= +github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources v1.1.1 h1:7CBQ+Ei8SP2c6ydQTGCCrS35bDxgTMfoP2miAwK++OU= github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 h1:UQHMgLO+TxOElx5B5HZ4hJQsoJ/PvUvKRhJHDQXO8P8= github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E= github.com/AzureAD/microsoft-authentication-library-for-go v1.1.1 h1:WpB/QDNLpMw72xHJc34BNNykqSOeEJDAWkhf0u12/Jk= From 9ab7e3b3dec4b82870487154dce53baa3266a614 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 18 Dec 2023 14:52:42 +0000 Subject: [PATCH 12/23] relabel: refactor: extract config.Validate method And add a test for it, which fails because validation is not strong enough. Signed-off-by: Bryan Boreham --- model/relabel/relabel.go | 4 ++ model/relabel/relabel_test.go | 72 +++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/model/relabel/relabel.go b/model/relabel/relabel.go index fadf35b86..fa0d809de 100644 --- a/model/relabel/relabel.go +++ b/model/relabel/relabel.go @@ -108,6 +108,10 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { if c.Regex.Regexp == nil { c.Regex = MustNewRegexp("") } + return c.Validate() +} + +func (c *Config) Validate() error { if c.Action == "" { return fmt.Errorf("relabel action cannot be empty") } diff --git a/model/relabel/relabel_test.go b/model/relabel/relabel_test.go index b50ff4010..fe040be3a 100644 --- a/model/relabel/relabel_test.go +++ b/model/relabel/relabel_test.go @@ -14,6 +14,7 @@ package relabel import ( + "fmt" "testing" "github.com/prometheus/common/model" @@ -575,6 +576,77 @@ func TestRelabel(t *testing.T) { } } +func TestRelabelValidate(t *testing.T) { + tests := []struct { + config Config + expected string + }{ + { + config: Config{}, + expected: `relabel action cannot be empty`, + }, + { + config: Config{ + Action: Replace, + }, + expected: `requires 'target_label' value`, + }, + { + config: Config{ + Action: Lowercase, + }, + expected: `requires 'target_label' value`, + }, + { + config: Config{ + Action: Lowercase, + Replacement: DefaultRelabelConfig.Replacement, + TargetLabel: "${3}", + }, + expected: `"${3}" is invalid 'target_label'`, + }, + { + config: Config{ + SourceLabels: model.LabelNames{"a"}, + Regex: MustNewRegexp("some-([^-]+)-([^,]+)"), + Action: Replace, + Replacement: "${1}", + TargetLabel: "${3}", + }, + }, + { + config: Config{ + SourceLabels: model.LabelNames{"a"}, + Regex: MustNewRegexp("some-([^-]+)-([^,]+)"), + Action: Replace, + Replacement: "${1}", + TargetLabel: "0${3}", + }, + expected: `"0${3}" is invalid 'target_label'`, + }, + { + config: Config{ + SourceLabels: model.LabelNames{"a"}, + Regex: MustNewRegexp("some-([^-]+)-([^,]+)"), + Action: Replace, + Replacement: "${1}", + TargetLabel: "-${3}", + }, + expected: `"-${3}" is invalid 'target_label' for replace action`, + }, + } + for i, test := range tests { + t.Run(fmt.Sprint(i), func(t *testing.T) { + err := test.config.Validate() + if test.expected == "" { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, test.expected) + } + }) + } +} + func TestTargetLabelValidity(t *testing.T) { tests := []struct { str string From 2d4c367d87cdd99eac1b4786cf1621c58d43735d Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 18 Dec 2023 14:58:56 +0000 Subject: [PATCH 13/23] relabel: stricter check that target labels are valid For `Lowercase`, `KeepEqual`, etc., we do not expand a regexp, so the target label name must not contain anything like `${1}`. Also for the common case that the `Replace` target does not require any template expansion, check that the entire string passes label name validity rules. Signed-off-by: Bryan Boreham --- model/relabel/relabel.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/model/relabel/relabel.go b/model/relabel/relabel.go index fa0d809de..1947e6273 100644 --- a/model/relabel/relabel.go +++ b/model/relabel/relabel.go @@ -121,7 +121,13 @@ func (c *Config) Validate() error { if (c.Action == Replace || c.Action == HashMod || c.Action == Lowercase || c.Action == Uppercase || c.Action == KeepEqual || c.Action == DropEqual) && c.TargetLabel == "" { return fmt.Errorf("relabel configuration for %s action requires 'target_label' value", c.Action) } - if (c.Action == Replace || c.Action == Lowercase || c.Action == Uppercase || c.Action == KeepEqual || c.Action == DropEqual) && !relabelTarget.MatchString(c.TargetLabel) { + if c.Action == Replace && !strings.Contains(c.TargetLabel, "$") && !model.LabelName(c.TargetLabel).IsValid() { + return fmt.Errorf("%q is invalid 'target_label' for %s action", c.TargetLabel, c.Action) + } + if c.Action == Replace && strings.Contains(c.TargetLabel, "$") && !relabelTarget.MatchString(c.TargetLabel) { + return fmt.Errorf("%q is invalid 'target_label' for %s action", c.TargetLabel, c.Action) + } + if (c.Action == Lowercase || c.Action == Uppercase || c.Action == KeepEqual || c.Action == DropEqual) && !model.LabelName(c.TargetLabel).IsValid() { return fmt.Errorf("%q is invalid 'target_label' for %s action", c.TargetLabel, c.Action) } if (c.Action == Lowercase || c.Action == Uppercase || c.Action == KeepEqual || c.Action == DropEqual) && c.Replacement != DefaultRelabelConfig.Replacement { From 000182e4b8638bfe781a583c27a1e8a366ac82bf Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 18 Dec 2023 15:03:21 +0000 Subject: [PATCH 14/23] relabel: check validity of all test cases Thought this would be a nice check on the `Validate()` function, but some of the test cases needed tweaking to pass. Signed-off-by: Bryan Boreham --- model/relabel/relabel_test.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/model/relabel/relabel_test.go b/model/relabel/relabel_test.go index fe040be3a..2b8fc911d 100644 --- a/model/relabel/relabel_test.go +++ b/model/relabel/relabel_test.go @@ -335,7 +335,7 @@ func TestRelabel(t *testing.T) { }, { // invalid target_labels input: labels.FromMap(map[string]string{ - "a": "some-name-value", + "a": "some-name-0", }), relabel: []*Config{ { @@ -350,18 +350,18 @@ func TestRelabel(t *testing.T) { Regex: MustNewRegexp("some-([^-]+)-([^,]+)"), Action: Replace, Replacement: "${1}", - TargetLabel: "0${3}", + TargetLabel: "${3}", }, { SourceLabels: model.LabelNames{"a"}, - Regex: MustNewRegexp("some-([^-]+)-([^,]+)"), + Regex: MustNewRegexp("some-([^-]+)(-[^,]+)"), Action: Replace, Replacement: "${1}", - TargetLabel: "-${3}", + TargetLabel: "${3}", }, }, output: labels.FromMap(map[string]string{ - "a": "some-name-value", + "a": "some-name-0", }), }, { // more complex real-life like usecase @@ -566,6 +566,7 @@ func TestRelabel(t *testing.T) { if cfg.Replacement == "" { cfg.Replacement = DefaultRelabelConfig.Replacement } + require.NoError(t, cfg.Validate()) } res, keep := Process(test.input, test.relabel...) From 0289dd61571c7a812a235ecec5bc3f74fd7ccf50 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 18 Dec 2023 16:38:59 +0000 Subject: [PATCH 15/23] relabel: blank replacement deletes label post-regexp If `cfg.TargetLabel` is a template like `$1`, it won't match any labels, so no point calling `lb.Del` with it. Similarly if `target` is not a valid label name, it won't match any labels, so don't call with that either. The intention seems to have been that a blank _value_ would delete the target, so change that code to use `target` instead of `cfg.TargetLabel`. Signed-off-by: Bryan Boreham --- model/relabel/relabel.go | 3 +-- model/relabel/relabel_test.go | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/model/relabel/relabel.go b/model/relabel/relabel.go index 1947e6273..d29c3d07a 100644 --- a/model/relabel/relabel.go +++ b/model/relabel/relabel.go @@ -274,12 +274,11 @@ func relabel(cfg *Config, lb *labels.Builder) (keep bool) { } target := model.LabelName(cfg.Regex.ExpandString([]byte{}, cfg.TargetLabel, val, indexes)) if !target.IsValid() { - lb.Del(cfg.TargetLabel) break } res := cfg.Regex.ExpandString([]byte{}, cfg.Replacement, val, indexes) if len(res) == 0 { - lb.Del(cfg.TargetLabel) + lb.Del(string(target)) break } lb.Set(string(target), string(res)) diff --git a/model/relabel/relabel_test.go b/model/relabel/relabel_test.go index 2b8fc911d..517b9b822 100644 --- a/model/relabel/relabel_test.go +++ b/model/relabel/relabel_test.go @@ -214,6 +214,25 @@ func TestRelabel(t *testing.T) { "a": "boo", }), }, + { + // Blank replacement should delete the label. + input: labels.FromMap(map[string]string{ + "a": "foo", + "f": "baz", + }), + relabel: []*Config{ + { + SourceLabels: model.LabelNames{"a"}, + Regex: MustNewRegexp("(f).*"), + TargetLabel: "$1", + Replacement: "$2", + Action: Replace, + }, + }, + output: labels.FromMap(map[string]string{ + "a": "foo", + }), + }, { input: labels.FromMap(map[string]string{ "a": "foo", From 8065bef172e8d88e22399504b175a8c9115e9da3 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 22 Nov 2023 14:39:21 +0000 Subject: [PATCH 16/23] Move metric type definitions to common/model They are used in multiple repos, so common is a better place for them. Several packages now don't depend on `model/textparse`, e.g. `storage/remote`. Also remove `metadata` struct from `api.go`, since it was identical to a struct in the `metadata` package. Signed-off-by: Bryan Boreham --- go.mod | 2 +- go.sum | 4 +- model/metadata/metadata.go | 4 +- model/textparse/interface.go | 16 +--- model/textparse/openmetricsparse.go | 20 +++-- model/textparse/openmetricsparse_test.go | 25 +++--- model/textparse/promparse.go | 12 +-- model/textparse/promparse_test.go | 6 +- model/textparse/protobufparse.go | 14 +-- model/textparse/protobufparse_test.go | 69 +++++++-------- scrape/scrape.go | 6 +- scrape/scrape_test.go | 6 +- storage/remote/codec.go | 3 +- storage/remote/codec_test.go | 8 +- storage/remote/metadata_watcher_test.go | 7 +- storage/remote/queue_manager_test.go | 3 +- tsdb/head_append.go | 2 +- tsdb/head_wal.go | 2 +- tsdb/record/record.go | 37 ++++---- web/api/v1/api.go | 32 +++---- web/api/v1/api_test.go | 104 +++++++++++------------ 21 files changed, 185 insertions(+), 197 deletions(-) diff --git a/go.mod b/go.mod index 9ea078083..0ecfd4374 100644 --- a/go.mod +++ b/go.mod @@ -48,7 +48,7 @@ require ( github.com/prometheus/alertmanager v0.26.0 github.com/prometheus/client_golang v1.17.0 github.com/prometheus/client_model v0.5.0 - github.com/prometheus/common v0.45.0 + github.com/prometheus/common v0.45.1-0.20231122191551-832cd6e99f99 github.com/prometheus/common/assets v0.2.0 github.com/prometheus/common/sigv4 v0.1.0 github.com/prometheus/exporter-toolkit v0.10.0 diff --git a/go.sum b/go.sum index a6f535dd7..fb31c7a84 100644 --- a/go.sum +++ b/go.sum @@ -653,8 +653,8 @@ github.com/prometheus/common v0.9.1/go.mod h1:yhUN8i9wzaXS3w1O07YhxHEBxD+W35wd8b github.com/prometheus/common v0.10.0/go.mod h1:Tlit/dnDKsSWFlCLTWaA1cyBgKHSMdTB80sz/V91rCo= github.com/prometheus/common v0.26.0/go.mod h1:M7rCNAaPfAosfx8veZJCuw84e35h3Cfd9VFqTh1DIvc= github.com/prometheus/common v0.29.0/go.mod h1:vu+V0TpY+O6vW9J44gczi3Ap/oXXR10b+M/gUGO4Hls= -github.com/prometheus/common v0.45.0 h1:2BGz0eBc2hdMDLnO/8n0jeB3oPrt2D08CekT0lneoxM= -github.com/prometheus/common v0.45.0/go.mod h1:YJmSTw9BoKxJplESWWxlbyttQR4uaEcGyv9MZjVOJsY= +github.com/prometheus/common v0.45.1-0.20231122191551-832cd6e99f99 h1:V5ajRiLiCQGO+ggTr+07gMUcTqlIMMkDBfrJe5zKLmc= +github.com/prometheus/common v0.45.1-0.20231122191551-832cd6e99f99/go.mod h1:YJmSTw9BoKxJplESWWxlbyttQR4uaEcGyv9MZjVOJsY= github.com/prometheus/common/assets v0.2.0 h1:0P5OrzoHrYBOSM1OigWL3mY8ZvV2N4zIE/5AahrSrfM= github.com/prometheus/common/assets v0.2.0/go.mod h1:D17UVUE12bHbim7HzwUvtqm6gwBEaDQ0F+hIGbFbccI= github.com/prometheus/common/sigv4 v0.1.0 h1:qoVebwtwwEhS85Czm2dSROY5fTo2PAPEVdDeppTwGX4= diff --git a/model/metadata/metadata.go b/model/metadata/metadata.go index f227af0b9..f6f2827a4 100644 --- a/model/metadata/metadata.go +++ b/model/metadata/metadata.go @@ -13,11 +13,11 @@ package metadata -import "github.com/prometheus/prometheus/model/textparse" +import "github.com/prometheus/common/model" // Metadata stores a series' metadata information. type Metadata struct { - Type textparse.MetricType + Type model.MetricType Unit string Help string } diff --git a/model/textparse/interface.go b/model/textparse/interface.go index df4259c85..f6d93f063 100644 --- a/model/textparse/interface.go +++ b/model/textparse/interface.go @@ -16,6 +16,8 @@ package textparse import ( "mime" + "github.com/prometheus/common/model" + "github.com/prometheus/prometheus/model/exemplar" "github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/model/labels" @@ -110,16 +112,4 @@ const ( EntryHistogram Entry = 5 // A series with a native histogram as a value. ) -// MetricType represents metric type values. -type MetricType string - -const ( - MetricTypeCounter = MetricType("counter") - MetricTypeGauge = MetricType("gauge") - MetricTypeHistogram = MetricType("histogram") - MetricTypeGaugeHistogram = MetricType("gaugehistogram") - MetricTypeSummary = MetricType("summary") - MetricTypeInfo = MetricType("info") - MetricTypeStateset = MetricType("stateset") - MetricTypeUnknown = MetricType("unknown") -) +type MetricType = model.MetricType diff --git a/model/textparse/openmetricsparse.go b/model/textparse/openmetricsparse.go index f0c383723..d6b01eb8e 100644 --- a/model/textparse/openmetricsparse.go +++ b/model/textparse/openmetricsparse.go @@ -24,6 +24,8 @@ import ( "strings" "unicode/utf8" + "github.com/prometheus/common/model" + "github.com/prometheus/prometheus/model/exemplar" "github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/model/labels" @@ -74,7 +76,7 @@ type OpenMetricsParser struct { builder labels.ScratchBuilder series []byte text []byte - mtype MetricType + mtype model.MetricType val float64 ts int64 hasTS bool @@ -272,21 +274,21 @@ func (p *OpenMetricsParser) Next() (Entry, error) { case tType: switch s := yoloString(p.text); s { case "counter": - p.mtype = MetricTypeCounter + p.mtype = model.MetricTypeCounter case "gauge": - p.mtype = MetricTypeGauge + p.mtype = model.MetricTypeGauge case "histogram": - p.mtype = MetricTypeHistogram + p.mtype = model.MetricTypeHistogram case "gaugehistogram": - p.mtype = MetricTypeGaugeHistogram + p.mtype = model.MetricTypeGaugeHistogram case "summary": - p.mtype = MetricTypeSummary + p.mtype = model.MetricTypeSummary case "info": - p.mtype = MetricTypeInfo + p.mtype = model.MetricTypeInfo case "stateset": - p.mtype = MetricTypeStateset + p.mtype = model.MetricTypeStateset case "unknown": - p.mtype = MetricTypeUnknown + p.mtype = model.MetricTypeUnknown default: return EntryInvalid, fmt.Errorf("invalid metric type %q", s) } diff --git a/model/textparse/openmetricsparse_test.go b/model/textparse/openmetricsparse_test.go index eed30364c..2b1d909f3 100644 --- a/model/textparse/openmetricsparse_test.go +++ b/model/textparse/openmetricsparse_test.go @@ -18,6 +18,7 @@ import ( "io" "testing" + "github.com/prometheus/common/model" "github.com/stretchr/testify/require" "github.com/prometheus/prometheus/model/exemplar" @@ -77,7 +78,7 @@ foo_total 17.0 1520879607.789 # {id="counter-test"} 5` m string t *int64 v float64 - typ MetricType + typ model.MetricType help string unit string comment string @@ -88,7 +89,7 @@ foo_total 17.0 1520879607.789 # {id="counter-test"} 5` help: "A summary of the GC invocation durations.", }, { m: "go_gc_duration_seconds", - typ: MetricTypeSummary, + typ: model.MetricTypeSummary, }, { m: "go_gc_duration_seconds", unit: "seconds", @@ -130,7 +131,7 @@ foo_total 17.0 1520879607.789 # {id="counter-test"} 5` help: "Number of goroutines that currently exist.", }, { m: "go_goroutines", - typ: MetricTypeGauge, + typ: model.MetricTypeGauge, }, { m: `go_goroutines`, v: 33, @@ -138,21 +139,21 @@ foo_total 17.0 1520879607.789 # {id="counter-test"} 5` lset: labels.FromStrings("__name__", "go_goroutines"), }, { m: "hh", - typ: MetricTypeHistogram, + typ: model.MetricTypeHistogram, }, { m: `hh_bucket{le="+Inf"}`, v: 1, lset: labels.FromStrings("__name__", "hh_bucket", "le", "+Inf"), }, { m: "gh", - typ: MetricTypeGaugeHistogram, + typ: model.MetricTypeGaugeHistogram, }, { m: `gh_bucket{le="+Inf"}`, v: 1, lset: labels.FromStrings("__name__", "gh_bucket", "le", "+Inf"), }, { m: "hhh", - typ: MetricTypeHistogram, + typ: model.MetricTypeHistogram, }, { m: `hhh_bucket{le="+Inf"}`, v: 1, @@ -165,7 +166,7 @@ foo_total 17.0 1520879607.789 # {id="counter-test"} 5` e: &exemplar.Exemplar{Labels: labels.FromStrings("id", "histogram-count-test"), Value: 4}, }, { m: "ggh", - typ: MetricTypeGaugeHistogram, + typ: model.MetricTypeGaugeHistogram, }, { m: `ggh_bucket{le="+Inf"}`, v: 1, @@ -178,7 +179,7 @@ foo_total 17.0 1520879607.789 # {id="counter-test"} 5` e: &exemplar.Exemplar{Labels: labels.FromStrings("id", "gaugehistogram-count-test", "xx", "yy"), Value: 4, HasTs: true, Ts: 123123}, }, { m: "smr_seconds", - typ: MetricTypeSummary, + typ: model.MetricTypeSummary, }, { m: `smr_seconds_count`, v: 2, @@ -191,14 +192,14 @@ foo_total 17.0 1520879607.789 # {id="counter-test"} 5` e: &exemplar.Exemplar{Labels: labels.FromStrings("id", "summary-sum-test"), Value: 1, HasTs: true, Ts: 123321}, }, { m: "ii", - typ: MetricTypeInfo, + typ: model.MetricTypeInfo, }, { m: `ii{foo="bar"}`, v: 1, lset: labels.FromStrings("__name__", "ii", "foo", "bar"), }, { m: "ss", - typ: MetricTypeStateset, + typ: model.MetricTypeStateset, }, { m: `ss{ss="foo"}`, v: 1, @@ -213,7 +214,7 @@ foo_total 17.0 1520879607.789 # {id="counter-test"} 5` lset: labels.FromStrings("A", "a", "__name__", "ss"), }, { m: "un", - typ: MetricTypeUnknown, + typ: model.MetricTypeUnknown, }, { m: "_metric_starting_with_underscore", v: 1, @@ -228,7 +229,7 @@ foo_total 17.0 1520879607.789 # {id="counter-test"} 5` lset: labels.FromStrings("__name__", "testmetric", "label", `"bar"`), }, { m: "foo", - typ: MetricTypeCounter, + typ: model.MetricTypeCounter, }, { m: "foo_total", v: 17, diff --git a/model/textparse/promparse.go b/model/textparse/promparse.go index 935801fb9..cd028ef90 100644 --- a/model/textparse/promparse.go +++ b/model/textparse/promparse.go @@ -26,6 +26,8 @@ import ( "unicode/utf8" "unsafe" + "github.com/prometheus/common/model" + "github.com/prometheus/prometheus/model/exemplar" "github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/model/labels" @@ -305,15 +307,15 @@ func (p *PromParser) Next() (Entry, error) { case tType: switch s := yoloString(p.text); s { case "counter": - p.mtype = MetricTypeCounter + p.mtype = model.MetricTypeCounter case "gauge": - p.mtype = MetricTypeGauge + p.mtype = model.MetricTypeGauge case "histogram": - p.mtype = MetricTypeHistogram + p.mtype = model.MetricTypeHistogram case "summary": - p.mtype = MetricTypeSummary + p.mtype = model.MetricTypeSummary case "untyped": - p.mtype = MetricTypeUnknown + p.mtype = model.MetricTypeUnknown default: return EntryInvalid, fmt.Errorf("invalid metric type %q", s) } diff --git a/model/textparse/promparse_test.go b/model/textparse/promparse_test.go index ac79a1394..d34b26ba5 100644 --- a/model/textparse/promparse_test.go +++ b/model/textparse/promparse_test.go @@ -65,7 +65,7 @@ testmetric{label="\"bar\""} 1` m string t *int64 v float64 - typ MetricType + typ model.MetricType help string comment string }{ @@ -74,7 +74,7 @@ testmetric{label="\"bar\""} 1` help: "A summary of the GC invocation durations.", }, { m: "go_gc_duration_seconds", - typ: MetricTypeSummary, + typ: model.MetricTypeSummary, }, { m: `go_gc_duration_seconds{quantile="0"}`, v: 4.9351e-05, @@ -142,7 +142,7 @@ testmetric{label="\"bar\""} 1` help: "Number of goroutines that currently exist.", }, { m: "go_goroutines", - typ: MetricTypeGauge, + typ: model.MetricTypeGauge, }, { m: `go_goroutines`, v: 33, diff --git a/model/textparse/protobufparse.go b/model/textparse/protobufparse.go index baede7e1d..534bbebb2 100644 --- a/model/textparse/protobufparse.go +++ b/model/textparse/protobufparse.go @@ -252,21 +252,21 @@ func (p *ProtobufParser) Help() ([]byte, []byte) { // Type returns the metric name and type in the current entry. // Must only be called after Next returned a type entry. // The returned byte slices become invalid after the next call to Next. -func (p *ProtobufParser) Type() ([]byte, MetricType) { +func (p *ProtobufParser) Type() ([]byte, model.MetricType) { n := p.metricBytes.Bytes() switch p.mf.GetType() { case dto.MetricType_COUNTER: - return n, MetricTypeCounter + return n, model.MetricTypeCounter case dto.MetricType_GAUGE: - return n, MetricTypeGauge + return n, model.MetricTypeGauge case dto.MetricType_HISTOGRAM: - return n, MetricTypeHistogram + return n, model.MetricTypeHistogram case dto.MetricType_GAUGE_HISTOGRAM: - return n, MetricTypeGaugeHistogram + return n, model.MetricTypeGaugeHistogram case dto.MetricType_SUMMARY: - return n, MetricTypeSummary + return n, model.MetricTypeSummary } - return n, MetricTypeUnknown + return n, model.MetricTypeUnknown } // Unit always returns (nil, nil) because units aren't supported by the protobuf diff --git a/model/textparse/protobufparse_test.go b/model/textparse/protobufparse_test.go index c5b672dbc..f994ff966 100644 --- a/model/textparse/protobufparse_test.go +++ b/model/textparse/protobufparse_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/gogo/protobuf/proto" + "github.com/prometheus/common/model" "github.com/stretchr/testify/require" "github.com/prometheus/prometheus/model/exemplar" @@ -649,7 +650,7 @@ func TestProtobufParse(t *testing.T) { }, { m: "go_build_info", - typ: MetricTypeGauge, + typ: model.MetricTypeGauge, }, { m: "go_build_info\xFFchecksum\xFF\xFFpath\xFFgithub.com/prometheus/client_golang\xFFversion\xFF(devel)", @@ -667,7 +668,7 @@ func TestProtobufParse(t *testing.T) { }, { m: "go_memstats_alloc_bytes_total", - typ: MetricTypeCounter, + typ: model.MetricTypeCounter, }, { m: "go_memstats_alloc_bytes_total", @@ -685,7 +686,7 @@ func TestProtobufParse(t *testing.T) { }, { m: "something_untyped", - typ: MetricTypeUnknown, + typ: model.MetricTypeUnknown, }, { m: "something_untyped", @@ -701,7 +702,7 @@ func TestProtobufParse(t *testing.T) { }, { m: "test_histogram", - typ: MetricTypeHistogram, + typ: model.MetricTypeHistogram, }, { m: "test_histogram", @@ -736,7 +737,7 @@ func TestProtobufParse(t *testing.T) { }, { m: "test_gauge_histogram", - typ: MetricTypeGaugeHistogram, + typ: model.MetricTypeGaugeHistogram, }, { m: "test_gauge_histogram", @@ -772,7 +773,7 @@ func TestProtobufParse(t *testing.T) { }, { m: "test_float_histogram", - typ: MetricTypeHistogram, + typ: model.MetricTypeHistogram, }, { m: "test_float_histogram", @@ -807,7 +808,7 @@ func TestProtobufParse(t *testing.T) { }, { m: "test_gauge_float_histogram", - typ: MetricTypeGaugeHistogram, + typ: model.MetricTypeGaugeHistogram, }, { m: "test_gauge_float_histogram", @@ -843,7 +844,7 @@ func TestProtobufParse(t *testing.T) { }, { m: "test_histogram2", - typ: MetricTypeHistogram, + typ: model.MetricTypeHistogram, }, { m: "test_histogram2_count", @@ -903,7 +904,7 @@ func TestProtobufParse(t *testing.T) { }, { m: "test_histogram_family", - typ: MetricTypeHistogram, + typ: model.MetricTypeHistogram, }, { m: "test_histogram_family\xfffoo\xffbar", @@ -947,7 +948,7 @@ func TestProtobufParse(t *testing.T) { }, { m: "test_float_histogram_with_zerothreshold_zero", - typ: MetricTypeHistogram, + typ: model.MetricTypeHistogram, }, { m: "test_float_histogram_with_zerothreshold_zero", @@ -971,7 +972,7 @@ func TestProtobufParse(t *testing.T) { }, { m: "rpc_durations_seconds", - typ: MetricTypeSummary, + typ: model.MetricTypeSummary, }, { m: "rpc_durations_seconds_count\xffservice\xffexponential", @@ -1022,7 +1023,7 @@ func TestProtobufParse(t *testing.T) { }, { m: "without_quantiles", - typ: MetricTypeSummary, + typ: model.MetricTypeSummary, }, { m: "without_quantiles_count", @@ -1044,7 +1045,7 @@ func TestProtobufParse(t *testing.T) { }, { m: "empty_histogram", - typ: MetricTypeHistogram, + typ: model.MetricTypeHistogram, }, { m: "empty_histogram", @@ -1063,7 +1064,7 @@ func TestProtobufParse(t *testing.T) { }, { m: "test_counter_with_createdtimestamp", - typ: MetricTypeCounter, + typ: model.MetricTypeCounter, }, { m: "test_counter_with_createdtimestamp", @@ -1079,7 +1080,7 @@ func TestProtobufParse(t *testing.T) { }, { m: "test_summary_with_createdtimestamp", - typ: MetricTypeSummary, + typ: model.MetricTypeSummary, }, { m: "test_summary_with_createdtimestamp_count", @@ -1103,7 +1104,7 @@ func TestProtobufParse(t *testing.T) { }, { m: "test_histogram_with_createdtimestamp", - typ: MetricTypeHistogram, + typ: model.MetricTypeHistogram, }, { m: "test_histogram_with_createdtimestamp", @@ -1123,7 +1124,7 @@ func TestProtobufParse(t *testing.T) { }, { m: "test_gaugehistogram_with_createdtimestamp", - typ: MetricTypeGaugeHistogram, + typ: model.MetricTypeGaugeHistogram, }, { m: "test_gaugehistogram_with_createdtimestamp", @@ -1149,7 +1150,7 @@ func TestProtobufParse(t *testing.T) { }, { // 1 m: "go_build_info", - typ: MetricTypeGauge, + typ: model.MetricTypeGauge, }, { // 2 m: "go_build_info\xFFchecksum\xFF\xFFpath\xFFgithub.com/prometheus/client_golang\xFFversion\xFF(devel)", @@ -1167,7 +1168,7 @@ func TestProtobufParse(t *testing.T) { }, { // 4 m: "go_memstats_alloc_bytes_total", - typ: MetricTypeCounter, + typ: model.MetricTypeCounter, }, { // 5 m: "go_memstats_alloc_bytes_total", @@ -1185,7 +1186,7 @@ func TestProtobufParse(t *testing.T) { }, { // 7 m: "something_untyped", - typ: MetricTypeUnknown, + typ: model.MetricTypeUnknown, }, { // 8 m: "something_untyped", @@ -1201,7 +1202,7 @@ func TestProtobufParse(t *testing.T) { }, { // 10 m: "test_histogram", - typ: MetricTypeHistogram, + typ: model.MetricTypeHistogram, }, { // 11 m: "test_histogram", @@ -1294,7 +1295,7 @@ func TestProtobufParse(t *testing.T) { }, { // 19 m: "test_gauge_histogram", - typ: MetricTypeGaugeHistogram, + typ: model.MetricTypeGaugeHistogram, }, { // 20 m: "test_gauge_histogram", @@ -1388,7 +1389,7 @@ func TestProtobufParse(t *testing.T) { }, { // 28 m: "test_float_histogram", - typ: MetricTypeHistogram, + typ: model.MetricTypeHistogram, }, { // 29 m: "test_float_histogram", @@ -1481,7 +1482,7 @@ func TestProtobufParse(t *testing.T) { }, { // 37 m: "test_gauge_float_histogram", - typ: MetricTypeGaugeHistogram, + typ: model.MetricTypeGaugeHistogram, }, { // 38 m: "test_gauge_float_histogram", @@ -1575,7 +1576,7 @@ func TestProtobufParse(t *testing.T) { }, { // 46 m: "test_histogram2", - typ: MetricTypeHistogram, + typ: model.MetricTypeHistogram, }, { // 47 m: "test_histogram2_count", @@ -1635,7 +1636,7 @@ func TestProtobufParse(t *testing.T) { }, { // 54 m: "test_histogram_family", - typ: MetricTypeHistogram, + typ: model.MetricTypeHistogram, }, { // 55 m: "test_histogram_family\xfffoo\xffbar", @@ -1765,7 +1766,7 @@ func TestProtobufParse(t *testing.T) { }, { // 68 m: "test_float_histogram_with_zerothreshold_zero", - typ: MetricTypeHistogram, + typ: model.MetricTypeHistogram, }, { // 69 m: "test_float_histogram_with_zerothreshold_zero", @@ -1789,7 +1790,7 @@ func TestProtobufParse(t *testing.T) { }, { // 71 m: "rpc_durations_seconds", - typ: MetricTypeSummary, + typ: model.MetricTypeSummary, }, { // 72 m: "rpc_durations_seconds_count\xffservice\xffexponential", @@ -1840,7 +1841,7 @@ func TestProtobufParse(t *testing.T) { }, { // 78 m: "without_quantiles", - typ: MetricTypeSummary, + typ: model.MetricTypeSummary, }, { // 79 m: "without_quantiles_count", @@ -1862,7 +1863,7 @@ func TestProtobufParse(t *testing.T) { }, { // 79 m: "empty_histogram", - typ: MetricTypeHistogram, + typ: model.MetricTypeHistogram, }, { // 80 m: "empty_histogram", @@ -1881,7 +1882,7 @@ func TestProtobufParse(t *testing.T) { }, { // 82 m: "test_counter_with_createdtimestamp", - typ: MetricTypeCounter, + typ: model.MetricTypeCounter, }, { // 83 m: "test_counter_with_createdtimestamp", @@ -1897,7 +1898,7 @@ func TestProtobufParse(t *testing.T) { }, { // 85 m: "test_summary_with_createdtimestamp", - typ: MetricTypeSummary, + typ: model.MetricTypeSummary, }, { // 86 m: "test_summary_with_createdtimestamp_count", @@ -1921,7 +1922,7 @@ func TestProtobufParse(t *testing.T) { }, { // 89 m: "test_histogram_with_createdtimestamp", - typ: MetricTypeHistogram, + typ: model.MetricTypeHistogram, }, { // 90 m: "test_histogram_with_createdtimestamp", @@ -1941,7 +1942,7 @@ func TestProtobufParse(t *testing.T) { }, { // 92 m: "test_gaugehistogram_with_createdtimestamp", - typ: MetricTypeGaugeHistogram, + typ: model.MetricTypeGaugeHistogram, }, { // 93 m: "test_gaugehistogram_with_createdtimestamp", diff --git a/scrape/scrape.go b/scrape/scrape.go index be27a5d48..de74987cc 100644 --- a/scrape/scrape.go +++ b/scrape/scrape.go @@ -966,7 +966,7 @@ func (c *scrapeCache) setType(metric []byte, t textparse.MetricType) { e, ok := c.metadata[string(metric)] if !ok { - e = &metaEntry{Metadata: metadata.Metadata{Type: textparse.MetricTypeUnknown}} + e = &metaEntry{Metadata: metadata.Metadata{Type: model.MetricTypeUnknown}} c.metadata[string(metric)] = e } if e.Type != t { @@ -983,7 +983,7 @@ func (c *scrapeCache) setHelp(metric, help []byte) { e, ok := c.metadata[string(metric)] if !ok { - e = &metaEntry{Metadata: metadata.Metadata{Type: textparse.MetricTypeUnknown}} + e = &metaEntry{Metadata: metadata.Metadata{Type: model.MetricTypeUnknown}} c.metadata[string(metric)] = e } if e.Help != string(help) { @@ -1000,7 +1000,7 @@ func (c *scrapeCache) setUnit(metric, unit []byte) { e, ok := c.metadata[string(metric)] if !ok { - e = &metaEntry{Metadata: metadata.Metadata{Type: textparse.MetricTypeUnknown}} + e = &metaEntry{Metadata: metadata.Metadata{Type: model.MetricTypeUnknown}} c.metadata[string(metric)] = e } if e.Unit != string(unit) { diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index 90578f2e9..1a416eeb6 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -971,19 +971,19 @@ test_metric 1 md, ok := cache.GetMetadata("test_metric") require.True(t, ok, "expected metadata to be present") - require.Equal(t, textparse.MetricTypeCounter, md.Type, "unexpected metric type") + require.Equal(t, model.MetricTypeCounter, md.Type, "unexpected metric type") require.Equal(t, "some help text", md.Help) require.Equal(t, "metric", md.Unit) md, ok = cache.GetMetadata("test_metric_no_help") require.True(t, ok, "expected metadata to be present") - require.Equal(t, textparse.MetricTypeGauge, md.Type, "unexpected metric type") + require.Equal(t, model.MetricTypeGauge, md.Type, "unexpected metric type") require.Equal(t, "", md.Help) require.Equal(t, "", md.Unit) md, ok = cache.GetMetadata("test_metric_no_type") require.True(t, ok, "expected metadata to be present") - require.Equal(t, textparse.MetricTypeUnknown, md.Type, "unexpected metric type") + require.Equal(t, model.MetricTypeUnknown, md.Type, "unexpected metric type") require.Equal(t, "other help text", md.Help) require.Equal(t, "", md.Unit) } diff --git a/storage/remote/codec.go b/storage/remote/codec.go index 67035cd8e..ffab821a5 100644 --- a/storage/remote/codec.go +++ b/storage/remote/codec.go @@ -33,7 +33,6 @@ import ( "github.com/prometheus/prometheus/model/exemplar" "github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/model/labels" - "github.com/prometheus/prometheus/model/textparse" "github.com/prometheus/prometheus/prompb" "github.com/prometheus/prometheus/storage" "github.com/prometheus/prometheus/tsdb/chunkenc" @@ -784,7 +783,7 @@ func labelsToLabelsProto(lbls labels.Labels, buf []prompb.Label) []prompb.Label } // metricTypeToMetricTypeProto transforms a Prometheus metricType into prompb metricType. Since the former is a string we need to transform it to an enum. -func metricTypeToMetricTypeProto(t textparse.MetricType) prompb.MetricMetadata_MetricType { +func metricTypeToMetricTypeProto(t model.MetricType) prompb.MetricMetadata_MetricType { mt := strings.ToUpper(string(t)) v, ok := prompb.MetricMetadata_MetricType_value[mt] if !ok { diff --git a/storage/remote/codec_test.go b/storage/remote/codec_test.go index d2a7d45be..ac8b0f0b5 100644 --- a/storage/remote/codec_test.go +++ b/storage/remote/codec_test.go @@ -20,11 +20,11 @@ import ( "testing" "github.com/gogo/protobuf/proto" + "github.com/prometheus/common/model" "github.com/stretchr/testify/require" "github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/model/labels" - "github.com/prometheus/prometheus/model/textparse" "github.com/prometheus/prometheus/prompb" "github.com/prometheus/prometheus/storage" "github.com/prometheus/prometheus/tsdb/chunkenc" @@ -488,17 +488,17 @@ func TestMergeLabels(t *testing.T) { func TestMetricTypeToMetricTypeProto(t *testing.T) { tc := []struct { desc string - input textparse.MetricType + input model.MetricType expected prompb.MetricMetadata_MetricType }{ { desc: "with a single-word metric", - input: textparse.MetricTypeCounter, + input: model.MetricTypeCounter, expected: prompb.MetricMetadata_COUNTER, }, { desc: "with a two-word metric", - input: textparse.MetricTypeStateset, + input: model.MetricTypeStateset, expected: prompb.MetricMetadata_STATESET, }, { diff --git a/storage/remote/metadata_watcher_test.go b/storage/remote/metadata_watcher_test.go index cd664bc8b..0cd6027a8 100644 --- a/storage/remote/metadata_watcher_test.go +++ b/storage/remote/metadata_watcher_test.go @@ -22,7 +22,6 @@ import ( "github.com/prometheus/common/model" "github.com/stretchr/testify/require" - "github.com/prometheus/prometheus/model/textparse" "github.com/prometheus/prometheus/scrape" ) @@ -108,13 +107,13 @@ func TestWatchScrapeManager_ReadyForCollection(t *testing.T) { Metadata: []scrape.MetricMetadata{ { Metric: "prometheus_tsdb_head_chunks_created_total", - Type: textparse.MetricTypeCounter, + Type: model.MetricTypeCounter, Help: "Total number", Unit: "", }, { Metric: "prometheus_remote_storage_retried_samples_total", - Type: textparse.MetricTypeCounter, + Type: model.MetricTypeCounter, Help: "Total number", Unit: "", }, @@ -124,7 +123,7 @@ func TestWatchScrapeManager_ReadyForCollection(t *testing.T) { Metadata: []scrape.MetricMetadata{ { Metric: "prometheus_tsdb_head_chunks_created_total", - Type: textparse.MetricTypeCounter, + Type: model.MetricTypeCounter, Help: "Total number", Unit: "", }, diff --git a/storage/remote/queue_manager_test.go b/storage/remote/queue_manager_test.go index c878c750b..17a904fcd 100644 --- a/storage/remote/queue_manager_test.go +++ b/storage/remote/queue_manager_test.go @@ -38,7 +38,6 @@ import ( "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/model/labels" - "github.com/prometheus/prometheus/model/textparse" "github.com/prometheus/prometheus/model/timestamp" "github.com/prometheus/prometheus/prompb" "github.com/prometheus/prometheus/scrape" @@ -180,7 +179,7 @@ func TestMetadataDelivery(t *testing.T) { for i := 0; i < numMetadata; i++ { metadata = append(metadata, scrape.MetricMetadata{ Metric: "prometheus_remote_storage_sent_metadata_bytes_total_" + strconv.Itoa(i), - Type: textparse.MetricTypeCounter, + Type: model.MetricTypeCounter, Help: "a nice help text", Unit: "", }) diff --git a/tsdb/head_append.go b/tsdb/head_append.go index f509317c8..f112ffa3a 100644 --- a/tsdb/head_append.go +++ b/tsdb/head_append.go @@ -1038,7 +1038,7 @@ func (a *headAppender) Commit() (err error) { for i, m := range a.metadata { series = a.metadataSeries[i] series.Lock() - series.meta = &metadata.Metadata{Type: record.ToTextparseMetricType(m.Type), Unit: m.Unit, Help: m.Help} + series.meta = &metadata.Metadata{Type: record.ToMetricType(m.Type), Unit: m.Unit, Help: m.Help} series.Unlock() } diff --git a/tsdb/head_wal.go b/tsdb/head_wal.go index a492a85a0..1be65f134 100644 --- a/tsdb/head_wal.go +++ b/tsdb/head_wal.go @@ -388,7 +388,7 @@ Outer: continue } s.meta = &metadata.Metadata{ - Type: record.ToTextparseMetricType(m.Type), + Type: record.ToMetricType(m.Type), Unit: m.Unit, Help: m.Help, } diff --git a/tsdb/record/record.go b/tsdb/record/record.go index 42a656dfe..3931ad05d 100644 --- a/tsdb/record/record.go +++ b/tsdb/record/record.go @@ -20,9 +20,10 @@ import ( "fmt" "math" + "github.com/prometheus/common/model" + "github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/model/labels" - "github.com/prometheus/prometheus/model/textparse" "github.com/prometheus/prometheus/storage" "github.com/prometheus/prometheus/tsdb/chunks" "github.com/prometheus/prometheus/tsdb/encoding" @@ -90,45 +91,45 @@ const ( Stateset MetricType = 7 ) -func GetMetricType(t textparse.MetricType) uint8 { +func GetMetricType(t model.MetricType) uint8 { switch t { - case textparse.MetricTypeCounter: + case model.MetricTypeCounter: return uint8(Counter) - case textparse.MetricTypeGauge: + case model.MetricTypeGauge: return uint8(Gauge) - case textparse.MetricTypeHistogram: + case model.MetricTypeHistogram: return uint8(HistogramSample) - case textparse.MetricTypeGaugeHistogram: + case model.MetricTypeGaugeHistogram: return uint8(GaugeHistogram) - case textparse.MetricTypeSummary: + case model.MetricTypeSummary: return uint8(Summary) - case textparse.MetricTypeInfo: + case model.MetricTypeInfo: return uint8(Info) - case textparse.MetricTypeStateset: + case model.MetricTypeStateset: return uint8(Stateset) default: return uint8(UnknownMT) } } -func ToTextparseMetricType(m uint8) textparse.MetricType { +func ToMetricType(m uint8) model.MetricType { switch m { case uint8(Counter): - return textparse.MetricTypeCounter + return model.MetricTypeCounter case uint8(Gauge): - return textparse.MetricTypeGauge + return model.MetricTypeGauge case uint8(HistogramSample): - return textparse.MetricTypeHistogram + return model.MetricTypeHistogram case uint8(GaugeHistogram): - return textparse.MetricTypeGaugeHistogram + return model.MetricTypeGaugeHistogram case uint8(Summary): - return textparse.MetricTypeSummary + return model.MetricTypeSummary case uint8(Info): - return textparse.MetricTypeInfo + return model.MetricTypeInfo case uint8(Stateset): - return textparse.MetricTypeStateset + return model.MetricTypeStateset default: - return textparse.MetricTypeUnknown + return model.MetricTypeUnknown } } diff --git a/web/api/v1/api.go b/web/api/v1/api.go index 8fa7ce14a..dd35d1fe9 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -41,7 +41,7 @@ import ( "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/model/labels" - "github.com/prometheus/prometheus/model/textparse" + "github.com/prometheus/prometheus/model/metadata" "github.com/prometheus/prometheus/model/timestamp" "github.com/prometheus/prometheus/promql" "github.com/prometheus/prometheus/promql/parser" @@ -1141,11 +1141,11 @@ func (api *API) targetMetadata(r *http.Request) apiFuncResult { } type metricMetadata struct { - Target labels.Labels `json:"target"` - Metric string `json:"metric,omitempty"` - Type textparse.MetricType `json:"type"` - Help string `json:"help"` - Unit string `json:"unit"` + Target labels.Labels `json:"target"` + Metric string `json:"metric,omitempty"` + Type model.MetricType `json:"type"` + Help string `json:"help"` + Unit string `json:"unit"` } // AlertmanagerDiscovery has all the active Alertmanagers. @@ -1221,14 +1221,8 @@ func rulesAlertsToAPIAlerts(rulesAlerts []*rules.Alert) []*Alert { return apiAlerts } -type metadata struct { - Type textparse.MetricType `json:"type"` - Help string `json:"help"` - Unit string `json:"unit"` -} - func (api *API) metricMetadata(r *http.Request) apiFuncResult { - metrics := map[string]map[metadata]struct{}{} + metrics := map[string]map[metadata.Metadata]struct{}{} limit := -1 if s := r.FormValue("limit"); s != "" { @@ -1250,7 +1244,7 @@ func (api *API) metricMetadata(r *http.Request) apiFuncResult { for _, t := range tt { if metric == "" { for _, mm := range t.ListMetadata() { - m := metadata{Type: mm.Type, Help: mm.Help, Unit: mm.Unit} + m := metadata.Metadata{Type: mm.Type, Help: mm.Help, Unit: mm.Unit} ms, ok := metrics[mm.Metric] if limitPerMetric > 0 && len(ms) >= limitPerMetric { @@ -1258,7 +1252,7 @@ func (api *API) metricMetadata(r *http.Request) apiFuncResult { } if !ok { - ms = map[metadata]struct{}{} + ms = map[metadata.Metadata]struct{}{} metrics[mm.Metric] = ms } ms[m] = struct{}{} @@ -1267,7 +1261,7 @@ func (api *API) metricMetadata(r *http.Request) apiFuncResult { } if md, ok := t.GetMetadata(metric); ok { - m := metadata{Type: md.Type, Help: md.Help, Unit: md.Unit} + m := metadata.Metadata{Type: md.Type, Help: md.Help, Unit: md.Unit} ms, ok := metrics[md.Metric] if limitPerMetric > 0 && len(ms) >= limitPerMetric { @@ -1275,7 +1269,7 @@ func (api *API) metricMetadata(r *http.Request) apiFuncResult { } if !ok { - ms = map[metadata]struct{}{} + ms = map[metadata.Metadata]struct{}{} metrics[md.Metric] = ms } ms[m] = struct{}{} @@ -1284,13 +1278,13 @@ func (api *API) metricMetadata(r *http.Request) apiFuncResult { } // Put the elements from the pseudo-set into a slice for marshaling. - res := map[string][]metadata{} + res := map[string][]metadata.Metadata{} for name, set := range metrics { if limit >= 0 && len(res) >= limit { break } - s := []metadata{} + s := []metadata.Metadata{} for metadata := range set { s = append(s, metadata) } diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index d4da05e46..c9ab84087 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -44,7 +44,7 @@ import ( "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/model/exemplar" "github.com/prometheus/prometheus/model/labels" - "github.com/prometheus/prometheus/model/textparse" + "github.com/prometheus/prometheus/model/metadata" "github.com/prometheus/prometheus/model/timestamp" "github.com/prometheus/prometheus/promql" "github.com/prometheus/prometheus/promql/parser" @@ -1584,7 +1584,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E metadata: []scrape.MetricMetadata{ { Metric: "go_threads", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "Number of OS threads created.", Unit: "", }, @@ -1597,7 +1597,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E "job": "test", }), Help: "Number of OS threads created.", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Unit: "", }, }, @@ -1614,7 +1614,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E metadata: []scrape.MetricMetadata{ { Metric: "prometheus_tsdb_storage_blocks_bytes", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "The number of bytes that are currently used for local storage by all blocks.", Unit: "", }, @@ -1628,7 +1628,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E }), Metric: "prometheus_tsdb_storage_blocks_bytes", Help: "The number of bytes that are currently used for local storage by all blocks.", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Unit: "", }, }, @@ -1642,7 +1642,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E metadata: []scrape.MetricMetadata{ { Metric: "go_threads", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "Number of OS threads created.", Unit: "", }, @@ -1653,7 +1653,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E metadata: []scrape.MetricMetadata{ { Metric: "prometheus_tsdb_storage_blocks_bytes", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "The number of bytes that are currently used for local storage by all blocks.", Unit: "", }, @@ -1667,7 +1667,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E }), Metric: "go_threads", Help: "Number of OS threads created.", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Unit: "", }, { @@ -1676,7 +1676,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E }), Metric: "prometheus_tsdb_storage_blocks_bytes", Help: "The number of bytes that are currently used for local storage by all blocks.", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Unit: "", }, }, @@ -1719,22 +1719,22 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E metadata: []scrape.MetricMetadata{ { Metric: "prometheus_engine_query_duration_seconds", - Type: textparse.MetricTypeSummary, + Type: model.MetricTypeSummary, Help: "Query timings", Unit: "", }, { Metric: "go_info", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "Information about the Go environment.", Unit: "", }, }, }, }, - response: map[string][]metadata{ - "prometheus_engine_query_duration_seconds": {{textparse.MetricTypeSummary, "Query timings", ""}}, - "go_info": {{textparse.MetricTypeGauge, "Information about the Go environment.", ""}}, + response: map[string][]metadata.Metadata{ + "prometheus_engine_query_duration_seconds": {{Type: model.MetricTypeSummary, Help: "Query timings", Unit: ""}}, + "go_info": {{Type: model.MetricTypeGauge, Help: "Information about the Go environment.", Unit: ""}}, }, }, // With duplicate metadata for a metric that comes from different targets. @@ -1746,7 +1746,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E metadata: []scrape.MetricMetadata{ { Metric: "go_threads", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "Number of OS threads created", Unit: "", }, @@ -1757,15 +1757,15 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E metadata: []scrape.MetricMetadata{ { Metric: "go_threads", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "Number of OS threads created", Unit: "", }, }, }, }, - response: map[string][]metadata{ - "go_threads": {{textparse.MetricTypeGauge, "Number of OS threads created", ""}}, + response: map[string][]metadata.Metadata{ + "go_threads": {{Type: model.MetricTypeGauge, Help: "Number of OS threads created"}}, }, }, // With non-duplicate metadata for the same metric from different targets. @@ -1777,7 +1777,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E metadata: []scrape.MetricMetadata{ { Metric: "go_threads", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "Number of OS threads created", Unit: "", }, @@ -1788,21 +1788,21 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E metadata: []scrape.MetricMetadata{ { Metric: "go_threads", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "Number of OS threads that were created.", Unit: "", }, }, }, }, - response: map[string][]metadata{ + response: map[string][]metadata.Metadata{ "go_threads": { - {textparse.MetricTypeGauge, "Number of OS threads created", ""}, - {textparse.MetricTypeGauge, "Number of OS threads that were created.", ""}, + {Type: model.MetricTypeGauge, Help: "Number of OS threads created"}, + {Type: model.MetricTypeGauge, Help: "Number of OS threads that were created."}, }, }, sorter: func(m interface{}) { - v := m.(map[string][]metadata)["go_threads"] + v := m.(map[string][]metadata.Metadata)["go_threads"] sort.Slice(v, func(i, j int) bool { return v[i].Help < v[j].Help @@ -1821,13 +1821,13 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E metadata: []scrape.MetricMetadata{ { Metric: "go_threads", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "Number of OS threads created", Unit: "", }, { Metric: "prometheus_engine_query_duration_seconds", - Type: textparse.MetricTypeSummary, + Type: model.MetricTypeSummary, Help: "Query Timmings.", Unit: "", }, @@ -1838,7 +1838,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E metadata: []scrape.MetricMetadata{ { Metric: "go_gc_duration_seconds", - Type: textparse.MetricTypeSummary, + Type: model.MetricTypeSummary, Help: "A summary of the GC invocation durations.", Unit: "", }, @@ -1857,31 +1857,31 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E metadata: []scrape.MetricMetadata{ { Metric: "go_threads", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "Number of OS threads created", Unit: "", }, { Metric: "go_threads", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "Repeated metadata", Unit: "", }, { Metric: "go_gc_duration_seconds", - Type: textparse.MetricTypeSummary, + Type: model.MetricTypeSummary, Help: "A summary of the GC invocation durations.", Unit: "", }, }, }, }, - response: map[string][]metadata{ + response: map[string][]metadata.Metadata{ "go_threads": { - {textparse.MetricTypeGauge, "Number of OS threads created", ""}, + {Type: model.MetricTypeGauge, Help: "Number of OS threads created"}, }, "go_gc_duration_seconds": { - {textparse.MetricTypeSummary, "A summary of the GC invocation durations.", ""}, + {Type: model.MetricTypeSummary, Help: "A summary of the GC invocation durations."}, }, }, }, @@ -1895,19 +1895,19 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E metadata: []scrape.MetricMetadata{ { Metric: "go_threads", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "Number of OS threads created", Unit: "", }, { Metric: "go_threads", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "Repeated metadata", Unit: "", }, { Metric: "go_gc_duration_seconds", - Type: textparse.MetricTypeSummary, + Type: model.MetricTypeSummary, Help: "A summary of the GC invocation durations.", Unit: "", }, @@ -1928,19 +1928,19 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E metadata: []scrape.MetricMetadata{ { Metric: "go_threads", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "Number of OS threads created", Unit: "", }, { Metric: "go_threads", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "Repeated metadata", Unit: "", }, { Metric: "go_gc_duration_seconds", - Type: textparse.MetricTypeSummary, + Type: model.MetricTypeSummary, Help: "A summary of the GC invocation durations.", Unit: "", }, @@ -1951,13 +1951,13 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E metadata: []scrape.MetricMetadata{ { Metric: "go_threads", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "Number of OS threads created, but from a different target", Unit: "", }, { Metric: "go_gc_duration_seconds", - Type: textparse.MetricTypeSummary, + Type: model.MetricTypeSummary, Help: "A summary of the GC invocation durations, but from a different target.", Unit: "", }, @@ -1977,7 +1977,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E metadata: []scrape.MetricMetadata{ { Metric: "go_threads", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "Number of OS threads created", Unit: "", }, @@ -1988,27 +1988,27 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E metadata: []scrape.MetricMetadata{ { Metric: "go_gc_duration_seconds", - Type: textparse.MetricTypeSummary, + Type: model.MetricTypeSummary, Help: "A summary of the GC invocation durations.", Unit: "", }, { Metric: "go_threads", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "Number of OS threads that were created.", Unit: "", }, }, }, }, - response: map[string][]metadata{ + response: map[string][]metadata.Metadata{ "go_threads": { - {textparse.MetricTypeGauge, "Number of OS threads created", ""}, - {textparse.MetricTypeGauge, "Number of OS threads that were created.", ""}, + {Type: model.MetricTypeGauge, Help: "Number of OS threads created"}, + {Type: model.MetricTypeGauge, Help: "Number of OS threads that were created."}, }, }, sorter: func(m interface{}) { - v := m.(map[string][]metadata)["go_threads"] + v := m.(map[string][]metadata.Metadata)["go_threads"] sort.Slice(v, func(i, j int) bool { return v[i].Help < v[j].Help @@ -2025,19 +2025,19 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E metadata: []scrape.MetricMetadata{ { Metric: "go_threads", - Type: textparse.MetricTypeGauge, + Type: model.MetricTypeGauge, Help: "Number of OS threads created", Unit: "", }, }, }, }, - response: map[string][]metadata{}, + response: map[string][]metadata.Metadata{}, }, // With no available metadata. { endpoint: api.metricMetadata, - response: map[string][]metadata{}, + response: map[string][]metadata.Metadata{}, }, { endpoint: api.serveConfig, @@ -2931,7 +2931,7 @@ func assertAPIResponseMetadataLen(t *testing.T, got interface{}, expLen int) { t.Helper() var gotLen int - response := got.(map[string][]metadata) + response := got.(map[string][]metadata.Metadata) for _, m := range response { gotLen += len(m) } From c83e1fc5748be3bd35bf0a31eb53690b412846a4 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 12 Dec 2023 12:14:36 +0000 Subject: [PATCH 17/23] textparse: remove MetricType alias No backwards-compatibility; make a clean break. Signed-off-by: Bryan Boreham --- model/textparse/interface.go | 4 +--- model/textparse/openmetricsparse.go | 2 +- model/textparse/promparse.go | 4 ++-- model/textparse/protobufparse_test.go | 2 +- scrape/scrape.go | 2 +- scrape/target.go | 3 +-- 6 files changed, 7 insertions(+), 10 deletions(-) diff --git a/model/textparse/interface.go b/model/textparse/interface.go index f6d93f063..3a363ebfb 100644 --- a/model/textparse/interface.go +++ b/model/textparse/interface.go @@ -44,7 +44,7 @@ type Parser interface { // Type returns the metric name and type in the current entry. // Must only be called after Next returned a type entry. // The returned byte slices become invalid after the next call to Next. - Type() ([]byte, MetricType) + Type() ([]byte, model.MetricType) // Unit returns the metric name and unit in the current entry. // Must only be called after Next returned a unit entry. @@ -111,5 +111,3 @@ const ( EntryUnit Entry = 4 EntryHistogram Entry = 5 // A series with a native histogram as a value. ) - -type MetricType = model.MetricType diff --git a/model/textparse/openmetricsparse.go b/model/textparse/openmetricsparse.go index d6b01eb8e..ddfbe4fc5 100644 --- a/model/textparse/openmetricsparse.go +++ b/model/textparse/openmetricsparse.go @@ -128,7 +128,7 @@ func (p *OpenMetricsParser) Help() ([]byte, []byte) { // Type returns the metric name and type in the current entry. // Must only be called after Next returned a type entry. // The returned byte slices become invalid after the next call to Next. -func (p *OpenMetricsParser) Type() ([]byte, MetricType) { +func (p *OpenMetricsParser) Type() ([]byte, model.MetricType) { return p.l.b[p.offsets[0]:p.offsets[1]], p.mtype } diff --git a/model/textparse/promparse.go b/model/textparse/promparse.go index cd028ef90..7123e52c3 100644 --- a/model/textparse/promparse.go +++ b/model/textparse/promparse.go @@ -148,7 +148,7 @@ type PromParser struct { builder labels.ScratchBuilder series []byte text []byte - mtype MetricType + mtype model.MetricType val float64 ts int64 hasTS bool @@ -192,7 +192,7 @@ func (p *PromParser) Help() ([]byte, []byte) { // Type returns the metric name and type in the current entry. // Must only be called after Next returned a type entry. // The returned byte slices become invalid after the next call to Next. -func (p *PromParser) Type() ([]byte, MetricType) { +func (p *PromParser) Type() ([]byte, model.MetricType) { return p.l.b[p.offsets[0]:p.offsets[1]], p.mtype } diff --git a/model/textparse/protobufparse_test.go b/model/textparse/protobufparse_test.go index f994ff966..7dcc85f54 100644 --- a/model/textparse/protobufparse_test.go +++ b/model/textparse/protobufparse_test.go @@ -623,7 +623,7 @@ func TestProtobufParse(t *testing.T) { m string t int64 v float64 - typ MetricType + typ model.MetricType help string unit string comment string diff --git a/scrape/scrape.go b/scrape/scrape.go index de74987cc..dd425db90 100644 --- a/scrape/scrape.go +++ b/scrape/scrape.go @@ -961,7 +961,7 @@ func (c *scrapeCache) forEachStale(f func(labels.Labels) bool) { } } -func (c *scrapeCache) setType(metric []byte, t textparse.MetricType) { +func (c *scrapeCache) setType(metric []byte, t model.MetricType) { c.metaMtx.Lock() e, ok := c.metadata[string(metric)] diff --git a/scrape/target.go b/scrape/target.go index fd984f5a6..0605f5349 100644 --- a/scrape/target.go +++ b/scrape/target.go @@ -30,7 +30,6 @@ import ( "github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/model/relabel" - "github.com/prometheus/prometheus/model/textparse" "github.com/prometheus/prometheus/model/value" "github.com/prometheus/prometheus/storage" ) @@ -87,7 +86,7 @@ type MetricMetadataStore interface { // MetricMetadata is a piece of metadata for a metric. type MetricMetadata struct { Metric string - Type textparse.MetricType + Type model.MetricType Help string Unit string } From 096ec129120b124ea14c78c8721c80dd0961b8ce Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 19 Dec 2023 18:53:09 +0000 Subject: [PATCH 18/23] Update comment about metadata in types.proto Signed-off-by: Bryan Boreham --- prompb/types.pb.go | 2 +- prompb/types.proto | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/prompb/types.pb.go b/prompb/types.pb.go index 125f868e9..93883daa1 100644 --- a/prompb/types.pb.go +++ b/prompb/types.pb.go @@ -164,7 +164,7 @@ func (Chunk_Encoding) EnumDescriptor() ([]byte, []int) { type MetricMetadata struct { // Represents the metric type, these match the set from Prometheus. - // Refer to model/textparse/interface.go for details. + // Refer to github.com/prometheus/common/model/metadata.go for details. Type MetricMetadata_MetricType `protobuf:"varint,1,opt,name=type,proto3,enum=prometheus.MetricMetadata_MetricType" json:"type,omitempty"` MetricFamilyName string `protobuf:"bytes,2,opt,name=metric_family_name,json=metricFamilyName,proto3" json:"metric_family_name,omitempty"` Help string `protobuf:"bytes,4,opt,name=help,proto3" json:"help,omitempty"` diff --git a/prompb/types.proto b/prompb/types.proto index aa322515c..61fc1e014 100644 --- a/prompb/types.proto +++ b/prompb/types.proto @@ -31,7 +31,7 @@ message MetricMetadata { } // Represents the metric type, these match the set from Prometheus. - // Refer to model/textparse/interface.go for details. + // Refer to github.com/prometheus/common/model/metadata.go for details. MetricType type = 1; string metric_family_name = 2; string help = 4; From b012366c33cb673110421b9e20e35fde99c992db Mon Sep 17 00:00:00 2001 From: Kumar Kalpadiptya Roy Date: Wed, 20 Dec 2023 00:28:59 +0530 Subject: [PATCH 19/23] Issue #13268: fix quality value in accept header Signed-off-by: Kumar Kalpadiptya Roy --- scrape/scrape.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrape/scrape.go b/scrape/scrape.go index be27a5d48..cb65053ba 100644 --- a/scrape/scrape.go +++ b/scrape/scrape.go @@ -675,7 +675,7 @@ func acceptHeader(sps []config.ScrapeProtocol) string { weight-- } // Default match anything. - vals = append(vals, fmt.Sprintf("*/*;q=%d", weight)) + vals = append(vals, fmt.Sprintf("*/*;q=0.%d", weight)) return strings.Join(vals, ",") } From 5df3820c7a5e099f0cdf8c503a01ac4b0498cf96 Mon Sep 17 00:00:00 2001 From: Filip Petkovski Date: Mon, 25 Dec 2023 11:19:16 +0100 Subject: [PATCH 20/23] Copy last histogram point Signed-off-by: Filip Petkovski --- promql/engine.go | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/promql/engine.go b/promql/engine.go index 12755663d..5d6f7b9a0 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -2126,17 +2126,20 @@ loop: // The sought sample might also be in the range. switch soughtValueType { case chunkenc.ValFloatHistogram, chunkenc.ValHistogram: - t, h := it.AtFloatHistogram() - if t == maxt && !value.IsStaleNaN(h.Sum) { - if ev.currentSamples >= ev.maxSamples { - ev.error(ErrTooManySamples(env)) + t := it.AtT() + if t == maxt { + _, h := it.AtFloatHistogram() + if !value.IsStaleNaN(h.Sum) { + if ev.currentSamples >= ev.maxSamples { + ev.error(ErrTooManySamples(env)) + } + if histograms == nil { + histograms = getHPointSlice(16) + } + point := HPoint{T: t, H: h.Copy()} + histograms = append(histograms, point) + ev.currentSamples += point.size() } - if histograms == nil { - histograms = getHPointSlice(16) - } - point := HPoint{T: t, H: h} - histograms = append(histograms, point) - ev.currentSamples += point.size() } case chunkenc.ValFloat: t, f := it.At() From 35f9620cd1f212b37cc3074bbd59bdfae713f87d Mon Sep 17 00:00:00 2001 From: Filip Petkovski Date: Mon, 25 Dec 2023 11:30:29 +0100 Subject: [PATCH 21/23] Expand benchmark Signed-off-by: Filip Petkovski --- promql/bench_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/promql/bench_test.go b/promql/bench_test.go index 13eba3714..b7a4978de 100644 --- a/promql/bench_test.go +++ b/promql/bench_test.go @@ -296,8 +296,12 @@ func BenchmarkNativeHistograms(b *testing.B) { query: "sum(native_histogram_series)", }, { - name: "sum rate", - query: "sum(rate(native_histogram_series[1m]))", + name: "sum rate with short rate interval", + query: "sum(rate(native_histogram_series[2m]))", + }, + { + name: "sum rate with long rate interval", + query: "sum(rate(native_histogram_series[20m]))", }, } From 0e1ae1d1caa65c398d0024a4ecb7977405e15a07 Mon Sep 17 00:00:00 2001 From: Filip Petkovski Date: Mon, 25 Dec 2023 11:41:07 +0100 Subject: [PATCH 22/23] Add comment Signed-off-by: Filip Petkovski --- promql/engine.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/promql/engine.go b/promql/engine.go index 5d6f7b9a0..2ea37dae6 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -2136,6 +2136,8 @@ loop: if histograms == nil { histograms = getHPointSlice(16) } + // The last sample comes directly from the iterator, so we need to copy it to + // avoid having the same reference twice in the buffer. point := HPoint{T: t, H: h.Copy()} histograms = append(histograms, point) ev.currentSamples += point.size() From 2ddb3596ef4d2b1de482337ce1943476699b82fb Mon Sep 17 00:00:00 2001 From: Mile Druzijanic <153705375+zedGGs@users.noreply.github.com> Date: Thu, 28 Dec 2023 21:49:57 +0100 Subject: [PATCH 23/23] Adding small test update for temp dir using t.TempDir (#13293) * Adding small test update for temp dir using t.TempDir Signed-off-by: Mile Druzijanic Signed-off-by: Mile Druzijanic * removing not required cleanup Signed-off-by: Mile Druzijanic --------- Signed-off-by: Mile Druzijanic Signed-off-by: Mile Druzijanic --- promql/engine_test.go | 5 +---- tsdb/tsdbutil/dir_locker_testutil.go | 10 +++------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/promql/engine_test.go b/promql/engine_test.go index 105cdc10d..b3e7d4094 100644 --- a/promql/engine_test.go +++ b/promql/engine_test.go @@ -18,7 +18,6 @@ import ( "errors" "fmt" "math" - "os" "sort" "testing" "time" @@ -47,9 +46,7 @@ func TestMain(m *testing.M) { func TestQueryConcurrency(t *testing.T) { maxConcurrency := 10 - dir, err := os.MkdirTemp("", "test_concurrency") - require.NoError(t, err) - defer os.RemoveAll(dir) + dir := t.TempDir() queryTracker := NewActiveQueryTracker(dir, maxConcurrency, nil) t.Cleanup(queryTracker.Close) diff --git a/tsdb/tsdbutil/dir_locker_testutil.go b/tsdb/tsdbutil/dir_locker_testutil.go index a4cf5abd6..a7f43d8ee 100644 --- a/tsdb/tsdbutil/dir_locker_testutil.go +++ b/tsdb/tsdbutil/dir_locker_testutil.go @@ -60,12 +60,8 @@ func TestDirLockerUsage(t *testing.T, open func(t *testing.T, data string, creat for _, c := range cases { t.Run(fmt.Sprintf("%+v", c), func(t *testing.T) { - tmpdir, err := os.MkdirTemp("", "test") - require.NoError(t, err) - t.Cleanup(func() { - require.NoError(t, os.RemoveAll(tmpdir)) - }) - + tmpdir := t.TempDir() + // Test preconditions (file already exists + lockfile option) if c.fileAlreadyExists { tmpLocker, err := NewDirLocker(tmpdir, "tsdb", log.NewNopLogger(), nil) @@ -82,7 +78,7 @@ func TestDirLockerUsage(t *testing.T, open func(t *testing.T, data string, creat // Check that the lockfile is always deleted if !c.lockFileDisabled { - _, err = os.Stat(locker.path) + _, err := os.Stat(locker.path) require.True(t, os.IsNotExist(err), "lockfile was not deleted") } })