Skip to content

Commit

Permalink
Address PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
pcewing committed Jun 3, 2021
1 parent 111bf58 commit ac39e4b
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 16 deletions.
16 changes: 8 additions & 8 deletions agent/consul/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -569,15 +569,15 @@ func NewServer(config *Config, flat Deps) (*Server, error) {
WithStateProvider(s.fsm).
WithLogger(s.logger).
WithDatacenter(s.config.Datacenter).
WithReportingInterval(s.config.MetricsReportingInterval),
func() []serf.Member {
members, err := s.LANMembersAllSegments()
if err != nil {
return []serf.Member{}
}
WithReportingInterval(s.config.MetricsReportingInterval).
WithGetMembersFunc(func() []serf.Member {
members, err := s.LANMembersAllSegments()
if err != nil {
return []serf.Member{}
}

return members
},
return members
}),
)
if err != nil {
s.Shutdown()
Expand Down
19 changes: 15 additions & 4 deletions agent/consul/usagemetrics/usagemetrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,16 @@ var Gauges = []prometheus.GaugeDefinition{
},
}

type getMembersFunc func() []serf.Member

// Config holds the settings for various parameters for the
// UsageMetricsReporter
type Config struct {
logger hclog.Logger
metricLabels []metrics.Label
stateProvider StateProvider
tickerInterval time.Duration
getMembersFunc getMembersFunc
}

// WithDatacenter adds the datacenter as a label to all metrics emitted by the
Expand Down Expand Up @@ -72,14 +75,18 @@ func (c *Config) WithStateProvider(sp StateProvider) *Config {
return c
}

// WithGetMembersFunc specifies the function used to identify cluster members
func (c *Config) WithGetMembersFunc(fn getMembersFunc) *Config {
c.getMembersFunc = fn
return c
}

// StateProvider defines an inteface for retrieving a state.Store handle. In
// non-test code, this is satisfied by the fsm.FSM struct.
type StateProvider interface {
State() *state.Store
}

type getMembersFunc func() []serf.Member

// UsageMetricsReporter provides functionality for emitting usage metrics into
// the metrics stream. This makes it essentially a translation layer
// between the state store and metrics stream.
Expand All @@ -91,11 +98,15 @@ type UsageMetricsReporter struct {
getMembersFunc getMembersFunc
}

func NewUsageMetricsReporter(cfg *Config, getMembersFunc getMembersFunc) (*UsageMetricsReporter, error) {
func NewUsageMetricsReporter(cfg *Config) (*UsageMetricsReporter, error) {
if cfg.stateProvider == nil {
return nil, errors.New("must provide a StateProvider to usage reporter")
}

if cfg.getMembersFunc == nil {
return nil, errors.New("must provide a getMembersFunc to usage reporter")
}

if cfg.logger == nil {
cfg.logger = hclog.NewNullLogger()
}
Expand All @@ -110,7 +121,7 @@ func NewUsageMetricsReporter(cfg *Config, getMembersFunc getMembersFunc) (*Usage
stateProvider: cfg.stateProvider,
metricLabels: cfg.metricLabels,
tickerInterval: cfg.tickerInterval,
getMembersFunc: getMembersFunc,
getMembersFunc: cfg.getMembersFunc,
}

return u, nil
Expand Down
5 changes: 3 additions & 2 deletions agent/consul/usagemetrics/usagemetrics_oss_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ func TestUsageReporter_emitServiceUsage_OSS(t *testing.T) {
},
},
},
getMembersFunc: func() []serf.Member { return []serf.Member{} },
},
"nodes-and-services": {
modfiyStateStore: func(t *testing.T, s *state.Store) {
Expand Down Expand Up @@ -159,8 +160,8 @@ func TestUsageReporter_emitServiceUsage_OSS(t *testing.T) {
new(Config).
WithStateProvider(mockStateProvider).
WithLogger(testutil.Logger(t)).
WithDatacenter("dc1"),
tcase.getMembersFunc,
WithDatacenter("dc1").
WithGetMembersFunc(tcase.getMembersFunc),
)
require.NoError(t, err)

Expand Down
5 changes: 3 additions & 2 deletions agent/consul/usagemetrics/usagemetrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ func TestUsageReporter_Run_Nodes(t *testing.T) {
Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}},
},
},
getMembersFunc: func() []serf.Member { return []serf.Member{} },
},
"nodes": {
modfiyStateStore: func(t *testing.T, s *state.Store) {
Expand Down Expand Up @@ -104,8 +105,8 @@ func TestUsageReporter_Run_Nodes(t *testing.T) {
new(Config).
WithStateProvider(mockStateProvider).
WithLogger(testutil.Logger(t)).
WithDatacenter("dc1"),
tcase.getMembersFunc,
WithDatacenter("dc1").
WithGetMembersFunc(tcase.getMembersFunc),
)
require.NoError(t, err)

Expand Down

0 comments on commit ac39e4b

Please sign in to comment.