From 4bf9c6bb82d7d81c010e698064df7b54e195d4d6 Mon Sep 17 00:00:00 2001 From: gotjosh Date: Wed, 4 Dec 2019 11:08:21 +0000 Subject: [PATCH 1/3] Allow targets to be injected as arguments when creating a testTargetRetriever Previously, the struct `testTargetRetriever` had hardcoded active and dropped targets. This made it difficult to change the target information depending on the test case. This change introduces a way to define them as arguments and pass it to a constructor for building. It lays a foundation for dynamically defining targets with various set of arguments to test different scenarios. Signed-off-by: gotjosh --- web/api/v1/api_test.go | 179 +++++++++++++++++++++++++++++++---------- 1 file changed, 136 insertions(+), 43 deletions(-) diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index 13bce6cfa..7bd4087ee 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -55,55 +55,62 @@ import ( "github.com/prometheus/prometheus/util/testutil" ) -type testTargetRetriever struct{} +type testTargetRetriever struct { + activeTargets map[string][]*scrape.Target + droppedTargets map[string][]*scrape.Target +} + +type testTargetParams struct { + Identifier string + Labels []labels.Label + DiscoveredLabels []labels.Label + Params url.Values + Reports []*testReport + Active bool +} + +type testReport struct { + Start time.Time + Duration time.Duration + Error error +} + +func newTestTargetRetriever(targetsInfo []*testTargetParams) *testTargetRetriever { + var activeTargets map[string][]*scrape.Target + var droppedTargets map[string][]*scrape.Target + activeTargets = make(map[string][]*scrape.Target) + droppedTargets = make(map[string][]*scrape.Target) + + for _, t := range targetsInfo { + nt := scrape.NewTarget(t.Labels, t.DiscoveredLabels, t.Params) + + for _, r := range t.Reports { + nt.Report(r.Start, r.Duration, r.Error) + } + + if t.Active { + activeTargets[t.Identifier] = []*scrape.Target{nt} + } else { + droppedTargets[t.Identifier] = []*scrape.Target{nt} + } + } + + return &testTargetRetriever{ + activeTargets: activeTargets, + droppedTargets: droppedTargets, + } +} var ( scrapeStart = time.Now().Add(-11 * time.Second) ) func (t testTargetRetriever) TargetsActive() map[string][]*scrape.Target { - testTarget := scrape.NewTarget( - labels.FromMap(map[string]string{ - model.SchemeLabel: "http", - model.AddressLabel: "example.com:8080", - model.MetricsPathLabel: "/metrics", - model.JobLabel: "test", - }), - nil, - url.Values{}, - ) - testTarget.Report(scrapeStart, 70*time.Millisecond, nil) - blackboxTarget := scrape.NewTarget( - labels.FromMap(map[string]string{ - model.SchemeLabel: "http", - model.AddressLabel: "localhost:9115", - model.MetricsPathLabel: "/probe", - model.JobLabel: "blackbox", - }), - nil, - url.Values{"target": []string{"example.com"}}, - ) - blackboxTarget.Report(scrapeStart, 100*time.Millisecond, errors.New("failed")) - return map[string][]*scrape.Target{ - "test": {testTarget}, - "blackbox": {blackboxTarget}, - } + return t.activeTargets } + func (t testTargetRetriever) TargetsDropped() map[string][]*scrape.Target { - return map[string][]*scrape.Target{ - "blackbox": { - scrape.NewTarget( - nil, - labels.FromMap(map[string]string{ - model.AddressLabel: "http://dropped.example.com:9115", - model.MetricsPathLabel: "/probe", - model.SchemeLabel: "http", - model.JobLabel: "blackbox", - }), - url.Values{}, - ), - }, - } + return t.droppedTargets } type testAlertmanagerRetriever struct{} @@ -243,10 +250,53 @@ func TestEndpoints(t *testing.T) { algr.RuleGroups() + targets := []*testTargetParams{ + { + Identifier: "test", + Labels: labels.FromMap(map[string]string{ + model.SchemeLabel: "http", + model.AddressLabel: "example.com:8080", + model.MetricsPathLabel: "/metrics", + model.JobLabel: "test", + }), + DiscoveredLabels: nil, + Params: url.Values{}, + Reports: []*testReport{{scrapeStart, 70 * time.Millisecond, nil}}, + Active: true, + }, + { + Identifier: "blackbox", + Labels: labels.FromMap(map[string]string{ + model.SchemeLabel: "http", + model.AddressLabel: "localhost:9115", + model.MetricsPathLabel: "/probe", + model.JobLabel: "blackbox", + }), + DiscoveredLabels: nil, + Params: url.Values{"target": []string{"example.com"}}, + Reports: []*testReport{{scrapeStart, 100 * time.Millisecond, errors.New("failed")}}, + Active: true, + }, + { + Identifier: "blackbox", + Labels: nil, + DiscoveredLabels: labels.FromMap(map[string]string{ + model.SchemeLabel: "http", + model.AddressLabel: "http://dropped.example.com:9115", + model.MetricsPathLabel: "/probe", + model.JobLabel: "blackbox", + }), + Params: url.Values{}, + Active: false, + }, + } + + testTargetRetriever := newTestTargetRetriever(targets) + api := &API{ Queryable: suite.Storage(), QueryEngine: suite.QueryEngine(), - targetRetriever: testTargetRetriever{}, + targetRetriever: testTargetRetriever, alertmanagerRetriever: testAlertmanagerRetriever{}, flagsMap: sampleFlagMap, now: func() time.Time { return now }, @@ -305,10 +355,53 @@ func TestEndpoints(t *testing.T) { algr.RuleGroups() + targets := []*testTargetParams{ + { + Identifier: "test", + Labels: labels.FromMap(map[string]string{ + model.SchemeLabel: "http", + model.AddressLabel: "example.com:8080", + model.MetricsPathLabel: "/metrics", + model.JobLabel: "test", + }), + DiscoveredLabels: nil, + Params: url.Values{}, + Reports: []*testReport{{scrapeStart, 70 * time.Millisecond, nil}}, + Active: true, + }, + { + Identifier: "blackbox", + Labels: labels.FromMap(map[string]string{ + model.SchemeLabel: "http", + model.AddressLabel: "localhost:9115", + model.MetricsPathLabel: "/probe", + model.JobLabel: "blackbox", + }), + DiscoveredLabels: nil, + Params: url.Values{"target": []string{"example.com"}}, + Reports: []*testReport{{scrapeStart, 100 * time.Millisecond, errors.New("failed")}}, + Active: true, + }, + { + Identifier: "blackbox", + Labels: nil, + DiscoveredLabels: labels.FromMap(map[string]string{ + model.SchemeLabel: "http", + model.AddressLabel: "http://dropped.example.com:9115", + model.MetricsPathLabel: "/probe", + model.JobLabel: "blackbox", + }), + Params: url.Values{}, + Active: false, + }, + } + + testTargetRetriever := newTestTargetRetriever(targets) + api := &API{ Queryable: remote, QueryEngine: suite.QueryEngine(), - targetRetriever: testTargetRetriever{}, + targetRetriever: testTargetRetriever, alertmanagerRetriever: testAlertmanagerRetriever{}, flagsMap: sampleFlagMap, now: func() time.Time { return now }, From 05842176a6080cbca76e917528714d2da95c8d92 Mon Sep 17 00:00:00 2001 From: gotjosh Date: Wed, 4 Dec 2019 15:18:27 +0000 Subject: [PATCH 2/3] Make the scrape.metricMetadataStore interface public To test the implementation of our metric metadata API, we need to represent various states of metadata in the scrape metadata store. That is currently not possible as the interface and method to set the store are private. This changes the interface, list and get methods, and the SetMetadaStore function to be public. Incidentally, the scrapeCache implementation needs to be renamed to match the new signature. Signed-off-by: gotjosh --- scrape/scrape.go | 6 +++--- scrape/scrape_test.go | 6 +++--- scrape/target.go | 14 +++++++------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/scrape/scrape.go b/scrape/scrape.go index 3c6ecf356..303d7974d 100644 --- a/scrape/scrape.go +++ b/scrape/scrape.go @@ -209,7 +209,7 @@ func newScrapePool(cfg *config.ScrapeConfig, app Appendable, jitterSeed uint64, sp.newLoop = func(opts scrapeLoopOptions) loop { // Update the targets retrieval function for metadata to a new scrape cache. cache := newScrapeCache() - opts.target.setMetadataStore(cache) + opts.target.SetMetadataStore(cache) return newScrapeLoop( ctx, @@ -794,7 +794,7 @@ func (c *scrapeCache) setUnit(metric, unit []byte) { c.metaMtx.Unlock() } -func (c *scrapeCache) getMetadata(metric string) (MetricMetadata, bool) { +func (c *scrapeCache) GetMetadata(metric string) (MetricMetadata, bool) { c.metaMtx.Lock() defer c.metaMtx.Unlock() @@ -810,7 +810,7 @@ func (c *scrapeCache) getMetadata(metric string) (MetricMetadata, bool) { }, true } -func (c *scrapeCache) listMetadata() []MetricMetadata { +func (c *scrapeCache) ListMetadata() []MetricMetadata { c.metaMtx.Lock() defer c.metaMtx.Unlock() diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index 406dd8c4c..01788bef8 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -615,19 +615,19 @@ test_metric 1 testutil.Ok(t, err) testutil.Equals(t, 1, total) - md, ok := cache.getMetadata("test_metric") + md, ok := cache.GetMetadata("test_metric") testutil.Assert(t, ok, "expected metadata to be present") testutil.Assert(t, textparse.MetricTypeCounter == md.Type, "unexpected metric type") testutil.Equals(t, "some help text", md.Help) testutil.Equals(t, "metric", md.Unit) - md, ok = cache.getMetadata("test_metric_no_help") + md, ok = cache.GetMetadata("test_metric_no_help") testutil.Assert(t, ok, "expected metadata to be present") testutil.Assert(t, textparse.MetricTypeGauge == md.Type, "unexpected metric type") testutil.Equals(t, "", md.Help) testutil.Equals(t, "", md.Unit) - md, ok = cache.getMetadata("test_metric_no_type") + md, ok = cache.GetMetadata("test_metric_no_type") testutil.Assert(t, ok, "expected metadata to be present") testutil.Assert(t, textparse.MetricTypeUnknown == md.Type, "unexpected metric type") testutil.Equals(t, "other help text", md.Help) diff --git a/scrape/target.go b/scrape/target.go index d3a6a379e..a6955d3ff 100644 --- a/scrape/target.go +++ b/scrape/target.go @@ -58,7 +58,7 @@ type Target struct { lastScrape time.Time lastScrapeDuration time.Duration health TargetHealth - metadata metricMetadataStore + metadata MetricMetadataStore } // NewTarget creates a reasonably configured target for querying. @@ -75,9 +75,9 @@ func (t *Target) String() string { return t.URL().String() } -type metricMetadataStore interface { - listMetadata() []MetricMetadata - getMetadata(metric string) (MetricMetadata, bool) +type MetricMetadataStore interface { + ListMetadata() []MetricMetadata + GetMetadata(metric string) (MetricMetadata, bool) } // MetricMetadata is a piece of metadata for a metric. @@ -95,7 +95,7 @@ func (t *Target) MetadataList() []MetricMetadata { if t.metadata == nil { return nil } - return t.metadata.listMetadata() + return t.metadata.ListMetadata() } // Metadata returns type and help metadata for the given metric. @@ -106,10 +106,10 @@ func (t *Target) Metadata(metric string) (MetricMetadata, bool) { if t.metadata == nil { return MetricMetadata{}, false } - return t.metadata.getMetadata(metric) + return t.metadata.GetMetadata(metric) } -func (t *Target) setMetadataStore(s metricMetadataStore) { +func (t *Target) SetMetadataStore(s MetricMetadataStore) { t.mtx.Lock() defer t.mtx.Unlock() t.metadata = s From 428089f83f8fb21c38b7cc8a865c71f98f41f416 Mon Sep 17 00:00:00 2001 From: gotjosh Date: Wed, 4 Dec 2019 19:33:01 +0000 Subject: [PATCH 3/3] api: tests for /target/metadata API endpoint This commit introduces several test cases for the current /targets/metadata API endpoint. To achieve so, we use a mock of the metadataStore and inject it to the targets under test. Currently, three success cases are covered: with a metric name, with a target matcher, and with both. As for the failure scenario, the one where we couldn't match against a particular metric is covered. Signed-off-by: gotjosh --- web/api/v1/api_test.go | 260 ++++++++++++++++++++++++++++------------- 1 file changed, 176 insertions(+), 84 deletions(-) diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index 7bd4087ee..acaff7d4c 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -43,6 +43,7 @@ import ( "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/pkg/gate" "github.com/prometheus/prometheus/pkg/labels" + "github.com/prometheus/prometheus/pkg/textparse" "github.com/prometheus/prometheus/pkg/timestamp" "github.com/prometheus/prometheus/prompb" "github.com/prometheus/prometheus/promql" @@ -55,6 +56,28 @@ import ( "github.com/prometheus/prometheus/util/testutil" ) +// testMetaStore satisfies the scrape.MetricMetadataStore interface. +// It is used to inject specific metadata as part of a test case. +type testMetaStore struct { + Metadata []scrape.MetricMetadata +} + +func (s *testMetaStore) ListMetadata() []scrape.MetricMetadata { + return s.Metadata +} + +func (s *testMetaStore) GetMetadata(metric string) (scrape.MetricMetadata, bool) { + for _, m := range s.Metadata { + if metric == m.Metric { + return m, true + } + } + + return scrape.MetricMetadata{}, false +} + +// testTargetRetriever represents a list of targets to scrape. +// It is used to represent targets as part of test cases. type testTargetRetriever struct { activeTargets map[string][]*scrape.Target droppedTargets map[string][]*scrape.Target @@ -113,6 +136,20 @@ func (t testTargetRetriever) TargetsDropped() map[string][]*scrape.Target { return t.droppedTargets } +func (t testTargetRetriever) setMetadataStoreForTargets(identifier string, metadata scrape.MetricMetadataStore) error { + targets, ok := t.activeTargets[identifier] + + if !ok { + return errors.New("targets not found") + } + + for _, at := range targets { + at.SetMetadataStore(metadata) + } + + return nil +} + type testAlertmanagerRetriever struct{} func (t testAlertmanagerRetriever) Alertmanagers() []*url.URL { @@ -250,48 +287,7 @@ func TestEndpoints(t *testing.T) { algr.RuleGroups() - targets := []*testTargetParams{ - { - Identifier: "test", - Labels: labels.FromMap(map[string]string{ - model.SchemeLabel: "http", - model.AddressLabel: "example.com:8080", - model.MetricsPathLabel: "/metrics", - model.JobLabel: "test", - }), - DiscoveredLabels: nil, - Params: url.Values{}, - Reports: []*testReport{{scrapeStart, 70 * time.Millisecond, nil}}, - Active: true, - }, - { - Identifier: "blackbox", - Labels: labels.FromMap(map[string]string{ - model.SchemeLabel: "http", - model.AddressLabel: "localhost:9115", - model.MetricsPathLabel: "/probe", - model.JobLabel: "blackbox", - }), - DiscoveredLabels: nil, - Params: url.Values{"target": []string{"example.com"}}, - Reports: []*testReport{{scrapeStart, 100 * time.Millisecond, errors.New("failed")}}, - Active: true, - }, - { - Identifier: "blackbox", - Labels: nil, - DiscoveredLabels: labels.FromMap(map[string]string{ - model.SchemeLabel: "http", - model.AddressLabel: "http://dropped.example.com:9115", - model.MetricsPathLabel: "/probe", - model.JobLabel: "blackbox", - }), - Params: url.Values{}, - Active: false, - }, - } - - testTargetRetriever := newTestTargetRetriever(targets) + testTargetRetriever := setupTestTargetRetriever(t) api := &API{ Queryable: suite.Storage(), @@ -355,48 +351,7 @@ func TestEndpoints(t *testing.T) { algr.RuleGroups() - targets := []*testTargetParams{ - { - Identifier: "test", - Labels: labels.FromMap(map[string]string{ - model.SchemeLabel: "http", - model.AddressLabel: "example.com:8080", - model.MetricsPathLabel: "/metrics", - model.JobLabel: "test", - }), - DiscoveredLabels: nil, - Params: url.Values{}, - Reports: []*testReport{{scrapeStart, 70 * time.Millisecond, nil}}, - Active: true, - }, - { - Identifier: "blackbox", - Labels: labels.FromMap(map[string]string{ - model.SchemeLabel: "http", - model.AddressLabel: "localhost:9115", - model.MetricsPathLabel: "/probe", - model.JobLabel: "blackbox", - }), - DiscoveredLabels: nil, - Params: url.Values{"target": []string{"example.com"}}, - Reports: []*testReport{{scrapeStart, 100 * time.Millisecond, errors.New("failed")}}, - Active: true, - }, - { - Identifier: "blackbox", - Labels: nil, - DiscoveredLabels: labels.FromMap(map[string]string{ - model.SchemeLabel: "http", - model.AddressLabel: "http://dropped.example.com:9115", - model.MetricsPathLabel: "/probe", - model.JobLabel: "blackbox", - }), - Params: url.Values{}, - Active: false, - }, - } - - testTargetRetriever := newTestTargetRetriever(targets) + testTargetRetriever := setupTestTargetRetriever(t) api := &API{ Queryable: remote, @@ -450,6 +405,76 @@ func TestLabelNames(t *testing.T) { } } +func setupTestTargetRetriever(t *testing.T) *testTargetRetriever { + t.Helper() + + targets := []*testTargetParams{ + { + Identifier: "test", + Labels: labels.FromMap(map[string]string{ + model.SchemeLabel: "http", + model.AddressLabel: "example.com:8080", + model.MetricsPathLabel: "/metrics", + model.JobLabel: "test", + }), + DiscoveredLabels: nil, + Params: url.Values{}, + Reports: []*testReport{{scrapeStart, 70 * time.Millisecond, nil}}, + Active: true, + }, + { + Identifier: "blackbox", + Labels: labels.FromMap(map[string]string{ + model.SchemeLabel: "http", + model.AddressLabel: "localhost:9115", + model.MetricsPathLabel: "/probe", + model.JobLabel: "blackbox", + }), + DiscoveredLabels: nil, + Params: url.Values{"target": []string{"example.com"}}, + Reports: []*testReport{{scrapeStart, 100 * time.Millisecond, errors.New("failed")}}, + Active: true, + }, + { + Identifier: "blackbox", + Labels: nil, + DiscoveredLabels: labels.FromMap(map[string]string{ + model.SchemeLabel: "http", + model.AddressLabel: "http://dropped.example.com:9115", + model.MetricsPathLabel: "/probe", + model.JobLabel: "blackbox", + }), + Params: url.Values{}, + Active: false, + }, + } + targetRetriever := newTestTargetRetriever(targets) + + targetRetriever.setMetadataStoreForTargets("test", &testMetaStore{ + Metadata: []scrape.MetricMetadata{ + { + Metric: "go_threads", + Type: textparse.MetricTypeGauge, + Help: "Number of OS threads created.", + Unit: "", + }, + }, + }) + + targetRetriever.setMetadataStoreForTargets("blackbox", &testMetaStore{ + Metadata: []scrape.MetricMetadata{ + { + Metric: "prometheus_tsdb_storage_blocks_bytes", + Type: textparse.MetricTypeGauge, + Help: "The number of bytes that are currently used for local storage by all blocks.", + Unit: "", + }, + }, + }) + + return targetRetriever +} + func setupRemote(s storage.Storage) *httptest.Server { handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { req, err := remote.DecodeReadRequest(r) @@ -926,6 +951,73 @@ func testEndpoints(t *testing.T, api *API, testLabelAPI bool) { }, }, }, + // With a matching metric. + { + endpoint: api.targetMetadata, + query: url.Values{ + "metric": []string{"go_threads"}, + }, + response: []metricMetadata{ + { + Target: labels.FromMap(map[string]string{ + "job": "test", + }), + Help: "Number of OS threads created.", + Type: textparse.MetricTypeGauge, + Unit: "", + }, + }, + }, + // With a matching target. + { + endpoint: api.targetMetadata, + query: url.Values{ + "match_target": []string{"{job=\"blackbox\"}"}, + }, + response: []metricMetadata{ + { + Target: labels.FromMap(map[string]string{ + "job": "blackbox", + }), + Metric: "prometheus_tsdb_storage_blocks_bytes", + Help: "The number of bytes that are currently used for local storage by all blocks.", + Type: textparse.MetricTypeGauge, + Unit: "", + }, + }, + }, + // Without a target or metric. + { + endpoint: api.targetMetadata, + response: []metricMetadata{ + { + Target: labels.FromMap(map[string]string{ + "job": "test", + }), + Metric: "go_threads", + Help: "Number of OS threads created.", + Type: textparse.MetricTypeGauge, + Unit: "", + }, + { + Target: labels.FromMap(map[string]string{ + "job": "blackbox", + }), + Metric: "prometheus_tsdb_storage_blocks_bytes", + Help: "The number of bytes that are currently used for local storage by all blocks.", + Type: textparse.MetricTypeGauge, + Unit: "", + }, + }, + }, + // Without a matching metric. + { + endpoint: api.targetMetadata, + query: url.Values{ + "match_target": []string{"{job=\"non-existentblackbox\"}"}, + }, + errType: errorNotFound, + }, { endpoint: api.alertmanagers, response: &AlertmanagerDiscovery{