From e44bce3c4f944dac21d1c328822b08a861db2dd6 Mon Sep 17 00:00:00 2001 From: "R.B. Boyer" <4903+rboyer@users.noreply.github.com> Date: Wed, 18 Aug 2021 09:27:15 -0500 Subject: [PATCH] state: partition the usage metrics subsystem (#10867) --- agent/consul/fsm/snapshot_oss_test.go | 4 +- agent/consul/state/usage.go | 57 +++-- agent/consul/state/usage_oss.go | 12 +- agent/consul/state/usage_oss_test.go | 25 -- agent/consul/state/usage_test.go | 40 +++- agent/consul/usagemetrics/usagemetrics.go | 67 ++---- agent/consul/usagemetrics/usagemetrics_oss.go | 38 +++ .../usagemetrics/usagemetrics_oss_test.go | 223 ++++++++++++++---- .../consul/usagemetrics/usagemetrics_test.go | 118 ++------- 9 files changed, 320 insertions(+), 264 deletions(-) delete mode 100644 agent/consul/state/usage_oss_test.go diff --git a/agent/consul/fsm/snapshot_oss_test.go b/agent/consul/fsm/snapshot_oss_test.go index 4675d87ca1..13d8bb90b9 100644 --- a/agent/consul/fsm/snapshot_oss_test.go +++ b/agent/consul/fsm/snapshot_oss_test.go @@ -692,9 +692,9 @@ func TestFSM_SnapshotRestore_OSS(t *testing.T) { require.Equal(t, fedState2, fedStateLoaded2) // Verify usage data is correctly updated - idx, nodeCount, err := fsm2.state.NodeCount() + idx, nodeUsage, err := fsm2.state.NodeUsage() require.NoError(t, err) - require.Equal(t, len(nodes), nodeCount) + require.Equal(t, len(nodes), nodeUsage.Nodes) require.NotZero(t, idx) // Verify system metadata is restored. diff --git a/agent/consul/state/usage.go b/agent/consul/state/usage.go index 536c49a6ca..5e1509766b 100644 --- a/agent/consul/state/usage.go +++ b/agent/consul/state/usage.go @@ -10,16 +10,18 @@ import ( const ( serviceNamesUsageTable = "service-names" + + tableUsage = "usage" ) // usageTableSchema returns a new table schema used for tracking various indexes // for the Raft log. func usageTableSchema() *memdb.TableSchema { return &memdb.TableSchema{ - Name: "usage", + Name: tableUsage, Indexes: map[string]*memdb.IndexSchema{ - "id": { - Name: "id", + indexID: { + Name: indexID, AllowMissing: false, Unique: true, Indexer: &memdb.StringFieldIndex{ @@ -46,6 +48,12 @@ type ServiceUsage struct { EnterpriseServiceUsage } +// NodeUsage contains all of the usage data related to nodes +type NodeUsage struct { + Nodes int + EnterpriseNodeUsage +} + type uniqueServiceState int const ( @@ -68,8 +76,10 @@ func updateUsage(tx WriteTxn, changes Changes) error { } switch change.Table { - case "nodes": + case tableNodes: usageDeltas[change.Table] += delta + addEnterpriseNodeUsage(usageDeltas, change) + case tableServices: svc := changeObject(change).(*structs.ServiceNode) usageDeltas[change.Table] += delta @@ -98,7 +108,8 @@ func updateUsage(tx WriteTxn, changes Changes) error { // This will happen when restoring from a snapshot, just take the max index // of the tables we are tracking. if idx == 0 { - idx = maxIndexTxn(tx, "nodes", tableServices) + // TODO(partitions? namespaces?) + idx = maxIndexTxn(tx, tableNodes, tableServices) } return writeUsageDeltas(tx, idx, usageDeltas) @@ -107,7 +118,10 @@ func updateUsage(tx WriteTxn, changes Changes) error { func updateServiceNameUsage(tx WriteTxn, usageDeltas map[string]int, serviceNameChanges map[structs.ServiceName]int) (map[structs.ServiceName]uniqueServiceState, error) { serviceStates := make(map[structs.ServiceName]uniqueServiceState, len(serviceNameChanges)) for svc, delta := range serviceNameChanges { - q := Query{Value: svc.Name, EnterpriseMeta: svc.EnterpriseMeta} + q := Query{ + Value: svc.Name, + EnterpriseMeta: svc.EnterpriseMeta, + } serviceIter, err := tx.Get(tableServices, indexService, q) if err != nil { return nil, err @@ -162,7 +176,7 @@ func serviceNameChanged(change memdb.Change) bool { // passed in will be recorded on the entry as well. func writeUsageDeltas(tx WriteTxn, idx uint64, usageDeltas map[string]int) error { for id, delta := range usageDeltas { - u, err := tx.First("usage", "id", id) + u, err := tx.First(tableUsage, indexID, id) if err != nil { return fmt.Errorf("failed to retrieve existing usage entry: %s", err) } @@ -175,7 +189,7 @@ func writeUsageDeltas(tx WriteTxn, idx uint64, usageDeltas map[string]int) error // large numbers. delta = 0 } - err := tx.Insert("usage", &UsageEntry{ + err := tx.Insert(tableUsage, &UsageEntry{ ID: id, Count: delta, Index: idx, @@ -192,7 +206,7 @@ func writeUsageDeltas(tx WriteTxn, idx uint64, usageDeltas map[string]int) error // large numbers. updated = 0 } - err := tx.Insert("usage", &UsageEntry{ + err := tx.Insert(tableUsage, &UsageEntry{ ID: id, Count: updated, Index: idx, @@ -205,17 +219,26 @@ func writeUsageDeltas(tx WriteTxn, idx uint64, usageDeltas map[string]int) error return nil } -// NodeCount returns the latest seen Raft index, a count of the number of nodes -// registered, and any errors. -func (s *Store) NodeCount() (uint64, int, error) { +// NodeUsage returns the latest seen Raft index, a compiled set of node usage +// data, and any errors. +func (s *Store) NodeUsage() (uint64, NodeUsage, error) { tx := s.db.ReadTxn() defer tx.Abort() - nodeUsage, err := firstUsageEntry(tx, "nodes") + nodes, err := firstUsageEntry(tx, tableNodes) if err != nil { - return 0, 0, fmt.Errorf("failed nodes lookup: %s", err) + return 0, NodeUsage{}, fmt.Errorf("failed nodes lookup: %s", err) } - return nodeUsage.Index, nodeUsage.Count, nil + + usage := NodeUsage{ + Nodes: nodes.Count, + } + results, err := compileEnterpriseNodeUsage(tx, usage) + if err != nil { + return 0, NodeUsage{}, fmt.Errorf("failed nodes lookup: %s", err) + } + + return nodes.Index, results, nil } // ServiceUsage returns the latest seen Raft index, a compiled set of service @@ -238,7 +261,7 @@ func (s *Store) ServiceUsage() (uint64, ServiceUsage, error) { ServiceInstances: serviceInstances.Count, Services: services.Count, } - results, err := compileEnterpriseUsage(tx, usage) + results, err := compileEnterpriseServiceUsage(tx, usage) if err != nil { return 0, ServiceUsage{}, fmt.Errorf("failed services lookup: %s", err) } @@ -247,7 +270,7 @@ func (s *Store) ServiceUsage() (uint64, ServiceUsage, error) { } func firstUsageEntry(tx ReadTxn, id string) (*UsageEntry, error) { - usage, err := tx.First("usage", "id", id) + usage, err := tx.First(tableUsage, indexID, id) if err != nil { return nil, err } diff --git a/agent/consul/state/usage_oss.go b/agent/consul/state/usage_oss.go index 355f1fbcc4..c45bf74d56 100644 --- a/agent/consul/state/usage_oss.go +++ b/agent/consul/state/usage_oss.go @@ -3,16 +3,24 @@ package state import ( - "github.com/hashicorp/consul/agent/structs" memdb "github.com/hashicorp/go-memdb" + + "github.com/hashicorp/consul/agent/structs" ) type EnterpriseServiceUsage struct{} +type EnterpriseNodeUsage struct{} + +func addEnterpriseNodeUsage(map[string]int, memdb.Change) {} func addEnterpriseServiceInstanceUsage(map[string]int, memdb.Change) {} func addEnterpriseServiceUsage(map[string]int, map[structs.ServiceName]uniqueServiceState) {} -func compileEnterpriseUsage(tx ReadTxn, usage ServiceUsage) (ServiceUsage, error) { +func compileEnterpriseServiceUsage(tx ReadTxn, usage ServiceUsage) (ServiceUsage, error) { + return usage, nil +} + +func compileEnterpriseNodeUsage(tx ReadTxn, usage NodeUsage) (NodeUsage, error) { return usage, nil } diff --git a/agent/consul/state/usage_oss_test.go b/agent/consul/state/usage_oss_test.go deleted file mode 100644 index b441c71635..0000000000 --- a/agent/consul/state/usage_oss_test.go +++ /dev/null @@ -1,25 +0,0 @@ -// +build !consulent - -package state - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func TestStateStore_Usage_ServiceUsage(t *testing.T) { - s := testStateStore(t) - - testRegisterNode(t, s, 0, "node1") - testRegisterNode(t, s, 1, "node2") - testRegisterService(t, s, 8, "node1", "service1") - testRegisterService(t, s, 9, "node2", "service1") - testRegisterService(t, s, 10, "node2", "service2") - - idx, usage, err := s.ServiceUsage() - require.NoError(t, err) - require.Equal(t, idx, uint64(10)) - require.Equal(t, 2, usage.Services) - require.Equal(t, 3, usage.ServiceInstances) -} diff --git a/agent/consul/state/usage_test.go b/agent/consul/state/usage_test.go index 1a0e8c13f5..2d650a5910 100644 --- a/agent/consul/state/usage_test.go +++ b/agent/consul/state/usage_test.go @@ -9,40 +9,40 @@ import ( "github.com/hashicorp/consul/agent/structs" ) -func TestStateStore_Usage_NodeCount(t *testing.T) { +func TestStateStore_Usage_NodeUsage(t *testing.T) { s := testStateStore(t) // No nodes have been registered, and thus no usage entry exists - idx, count, err := s.NodeCount() + idx, usage, err := s.NodeUsage() require.NoError(t, err) require.Equal(t, idx, uint64(0)) - require.Equal(t, count, 0) + require.Equal(t, usage.Nodes, 0) testRegisterNode(t, s, 0, "node1") testRegisterNode(t, s, 1, "node2") - idx, count, err = s.NodeCount() + idx, usage, err = s.NodeUsage() require.NoError(t, err) require.Equal(t, idx, uint64(1)) - require.Equal(t, count, 2) + require.Equal(t, usage.Nodes, 2) } -func TestStateStore_Usage_NodeCount_Delete(t *testing.T) { +func TestStateStore_Usage_NodeUsage_Delete(t *testing.T) { s := testStateStore(t) testRegisterNode(t, s, 0, "node1") testRegisterNode(t, s, 1, "node2") - idx, count, err := s.NodeCount() + idx, usage, err := s.NodeUsage() require.NoError(t, err) require.Equal(t, idx, uint64(1)) - require.Equal(t, count, 2) + require.Equal(t, usage.Nodes, 2) require.NoError(t, s.DeleteNode(2, "node2", nil)) - idx, count, err = s.NodeCount() + idx, usage, err = s.NodeUsage() require.NoError(t, err) require.Equal(t, idx, uint64(2)) - require.Equal(t, count, 1) + require.Equal(t, usage.Nodes, 1) } func TestStateStore_Usage_ServiceUsageEmpty(t *testing.T) { @@ -56,6 +56,22 @@ func TestStateStore_Usage_ServiceUsageEmpty(t *testing.T) { require.Equal(t, usage.ServiceInstances, 0) } +func TestStateStore_Usage_ServiceUsage(t *testing.T) { + s := testStateStore(t) + + testRegisterNode(t, s, 0, "node1") + testRegisterNode(t, s, 1, "node2") + testRegisterService(t, s, 8, "node1", "service1") + testRegisterService(t, s, 9, "node2", "service1") + testRegisterService(t, s, 10, "node2", "service2") + + idx, usage, err := s.ServiceUsage() + require.NoError(t, err) + require.Equal(t, idx, uint64(10)) + require.Equal(t, 2, usage.Services) + require.Equal(t, 3, usage.ServiceInstances) +} + func TestStateStore_Usage_ServiceUsage_DeleteNode(t *testing.T) { s := testStateStore(t) testRegisterNode(t, s, 1, "node1") @@ -116,10 +132,10 @@ func TestStateStore_Usage_Restore(t *testing.T) { }) require.NoError(t, restore.Commit()) - idx, count, err := s.NodeCount() + idx, nodeUsage, err := s.NodeUsage() require.NoError(t, err) require.Equal(t, idx, uint64(9)) - require.Equal(t, count, 1) + require.Equal(t, nodeUsage.Nodes, 1) idx, usage, err := s.ServiceUsage() require.NoError(t, err) diff --git a/agent/consul/usagemetrics/usagemetrics.go b/agent/consul/usagemetrics/usagemetrics.go index 353e9a45df..9669125c66 100644 --- a/agent/consul/usagemetrics/usagemetrics.go +++ b/agent/consul/usagemetrics/usagemetrics.go @@ -8,10 +8,11 @@ import ( "github.com/armon/go-metrics/prometheus" "github.com/armon/go-metrics" - "github.com/hashicorp/consul/agent/consul/state" - "github.com/hashicorp/consul/logging" "github.com/hashicorp/go-hclog" "github.com/hashicorp/serf/serf" + + "github.com/hashicorp/consul/agent/consul/state" + "github.com/hashicorp/consul/logging" ) var Gauges = []prometheus.GaugeDefinition{ @@ -145,15 +146,13 @@ func (u *UsageMetricsReporter) Run(ctx context.Context) { func (u *UsageMetricsReporter) runOnce() { state := u.stateProvider.State() - _, nodes, err := state.NodeCount() + + _, nodeUsage, err := state.NodeUsage() if err != nil { u.logger.Warn("failed to retrieve nodes from state store", "error", err) } - metrics.SetGaugeWithLabels( - []string{"consul", "state", "nodes"}, - float32(nodes), - u.metricLabels, - ) + + u.emitNodeUsage(nodeUsage) _, serviceUsage, err := state.ServiceUsage() if err != nil { @@ -162,65 +161,27 @@ func (u *UsageMetricsReporter) runOnce() { u.emitServiceUsage(serviceUsage) - servers, clients := u.memberUsage() - u.emitMemberUsage(servers, clients) + members := u.memberUsage() + u.emitMemberUsage(members) } -func (u *UsageMetricsReporter) memberUsage() (int, map[string]int) { +func (u *UsageMetricsReporter) memberUsage() []serf.Member { if u.getMembersFunc == nil { - return 0, nil + return nil } mems := u.getMembersFunc() if len(mems) <= 0 { u.logger.Warn("cluster reported zero members") - return 0, nil } - servers := 0 - clients := make(map[string]int) - + out := make([]serf.Member, 0, len(mems)) for _, m := range mems { if m.Status != serf.StatusAlive { continue } - - switch m.Tags["role"] { - case "node": - clients[m.Tags["segment"]]++ - case "consul": - servers++ - } + out = append(out, m) } - return servers, clients -} - -func (u *UsageMetricsReporter) emitMemberUsage(servers int, clients map[string]int) { - totalClients := 0 - - for seg, c := range clients { - segmentLabel := metrics.Label{Name: "segment", Value: seg} - labels := append([]metrics.Label{segmentLabel}, u.metricLabels...) - - metrics.SetGaugeWithLabels( - []string{"consul", "members", "clients"}, - float32(c), - labels, - ) - - totalClients += c - } - - metrics.SetGaugeWithLabels( - []string{"consul", "members", "clients"}, - float32(totalClients), - u.metricLabels, - ) - - metrics.SetGaugeWithLabels( - []string{"consul", "members", "servers"}, - float32(servers), - u.metricLabels, - ) + return out } diff --git a/agent/consul/usagemetrics/usagemetrics_oss.go b/agent/consul/usagemetrics/usagemetrics_oss.go index 1cf7e6bda2..68c8852117 100644 --- a/agent/consul/usagemetrics/usagemetrics_oss.go +++ b/agent/consul/usagemetrics/usagemetrics_oss.go @@ -4,9 +4,47 @@ package usagemetrics import ( "github.com/armon/go-metrics" + + "github.com/hashicorp/serf/serf" + "github.com/hashicorp/consul/agent/consul/state" ) +func (u *UsageMetricsReporter) emitNodeUsage(nodeUsage state.NodeUsage) { + metrics.SetGaugeWithLabels( + []string{"consul", "state", "nodes"}, + float32(nodeUsage.Nodes), + u.metricLabels, + ) +} + +func (u *UsageMetricsReporter) emitMemberUsage(members []serf.Member) { + var ( + servers int + clients int + ) + for _, m := range members { + switch m.Tags["role"] { + case "node": + clients++ + case "consul": + servers++ + } + } + + metrics.SetGaugeWithLabels( + []string{"consul", "members", "clients"}, + float32(clients), + u.metricLabels, + ) + + metrics.SetGaugeWithLabels( + []string{"consul", "members", "servers"}, + float32(servers), + u.metricLabels, + ) +} + func (u *UsageMetricsReporter) emitServiceUsage(serviceUsage state.ServiceUsage) { metrics.SetGaugeWithLabels( []string{"consul", "state", "services"}, diff --git a/agent/consul/usagemetrics/usagemetrics_oss_test.go b/agent/consul/usagemetrics/usagemetrics_oss_test.go index e232014358..810c05f3cb 100644 --- a/agent/consul/usagemetrics/usagemetrics_oss_test.go +++ b/agent/consul/usagemetrics/usagemetrics_oss_test.go @@ -9,16 +9,151 @@ import ( "github.com/armon/go-metrics" "github.com/stretchr/testify/require" + "github.com/hashicorp/serf/serf" + "github.com/hashicorp/consul/agent/consul/state" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/sdk/testutil" - "github.com/hashicorp/serf/serf" ) func newStateStore() (*state.Store, error) { return state.NewStateStore(nil), nil } +func TestUsageReporter_emitNodeUsage_OSS(t *testing.T) { + type testCase struct { + modfiyStateStore func(t *testing.T, s *state.Store) + getMembersFunc getMembersFunc + expectedGauges map[string]metrics.GaugeValue + } + cases := map[string]testCase{ + "empty-state": { + expectedGauges: map[string]metrics.GaugeValue{ + // --- node --- + "consul.usage.test.consul.state.nodes;datacenter=dc1": { + Name: "consul.usage.test.consul.state.nodes", + Value: 0, + Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, + }, + // --- member --- + "consul.usage.test.consul.members.clients;datacenter=dc1": { + Name: "consul.usage.test.consul.members.clients", + Value: 0, + Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, + }, + "consul.usage.test.consul.members.servers;datacenter=dc1": { + Name: "consul.usage.test.consul.members.servers", + Value: 0, + Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, + }, + // --- service --- + "consul.usage.test.consul.state.services;datacenter=dc1": { + Name: "consul.usage.test.consul.state.services", + Value: 0, + Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, + }, + "consul.usage.test.consul.state.service_instances;datacenter=dc1": { + Name: "consul.usage.test.consul.state.service_instances", + Value: 0, + Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, + }, + }, + getMembersFunc: func() []serf.Member { return []serf.Member{} }, + }, + "nodes": { + modfiyStateStore: func(t *testing.T, s *state.Store) { + require.NoError(t, s.EnsureNode(1, &structs.Node{Node: "foo", Address: "127.0.0.1"})) + require.NoError(t, s.EnsureNode(2, &structs.Node{Node: "bar", Address: "127.0.0.2"})) + require.NoError(t, s.EnsureNode(3, &structs.Node{Node: "baz", Address: "127.0.0.2"})) + }, + getMembersFunc: func() []serf.Member { + return []serf.Member{ + { + Name: "foo", + Tags: map[string]string{"role": "consul"}, + Status: serf.StatusAlive, + }, + { + Name: "bar", + Tags: map[string]string{"role": "consul"}, + Status: serf.StatusAlive, + }, + { + Name: "baz", + Tags: map[string]string{"role": "node"}, + Status: serf.StatusAlive, + }, + } + }, + expectedGauges: map[string]metrics.GaugeValue{ + // --- node --- + "consul.usage.test.consul.state.nodes;datacenter=dc1": { + Name: "consul.usage.test.consul.state.nodes", + Value: 3, + Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, + }, + // --- member --- + "consul.usage.test.consul.members.servers;datacenter=dc1": { + Name: "consul.usage.test.consul.members.servers", + Value: 2, + Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, + }, + "consul.usage.test.consul.members.clients;datacenter=dc1": { + Name: "consul.usage.test.consul.members.clients", + Value: 1, + Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, + }, + // --- service --- + "consul.usage.test.consul.state.services;datacenter=dc1": { + Name: "consul.usage.test.consul.state.services", + Value: 0, + Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, + }, + "consul.usage.test.consul.state.service_instances;datacenter=dc1": { + Name: "consul.usage.test.consul.state.service_instances", + Value: 0, + Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, + }, + }, + }, + } + + for name, tcase := range cases { + t.Run(name, func(t *testing.T) { + // Only have a single interval for the test + sink := metrics.NewInmemSink(1*time.Minute, 1*time.Minute) + cfg := metrics.DefaultConfig("consul.usage.test") + cfg.EnableHostname = false + metrics.NewGlobal(cfg, sink) + + mockStateProvider := &mockStateProvider{} + s, err := newStateStore() + require.NoError(t, err) + if tcase.modfiyStateStore != nil { + tcase.modfiyStateStore(t, s) + } + mockStateProvider.On("State").Return(s) + + reporter, err := NewUsageMetricsReporter( + new(Config). + WithStateProvider(mockStateProvider). + WithLogger(testutil.Logger(t)). + WithDatacenter("dc1"). + WithGetMembersFunc(tcase.getMembersFunc), + ) + require.NoError(t, err) + + reporter.runOnce() + + intervals := sink.Data() + require.Len(t, intervals, 1) + intv := intervals[0] + + assertEqualGaugeMaps(t, tcase.expectedGauges, intv.Gauges) + }) + } +} + func TestUsageReporter_emitServiceUsage_OSS(t *testing.T) { type testCase struct { modfiyStateStore func(t *testing.T, s *state.Store) @@ -28,11 +163,28 @@ func TestUsageReporter_emitServiceUsage_OSS(t *testing.T) { cases := map[string]testCase{ "empty-state": { expectedGauges: map[string]metrics.GaugeValue{ + // --- node --- "consul.usage.test.consul.state.nodes;datacenter=dc1": { Name: "consul.usage.test.consul.state.nodes", Value: 0, Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, }, + // --- member --- + "consul.usage.test.consul.members.servers;datacenter=dc1": { + Name: "consul.usage.test.consul.members.servers", + Value: 0, + Labels: []metrics.Label{ + {Name: "datacenter", Value: "dc1"}, + }, + }, + "consul.usage.test.consul.members.clients;datacenter=dc1": { + Name: "consul.usage.test.consul.members.clients", + Value: 0, + Labels: []metrics.Label{ + {Name: "datacenter", Value: "dc1"}, + }, + }, + // --- service --- "consul.usage.test.consul.state.services;datacenter=dc1": { Name: "consul.usage.test.consul.state.services", Value: 0, @@ -47,35 +199,21 @@ func TestUsageReporter_emitServiceUsage_OSS(t *testing.T) { {Name: "datacenter", Value: "dc1"}, }, }, - "consul.usage.test.consul.members.clients;datacenter=dc1": { - Name: "consul.usage.test.consul.members.clients", - Value: 0, - Labels: []metrics.Label{ - {Name: "datacenter", Value: "dc1"}, - }, - }, - "consul.usage.test.consul.members.servers;datacenter=dc1": { - Name: "consul.usage.test.consul.members.servers", - Value: 0, - Labels: []metrics.Label{ - {Name: "datacenter", Value: "dc1"}, - }, - }, }, getMembersFunc: func() []serf.Member { return []serf.Member{} }, }, "nodes-and-services": { modfiyStateStore: func(t *testing.T, s *state.Store) { - require.Nil(t, s.EnsureNode(1, &structs.Node{Node: "foo", Address: "127.0.0.1"})) - require.Nil(t, s.EnsureNode(2, &structs.Node{Node: "bar", Address: "127.0.0.2"})) - require.Nil(t, s.EnsureNode(3, &structs.Node{Node: "baz", Address: "127.0.0.2"})) - require.Nil(t, s.EnsureNode(4, &structs.Node{Node: "qux", Address: "127.0.0.3"})) + require.NoError(t, s.EnsureNode(1, &structs.Node{Node: "foo", Address: "127.0.0.1"})) + require.NoError(t, s.EnsureNode(2, &structs.Node{Node: "bar", Address: "127.0.0.2"})) + require.NoError(t, s.EnsureNode(3, &structs.Node{Node: "baz", Address: "127.0.0.2"})) + require.NoError(t, s.EnsureNode(4, &structs.Node{Node: "qux", Address: "127.0.0.3"})) // Typical services and some consul services spread across two nodes - require.Nil(t, s.EnsureService(5, "foo", &structs.NodeService{ID: "db", Service: "db", Tags: nil, Address: "", Port: 5000})) - require.Nil(t, s.EnsureService(6, "bar", &structs.NodeService{ID: "api", Service: "api", Tags: nil, Address: "", Port: 5000})) - require.Nil(t, s.EnsureService(7, "foo", &structs.NodeService{ID: "consul", Service: "consul", Tags: nil})) - require.Nil(t, s.EnsureService(8, "bar", &structs.NodeService{ID: "consul", Service: "consul", Tags: nil})) + require.NoError(t, s.EnsureService(5, "foo", &structs.NodeService{ID: "db", Service: "db", Tags: nil, Address: "", Port: 5000})) + require.NoError(t, s.EnsureService(6, "bar", &structs.NodeService{ID: "api", Service: "api", Tags: nil, Address: "", Port: 5000})) + require.NoError(t, s.EnsureService(7, "foo", &structs.NodeService{ID: "consul", Service: "consul", Tags: nil})) + require.NoError(t, s.EnsureService(8, "bar", &structs.NodeService{ID: "consul", Service: "consul", Tags: nil})) }, getMembersFunc: func() []serf.Member { return []serf.Member{ @@ -102,21 +240,16 @@ func TestUsageReporter_emitServiceUsage_OSS(t *testing.T) { } }, expectedGauges: map[string]metrics.GaugeValue{ + // --- node --- "consul.usage.test.consul.state.nodes;datacenter=dc1": { Name: "consul.usage.test.consul.state.nodes", Value: 4, Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, }, - "consul.usage.test.consul.state.services;datacenter=dc1": { - Name: "consul.usage.test.consul.state.services", - Value: 3, - Labels: []metrics.Label{ - {Name: "datacenter", Value: "dc1"}, - }, - }, - "consul.usage.test.consul.state.service_instances;datacenter=dc1": { - Name: "consul.usage.test.consul.state.service_instances", - Value: 4, + // --- member --- + "consul.usage.test.consul.members.servers;datacenter=dc1": { + Name: "consul.usage.test.consul.members.servers", + Value: 2, Labels: []metrics.Label{ {Name: "datacenter", Value: "dc1"}, }, @@ -128,26 +261,18 @@ func TestUsageReporter_emitServiceUsage_OSS(t *testing.T) { {Name: "datacenter", Value: "dc1"}, }, }, - "consul.usage.test.consul.members.servers;datacenter=dc1": { - Name: "consul.usage.test.consul.members.servers", - Value: 2, + // --- service --- + "consul.usage.test.consul.state.services;datacenter=dc1": { + Name: "consul.usage.test.consul.state.services", + Value: 3, Labels: []metrics.Label{ {Name: "datacenter", Value: "dc1"}, }, }, - "consul.usage.test.consul.members.clients;segment=a;datacenter=dc1": { - Name: "consul.usage.test.consul.members.clients", - Value: 1, + "consul.usage.test.consul.state.service_instances;datacenter=dc1": { + Name: "consul.usage.test.consul.state.service_instances", + Value: 4, Labels: []metrics.Label{ - {Name: "segment", Value: "a"}, - {Name: "datacenter", Value: "dc1"}, - }, - }, - "consul.usage.test.consul.members.clients;segment=b;datacenter=dc1": { - Name: "consul.usage.test.consul.members.clients", - Value: 1, - Labels: []metrics.Label{ - {Name: "segment", Value: "b"}, {Name: "datacenter", Value: "dc1"}, }, }, @@ -185,7 +310,7 @@ func TestUsageReporter_emitServiceUsage_OSS(t *testing.T) { require.Len(t, intervals, 1) intv := intervals[0] - require.Equal(t, tcase.expectedGauges, intv.Gauges) + assertEqualGaugeMaps(t, tcase.expectedGauges, intv.Gauges) }) } } diff --git a/agent/consul/usagemetrics/usagemetrics_test.go b/agent/consul/usagemetrics/usagemetrics_test.go index 1c4be1d5b1..cf0cd01e01 100644 --- a/agent/consul/usagemetrics/usagemetrics_test.go +++ b/agent/consul/usagemetrics/usagemetrics_test.go @@ -2,16 +2,12 @@ package usagemetrics import ( "testing" - "time" "github.com/armon/go-metrics" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" "github.com/hashicorp/consul/agent/consul/state" - "github.com/hashicorp/consul/agent/structs" - "github.com/hashicorp/consul/sdk/testutil" - "github.com/hashicorp/serf/serf" ) type mockStateProvider struct { @@ -23,106 +19,20 @@ func (m *mockStateProvider) State() *state.Store { return retValues.Get(0).(*state.Store) } -func TestUsageReporter_Run_Nodes(t *testing.T) { - type testCase struct { - modfiyStateStore func(t *testing.T, s *state.Store) - getMembersFunc getMembersFunc - expectedGauges map[string]metrics.GaugeValue - } - cases := map[string]testCase{ - "empty-state": { - expectedGauges: map[string]metrics.GaugeValue{ - "consul.usage.test.consul.state.nodes;datacenter=dc1": { - Name: "consul.usage.test.consul.state.nodes", - Value: 0, - Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, - }, - }, - getMembersFunc: func() []serf.Member { return []serf.Member{} }, - }, - "nodes": { - modfiyStateStore: func(t *testing.T, s *state.Store) { - require.Nil(t, s.EnsureNode(1, &structs.Node{Node: "foo", Address: "127.0.0.1"})) - require.Nil(t, s.EnsureNode(2, &structs.Node{Node: "bar", Address: "127.0.0.2"})) - require.Nil(t, s.EnsureNode(3, &structs.Node{Node: "baz", Address: "127.0.0.2"})) - }, - getMembersFunc: func() []serf.Member { - return []serf.Member{ - { - Name: "foo", - Tags: map[string]string{"role": "consul"}, - Status: serf.StatusAlive, - }, - { - Name: "bar", - Tags: map[string]string{"role": "consul"}, - Status: serf.StatusAlive, - }, - { - Name: "baz", - Tags: map[string]string{"role": "node"}, - Status: serf.StatusAlive, - }, - } - }, - expectedGauges: map[string]metrics.GaugeValue{ - "consul.usage.test.consul.state.nodes;datacenter=dc1": { - Name: "consul.usage.test.consul.state.nodes", - Value: 3, - Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, - }, - "consul.usage.test.consul.members.clients;datacenter=dc1": { - Name: "consul.usage.test.consul.members.clients", - Value: 1, - Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, - }, - "consul.usage.test.consul.members.servers;datacenter=dc1": { - Name: "consul.usage.test.consul.members.servers", - Value: 2, - Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, - }, - }, - }, +func assertEqualGaugeMaps(t *testing.T, expectedMap, foundMap map[string]metrics.GaugeValue) { + t.Helper() + + for key := range foundMap { + if _, ok := expectedMap[key]; !ok { + t.Errorf("found unexpected gauge key: %s", key) + } } - for name, tcase := range cases { - t.Run(name, func(t *testing.T) { - // Only have a single interval for the test - sink := metrics.NewInmemSink(1*time.Minute, 1*time.Minute) - cfg := metrics.DefaultConfig("consul.usage.test") - cfg.EnableHostname = false - metrics.NewGlobal(cfg, sink) - - mockStateProvider := &mockStateProvider{} - s, err := newStateStore() - require.NoError(t, err) - if tcase.modfiyStateStore != nil { - tcase.modfiyStateStore(t, s) - } - mockStateProvider.On("State").Return(s) - - reporter, err := NewUsageMetricsReporter( - new(Config). - WithStateProvider(mockStateProvider). - WithLogger(testutil.Logger(t)). - WithDatacenter("dc1"). - WithGetMembersFunc(tcase.getMembersFunc), - ) - require.NoError(t, err) - - reporter.runOnce() - - intervals := sink.Data() - require.Len(t, intervals, 1) - intv := intervals[0] - - // Range over the expected values instead of just doing an Equal - // comparison on the maps because of different metrics emitted between - // OSS and Ent. The enterprise and OSS tests have a full equality - // comparison on the maps. - for key, expected := range tcase.expectedGauges { - require.Equal(t, expected, intv.Gauges[key]) - } - }) + for key, expected := range expectedMap { + if _, ok := foundMap[key]; !ok { + t.Errorf("did not find expected gauge key: %s", key) + continue + } + assert.Equal(t, expected, foundMap[key], "gauge key mismatch on %q", key) } }