From 8ff7e7b62b4b96268f6e40b18e24086a3aeec358 Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Fri, 20 Mar 2020 12:02:08 +0100 Subject: [PATCH 1/5] Improve metrics integration tests Signed-off-by: Marco Pracucci --- integration/asserts.go | 67 ++++++++++++ integration/chunks_storage_backends_test.go | 5 + integration/ingester_flush_test.go | 14 ++- integration/ingester_hand_over_test.go | 5 + integration/metrics_test.go | 112 -------------------- integration/querier_test.go | 5 + integration/query_frontend_test.go | 6 ++ 7 files changed, 99 insertions(+), 115 deletions(-) create mode 100644 integration/asserts.go delete mode 100644 integration/metrics_test.go diff --git a/integration/asserts.go b/integration/asserts.go new file mode 100644 index 00000000000..08fb4faab00 --- /dev/null +++ b/integration/asserts.go @@ -0,0 +1,67 @@ +package main + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/cortexproject/cortex/integration/e2ecortex" +) + +type ServiceType string + +const ( + Distributor = ServiceType("distributor") + Ingester = ServiceType("ingester") + Querier = ServiceType("querier") + QueryFrontend = ServiceType("query-frontend") + TableManager = ServiceType("table-manager") +) + +var ( + // Service-specific metrics prefixes which shouldn't be used by any other service. + serviceMetricsPrefixes = map[ServiceType][]string{ + Distributor: []string{}, + Ingester: []string{}, + Querier: []string{}, + QueryFrontend: []string{"cortex_frontend", "cortex_query_frontend"}, + TableManager: []string{}, + } +) + +func assertServiceMetricsPrefixes(t *testing.T, serviceType ServiceType, service *e2ecortex.CortexService) { + metrics, err := service.Metrics() + require.NoError(t, err) + + // Build the list of blacklisted metrics prefixes for this specific service. + blacklist := getBlacklistedMetricsPrefixesByService(serviceType) + + // Ensure no metric name matches the blacklisted prefixes. + for _, metricLine := range strings.Split(metrics, "\n") { + metricLine = strings.TrimSpace(metricLine) + if metricLine == "" || strings.HasPrefix(metricLine, "#") { + continue + } + + for _, prefix := range blacklist { + assert.NotRegexp(t, "^"+prefix, metricLine, "service: %s", service.Name()) + } + } +} + +func getBlacklistedMetricsPrefixesByService(serviceType ServiceType) []string { + blacklist := []string{} + + // Add any service-specific metrics prefix excluding the service itself. + for t, prefixes := range serviceMetricsPrefixes { + if t == serviceType { + continue + } + + blacklist = append(blacklist, prefixes...) + } + + return blacklist +} diff --git a/integration/chunks_storage_backends_test.go b/integration/chunks_storage_backends_test.go index a0f94befb02..1c086e050e6 100644 --- a/integration/chunks_storage_backends_test.go +++ b/integration/chunks_storage_backends_test.go @@ -108,4 +108,9 @@ func TestChunksStorageAllIndexBackends(t *testing.T) { require.Equal(t, model.ValVector, result.Type()) assert.Equal(t, expectedVector, result.(model.Vector)) } + + // Ensure no service-specific metrics prefix is used by the wrong service. + assertServiceMetricsPrefixes(t, Distributor, distributor) + assertServiceMetricsPrefixes(t, Ingester, ingester) + assertServiceMetricsPrefixes(t, Querier, querier) } diff --git a/integration/ingester_flush_test.go b/integration/ingester_flush_test.go index 7a8a5dcd30a..d483677eed4 100644 --- a/integration/ingester_flush_test.go +++ b/integration/ingester_flush_test.go @@ -33,12 +33,12 @@ func TestIngesterFlushWithChunksStorage(t *testing.T) { require.NoError(t, writeFileToSharedDir(s, cortexSchemaConfigFile, []byte(cortexSchemaConfigYaml))) tableManager := e2ecortex.NewTableManager("table-manager", ChunksStorageFlags, "") - ingester1 := e2ecortex.NewIngester("ingester-1", consul.NetworkHTTPEndpoint(), mergeFlags(ChunksStorageFlags, map[string]string{ + ingester := e2ecortex.NewIngester("ingester-1", consul.NetworkHTTPEndpoint(), mergeFlags(ChunksStorageFlags, map[string]string{ "-ingester.max-transfer-retries": "0", }), "") querier := e2ecortex.NewQuerier("querier", consul.NetworkHTTPEndpoint(), ChunksStorageFlags, "") distributor := e2ecortex.NewDistributor("distributor", consul.NetworkHTTPEndpoint(), ChunksStorageFlags, "") - require.NoError(t, s.StartAndWaitReady(distributor, querier, ingester1, tableManager)) + require.NoError(t, s.StartAndWaitReady(distributor, querier, ingester, tableManager)) // Wait until the first table-manager sync has completed, so that we're // sure the tables have been created. @@ -73,9 +73,12 @@ func TestIngesterFlushWithChunksStorage(t *testing.T) { require.Equal(t, model.ValVector, result.Type()) assert.Equal(t, expectedVector2, result.(model.Vector)) + // Ensure no service-specific metrics prefix is used by the wrong service. + assertServiceMetricsPrefixes(t, Ingester, ingester) + // Stop ingester-1, so that it will flush all chunks to the storage. This function will return // once the ingester-1 is successfully stopped, which means the flushing is completed. - require.NoError(t, s.Stop(ingester1)) + require.NoError(t, s.Stop(ingester)) // Ensure chunks have been uploaded to the storage (DynamoDB). dynamoURL := "dynamodb://u:p@" + dynamo.Endpoint(8000) @@ -94,4 +97,9 @@ func TestIngesterFlushWithChunksStorage(t *testing.T) { out, err = dynamoClient.Scan(&dynamodb.ScanInput{TableName: aws.String(chunksTable)}) require.NoError(t, err) assert.Equal(t, int64(2), *out.Count) + + // Ensure no service-specific metrics prefix is used by the wrong service. + assertServiceMetricsPrefixes(t, Distributor, distributor) + assertServiceMetricsPrefixes(t, Querier, querier) + assertServiceMetricsPrefixes(t, TableManager, tableManager) } diff --git a/integration/ingester_hand_over_test.go b/integration/ingester_hand_over_test.go index ef69f2e3a52..2385e7695d3 100644 --- a/integration/ingester_hand_over_test.go +++ b/integration/ingester_hand_over_test.go @@ -89,4 +89,9 @@ func runIngesterHandOverTest(t *testing.T, flags map[string]string, setup func(t result, err = c.Query("series_1", now) require.NoError(t, err) assert.Equal(t, expectedVector, result.(model.Vector)) + + // Ensure no service-specific metrics prefix is used by the wrong service. + assertServiceMetricsPrefixes(t, Distributor, distributor) + assertServiceMetricsPrefixes(t, Ingester, ingester2) + assertServiceMetricsPrefixes(t, Querier, querier) } diff --git a/integration/metrics_test.go b/integration/metrics_test.go deleted file mode 100644 index cf2d0dfdc4e..00000000000 --- a/integration/metrics_test.go +++ /dev/null @@ -1,112 +0,0 @@ -// +build requires_docker - -package main - -import ( - "strings" - "testing" - "time" - - "github.com/prometheus/common/model" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/cortexproject/cortex/integration/e2e" - e2edb "github.com/cortexproject/cortex/integration/e2e/db" - "github.com/cortexproject/cortex/integration/e2ecortex" -) - -func TestExportedMetrics(t *testing.T) { - s, err := e2e.NewScenario(networkName) - require.NoError(t, err) - defer s.Close() - - // Start dependencies. - dynamo := e2edb.NewDynamoDB() - consul := e2edb.NewConsul() - require.NoError(t, s.StartAndWaitReady(dynamo, consul)) - - // Start Cortex components. - require.NoError(t, writeFileToSharedDir(s, cortexSchemaConfigFile, []byte(cortexSchemaConfigYaml))) - - tableManager := e2ecortex.NewTableManager("table-manager", ChunksStorageFlags, "") - ingester := e2ecortex.NewIngester("ingester", consul.NetworkHTTPEndpoint(), ChunksStorageFlags, "") - distributor := e2ecortex.NewDistributor("distributor", consul.NetworkHTTPEndpoint(), ChunksStorageFlags, "") - queryFrontend := e2ecortex.NewQueryFrontend("query-frontend", ChunksStorageFlags, "") - require.NoError(t, s.StartAndWaitReady(distributor, queryFrontend, ingester, tableManager)) - - // Start the querier after the query-frontend otherwise we're not - // able to get the query-frontend network endpoint. - querier := e2ecortex.NewQuerier("querier", consul.NetworkHTTPEndpoint(), mergeFlags(ChunksStorageFlags, map[string]string{ - "-querier.frontend-address": queryFrontend.NetworkGRPCEndpoint(), - }), "") - require.NoError(t, s.StartAndWaitReady(querier)) - - // Wait until the first table-manager sync has completed, so that we're - // sure the tables have been created. - require.NoError(t, tableManager.WaitSumMetrics(e2e.Greater(0), "cortex_table_manager_sync_success_timestamp_seconds")) - - // Wait until both the distributor and querier have updated the ring. - require.NoError(t, distributor.WaitSumMetrics(e2e.Equals(512), "cortex_ring_tokens_total")) - require.NoError(t, querier.WaitSumMetrics(e2e.Equals(512), "cortex_ring_tokens_total")) - - // Push some series to Cortex (to hit the write path). - now := time.Now() - series, expectedVector := generateSeries("series_1", now) - - c, err := e2ecortex.NewClient(distributor.HTTPEndpoint(), "", "", "", "user-1") - require.NoError(t, err) - - res, err := c.Push(series) - require.NoError(t, err) - require.Equal(t, 200, res.StatusCode) - - // Query the series both from the querier and query-frontend (to hit the read path). - for _, endpoint := range []string{querier.HTTPEndpoint(), queryFrontend.HTTPEndpoint()} { - c, err := e2ecortex.NewClient("", endpoint, "", "", "user-1") - require.NoError(t, err) - - result, err := c.Query("series_1", now) - require.NoError(t, err) - require.Equal(t, model.ValVector, result.Type()) - assert.Equal(t, expectedVector, result.(model.Vector)) - } - - // For each Cortex service, ensure its service-specific metrics prefix is not used by metrics - // exported by other services. - services := map[*e2ecortex.CortexService][]string{ - distributor: []string{}, - ingester: []string{}, - querier: []string{}, - queryFrontend: []string{"cortex_frontend", "cortex_query_frontend"}, - tableManager: []string{}, - } - - for service, prefixes := range services { - if len(prefixes) == 0 { - continue - } - - // Assert the prefixes against all other services. - for target, _ := range services { - if service == target { - continue - } - - metrics, err := target.Metrics() - require.NoError(t, err) - - // Ensure no metric name matches the "reserved" prefixes. - for _, metricLine := range strings.Split(metrics, "\n") { - metricLine = strings.TrimSpace(metricLine) - if metricLine == "" || strings.HasPrefix(metricLine, "#") { - continue - } - - for _, prefix := range prefixes { - assert.NotRegexp(t, "^"+prefix, metricLine, "service: %s", target.Name()) - } - } - } - } -} diff --git a/integration/querier_test.go b/integration/querier_test.go index 6925ab5563f..ea6b0794f95 100644 --- a/integration/querier_test.go +++ b/integration/querier_test.go @@ -169,6 +169,11 @@ func TestQuerierWithBlocksStorage(t *testing.T) { } else if indexCacheBackend == tsdb.IndexCacheBackendMemcached { require.NoError(t, querier.WaitSumMetrics(e2e.Equals(11+2), "cortex_querier_blocks_index_cache_memcached_operations_total")) // as before + 2 gets } + + // Ensure no service-specific metrics prefix is used by the wrong service. + assertServiceMetricsPrefixes(t, Distributor, distributor) + assertServiceMetricsPrefixes(t, Ingester, ingester) + assertServiceMetricsPrefixes(t, Querier, querier) }) } } diff --git a/integration/query_frontend_test.go b/integration/query_frontend_test.go index f5b730641db..195fb620a2c 100644 --- a/integration/query_frontend_test.go +++ b/integration/query_frontend_test.go @@ -158,4 +158,10 @@ func runQueryFrontendTest(t *testing.T, setup queryFrontendSetup) { } wg.Wait() + + // Ensure no service-specific metrics prefix is used by the wrong service. + assertServiceMetricsPrefixes(t, Distributor, distributor) + assertServiceMetricsPrefixes(t, Ingester, ingester) + assertServiceMetricsPrefixes(t, Querier, querier) + assertServiceMetricsPrefixes(t, QueryFrontend, queryFrontend) } From b859beb049c3b58ede723c4844d2219ccef0451b Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Fri, 20 Mar 2020 12:13:52 +0100 Subject: [PATCH 2/5] Added alertmanager metric prefix to integration tests Signed-off-by: Marco Pracucci --- integration/alertmanager_test.go | 3 +++ integration/asserts.go | 21 ++++++++++++++------- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/integration/alertmanager_test.go b/integration/alertmanager_test.go index 6a0139954df..083d8324fa3 100644 --- a/integration/alertmanager_test.go +++ b/integration/alertmanager_test.go @@ -36,4 +36,7 @@ func TestAlertmanager(t *testing.T) { require.Equal(t, "example_groupby", cfg.Route.GroupByStr[0]) require.Len(t, cfg.Receivers, 1) require.Equal(t, "example_receiver", cfg.Receivers[0].Name) + + // Ensure no service-specific metrics prefix is used by the wrong service. + assertServiceMetricsPrefixes(t, AlertManager, alertmanager) } diff --git a/integration/asserts.go b/integration/asserts.go index 08fb4faab00..9fd449b1fdb 100644 --- a/integration/asserts.go +++ b/integration/asserts.go @@ -10,14 +10,15 @@ import ( "github.com/cortexproject/cortex/integration/e2ecortex" ) -type ServiceType string +type ServiceType int const ( - Distributor = ServiceType("distributor") - Ingester = ServiceType("ingester") - Querier = ServiceType("querier") - QueryFrontend = ServiceType("query-frontend") - TableManager = ServiceType("table-manager") + Distributor = iota + Ingester + Querier + QueryFrontend + TableManager + AlertManager ) var ( @@ -28,6 +29,12 @@ var ( Querier: []string{}, QueryFrontend: []string{"cortex_frontend", "cortex_query_frontend"}, TableManager: []string{}, + AlertManager: []string{"cortex_alertmanager"}, + } + + // Blacklisted metrics prefixes across any Cortex service. + blacklistedMetricsPrefixes = []string{ + "cortex_alert_manager", // It should be "cortex_alertmanager" } ) @@ -52,7 +59,7 @@ func assertServiceMetricsPrefixes(t *testing.T, serviceType ServiceType, service } func getBlacklistedMetricsPrefixesByService(serviceType ServiceType) []string { - blacklist := []string{} + blacklist := append([]string{}, blacklistedMetricsPrefixes...) // Add any service-specific metrics prefix excluding the service itself. for t, prefixes := range serviceMetricsPrefixes { From 5f97a93d0dd3a343e867c966e9e69a0bfd6f4e0d Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Fri, 20 Mar 2020 12:30:00 +0100 Subject: [PATCH 3/5] Fixed linter and improved tests Signed-off-by: Marco Pracucci --- Makefile | 2 +- integration/asserts.go | 2 ++ integration/querier_test.go | 8 +++++++- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 71566166378..ecd34df6076 100644 --- a/Makefile +++ b/Makefile @@ -123,7 +123,7 @@ protos: $(PROTO_GOS) lint: misspell -error docs - golangci-lint run --build-tags netgo --timeout=5m --enable golint --enable misspell --enable gofmt + golangci-lint run --build-tags netgo,require_docker --timeout=5m --enable golint --enable misspell --enable gofmt # Ensure no blacklisted package is imported. faillint -paths "github.com/bmizerany/assert=github.com/stretchr/testify/assert" ./pkg/... ./cmd/... ./tools/... ./integration/... diff --git a/integration/asserts.go b/integration/asserts.go index 9fd449b1fdb..f68007c2f87 100644 --- a/integration/asserts.go +++ b/integration/asserts.go @@ -1,3 +1,5 @@ +// +build requires_docker + package main import ( diff --git a/integration/querier_test.go b/integration/querier_test.go index ea6b0794f95..1374f01e0d0 100644 --- a/integration/querier_test.go +++ b/integration/querier_test.go @@ -36,7 +36,7 @@ func TestQuerierWithBlocksStorage(t *testing.T) { "-experimental.tsdb.bucket-store.index-cache.backend": "inmemory", }), }, - "queintegration/e2e/service.gorier running with memcached index cache": { + "querier running with memcached index cache": { flags: mergeFlags(BlocksStorageFlags, map[string]string{ // The address will be inject during the test execution because it's dynamic. "-experimental.tsdb.bucket-store.index-cache.backend": "memcached", @@ -344,4 +344,10 @@ func TestQuerierWithChunksStorage(t *testing.T) { } wg.Wait() + + // Ensure no service-specific metrics prefix is used by the wrong service. + assertServiceMetricsPrefixes(t, Distributor, distributor) + assertServiceMetricsPrefixes(t, Ingester, ingester) + assertServiceMetricsPrefixes(t, Querier, querier) + assertServiceMetricsPrefixes(t, TableManager, tableManager) } From ad2a6aa3a1cd7ad036a70fd59d5bab897a0980c0 Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Fri, 20 Mar 2020 12:49:15 +0100 Subject: [PATCH 4/5] Remove cortex_alertmanager prefix check (I've been too much optimistic) Signed-off-by: Marco Pracucci --- integration/asserts.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/asserts.go b/integration/asserts.go index f68007c2f87..e8d813a3a64 100644 --- a/integration/asserts.go +++ b/integration/asserts.go @@ -31,7 +31,7 @@ var ( Querier: []string{}, QueryFrontend: []string{"cortex_frontend", "cortex_query_frontend"}, TableManager: []string{}, - AlertManager: []string{"cortex_alertmanager"}, + AlertManager: []string{}, } // Blacklisted metrics prefixes across any Cortex service. From 4c50a6b1baec7c5b78df4e73ff9caa1b930fd5d1 Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Fri, 20 Mar 2020 17:38:19 +0100 Subject: [PATCH 5/5] Fixed integration tests after rebasing master Signed-off-by: Marco Pracucci --- integration/api_ruler_test.go | 3 +++ integration/asserts.go | 2 ++ integration/e2ecortex/services.go | 2 +- integration/querier_test.go | 4 ++-- 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/integration/api_ruler_test.go b/integration/api_ruler_test.go index b1f44a8c3c0..5acf3adfb96 100644 --- a/integration/api_ruler_test.go +++ b/integration/api_ruler_test.go @@ -70,4 +70,7 @@ func TestRulerAPI(t *testing.T) { // Check to ensure the rule groups are no longer active _, err = c.GetRuleGroups() require.Error(t, err) + + // Ensure no service-specific metrics prefix is used by the wrong service. + assertServiceMetricsPrefixes(t, Ruler, ruler) } diff --git a/integration/asserts.go b/integration/asserts.go index e8d813a3a64..fc4e77c4a94 100644 --- a/integration/asserts.go +++ b/integration/asserts.go @@ -21,6 +21,7 @@ const ( QueryFrontend TableManager AlertManager + Ruler ) var ( @@ -32,6 +33,7 @@ var ( QueryFrontend: []string{"cortex_frontend", "cortex_query_frontend"}, TableManager: []string{}, AlertManager: []string{}, + Ruler: []string{}, } // Blacklisted metrics prefixes across any Cortex service. diff --git a/integration/e2ecortex/services.go b/integration/e2ecortex/services.go index 0988acc98e4..6ab7c866800 100644 --- a/integration/e2ecortex/services.go +++ b/integration/e2ecortex/services.go @@ -224,7 +224,7 @@ func NewRuler(name string, flags map[string]string, image string) *CortexService "-log.level": "warn", }, flags))...), // The alertmanager doesn't expose a readiness probe, so we just check if the / returns 200 - e2e.NewReadinessProbe(httpPort, "/", 200), + e2e.NewHTTPReadinessProbe(httpPort, "/", 200), httpPort, grpcPort, ) diff --git a/integration/querier_test.go b/integration/querier_test.go index 1374f01e0d0..eaf1d2494fe 100644 --- a/integration/querier_test.go +++ b/integration/querier_test.go @@ -297,7 +297,7 @@ func TestQuerierWithChunksStorage(t *testing.T) { expectedVectors := make([]model.Vector, numUsers) for u := 0; u < numUsers; u++ { - c, err := e2ecortex.NewClient(distributor.HTTPEndpoint(), "", "", fmt.Sprintf("user-%d", u)) + c, err := e2ecortex.NewClient(distributor.HTTPEndpoint(), "", "", "", fmt.Sprintf("user-%d", u)) require.NoError(t, err) var series []prompb.TimeSeries @@ -328,7 +328,7 @@ func TestQuerierWithChunksStorage(t *testing.T) { for u := 0; u < numUsers; u++ { userID := u - c, err := e2ecortex.NewClient("", querier.HTTPEndpoint(), "", fmt.Sprintf("user-%d", userID)) + c, err := e2ecortex.NewClient("", querier.HTTPEndpoint(), "", "", fmt.Sprintf("user-%d", userID)) require.NoError(t, err) for q := 0; q < numQueriesPerUser; q++ {