From 3e7cbad383572495664d2cc85f825c6f1cd2cb10 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Mon, 8 Jan 2024 18:40:12 +0000 Subject: [PATCH] Fixed name and type we're testing for --- builtin/audit/file/backend.go | 2 +- builtin/audit/file/backend_test.go | 3 ++- builtin/audit/socket/backend.go | 2 +- builtin/audit/socket/backend_test.go | 3 ++- builtin/audit/syslog/backend.go | 2 +- builtin/audit/syslog/backend_test.go | 3 ++- .../observability/event/node_metrics_counter.go | 10 +++++++++- .../event/node_metrics_counter_test.go | 14 +++++++++++--- 8 files changed, 29 insertions(+), 10 deletions(-) diff --git a/builtin/audit/file/backend.go b/builtin/audit/file/backend.go index 5456a57416..bcef973d3d 100644 --- a/builtin/audit/file/backend.go +++ b/builtin/audit/file/backend.go @@ -552,7 +552,7 @@ func (b *Backend) configureSinkNode(name string, filePath string, mode string, f metricLabeler = &audit.MetricLabelerAuditSink{} } - sinkMetricCounter, err := event.NewMetricsCounter(sinkMetricTimer, metricLabeler) + sinkMetricCounter, err := event.NewMetricsCounter(sinkName, sinkMetricTimer, metricLabeler) if err != nil { return fmt.Errorf("%s: unable to add counting metrics to sink for path %q: %w", op, filePath, err) } diff --git a/builtin/audit/file/backend_test.go b/builtin/audit/file/backend_test.go index dc279d0b42..3016b40e37 100644 --- a/builtin/audit/file/backend_test.go +++ b/builtin/audit/file/backend_test.go @@ -15,6 +15,7 @@ import ( "github.com/hashicorp/eventlogger" "github.com/hashicorp/vault/audit" "github.com/hashicorp/vault/helper/namespace" + "github.com/hashicorp/vault/internal/observability/event" "github.com/hashicorp/vault/sdk/helper/salt" "github.com/hashicorp/vault/sdk/logical" "github.com/stretchr/testify/require" @@ -529,7 +530,7 @@ func TestBackend_configureSinkNode(t *testing.T) { id := b.nodeIDList[0] node := b.nodeMap[id] require.Equal(t, eventlogger.NodeTypeSink, node.Type()) - sw, ok := node.(*audit.SinkMetricTimer) + sw, ok := node.(*event.MetricsCounter) require.True(t, ok) require.Equal(t, tc.expectedName, sw.Name) } diff --git a/builtin/audit/socket/backend.go b/builtin/audit/socket/backend.go index c313ee1dcb..e1a2bc71f0 100644 --- a/builtin/audit/socket/backend.go +++ b/builtin/audit/socket/backend.go @@ -443,7 +443,7 @@ func (b *Backend) configureSinkNode(name string, address string, format string, metricLabeler = &audit.MetricLabelerAuditSink{} } - sinkMetricCounter, err := event.NewMetricsCounter(sinkMetricTimer, metricLabeler) + sinkMetricCounter, err := event.NewMetricsCounter(name, sinkMetricTimer, metricLabeler) if err != nil { return fmt.Errorf("%s: unable to add counting metrics to sink for path %q: %w", op, name, err) } diff --git a/builtin/audit/socket/backend_test.go b/builtin/audit/socket/backend_test.go index af781cf47d..edf9795e9f 100644 --- a/builtin/audit/socket/backend_test.go +++ b/builtin/audit/socket/backend_test.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/eventlogger" "github.com/hashicorp/vault/audit" + "github.com/hashicorp/vault/internal/observability/event" "github.com/hashicorp/vault/sdk/helper/salt" "github.com/hashicorp/vault/sdk/logical" "github.com/stretchr/testify/require" @@ -285,7 +286,7 @@ func TestBackend_configureSinkNode(t *testing.T) { id := b.nodeIDList[0] node := b.nodeMap[id] require.Equal(t, eventlogger.NodeTypeSink, node.Type()) - sw, ok := node.(*audit.SinkMetricTimer) + sw, ok := node.(*event.MetricsCounter) require.True(t, ok) require.Equal(t, tc.expectedName, sw.Name) } diff --git a/builtin/audit/syslog/backend.go b/builtin/audit/syslog/backend.go index 8698fb981f..4ba8f1ded1 100644 --- a/builtin/audit/syslog/backend.go +++ b/builtin/audit/syslog/backend.go @@ -353,7 +353,7 @@ func (b *Backend) configureSinkNode(name string, format string, opts ...event.Op metricLabeler = &audit.MetricLabelerAuditSink{} } - sinkMetricCounter, err := event.NewMetricsCounter(sinkMetricTimer, metricLabeler) + sinkMetricCounter, err := event.NewMetricsCounter(name, sinkMetricTimer, metricLabeler) if err != nil { return fmt.Errorf("%s: unable to add counting metrics to sink for path %q: %w", op, name, err) } diff --git a/builtin/audit/syslog/backend_test.go b/builtin/audit/syslog/backend_test.go index 05ff34293d..bf8aece142 100644 --- a/builtin/audit/syslog/backend_test.go +++ b/builtin/audit/syslog/backend_test.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/eventlogger" "github.com/hashicorp/vault/audit" + "github.com/hashicorp/vault/internal/observability/event" "github.com/hashicorp/vault/sdk/helper/salt" "github.com/hashicorp/vault/sdk/logical" "github.com/stretchr/testify/require" @@ -267,7 +268,7 @@ func TestBackend_configureSinkNode(t *testing.T) { id := b.nodeIDList[0] node := b.nodeMap[id] require.Equal(t, eventlogger.NodeTypeSink, node.Type()) - sw, ok := node.(*audit.SinkMetricTimer) + sw, ok := node.(*event.MetricsCounter) require.True(t, ok) require.Equal(t, tc.expectedName, sw.Name) } diff --git a/internal/observability/event/node_metrics_counter.go b/internal/observability/event/node_metrics_counter.go index aaf2578344..04dd1e74b0 100644 --- a/internal/observability/event/node_metrics_counter.go +++ b/internal/observability/event/node_metrics_counter.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "reflect" + "strings" "github.com/hashicorp/eventlogger" "github.com/hashicorp/go-metrics" @@ -16,6 +17,7 @@ var _ eventlogger.Node = (*MetricsCounter)(nil) // MetricsCounter offers a way for nodes to emit metrics which increment a label by 1. type MetricsCounter struct { + Name string Node eventlogger.Node labeler Labeler } @@ -28,9 +30,14 @@ type Labeler interface { } // NewMetricsCounter should be used to create the MetricsCounter. -func NewMetricsCounter(node eventlogger.Node, labeler Labeler) (*MetricsCounter, error) { +func NewMetricsCounter(name string, node eventlogger.Node, labeler Labeler) (*MetricsCounter, error) { const op = "event.NewMetricsCounter" + name = strings.TrimSpace(name) + if name == "" { + return nil, fmt.Errorf("%s: name is required: %w", op, ErrInvalidParameter) + } + if node == nil || reflect.ValueOf(node).IsNil() { return nil, fmt.Errorf("%s: node is required: %w", op, ErrInvalidParameter) } @@ -40,6 +47,7 @@ func NewMetricsCounter(node eventlogger.Node, labeler Labeler) (*MetricsCounter, } return &MetricsCounter{ + Name: name, Node: node, labeler: labeler, }, nil diff --git a/internal/observability/event/node_metrics_counter_test.go b/internal/observability/event/node_metrics_counter_test.go index 18475a7eca..dd6d56db58 100644 --- a/internal/observability/event/node_metrics_counter_test.go +++ b/internal/observability/event/node_metrics_counter_test.go @@ -35,12 +35,20 @@ func TestNewMetricsCounter(t *testing.T) { labeler: &testMetricsCounter{}, isErrorExpected: false, }, + "no-name": { + node: nil, + labeler: nil, + isErrorExpected: true, + expectedErrorMessage: "event.NewMetricsCounter: name is required: invalid parameter", + }, "no-node": { + name: "foo", node: nil, isErrorExpected: true, expectedErrorMessage: "event.NewMetricsCounter: node is required: invalid parameter", }, "no-labeler": { + name: "foo", node: &testEventLoggerNode{}, labeler: nil, isErrorExpected: true, @@ -52,7 +60,7 @@ func TestNewMetricsCounter(t *testing.T) { name := name tc := tc t.Run(name, func(t *testing.T) { - m, err := NewMetricsCounter(tc.node, tc.labeler) + m, err := NewMetricsCounter(tc.name, tc.node, tc.labeler) switch { case tc.isErrorExpected: @@ -84,6 +92,6 @@ func (t testEventLoggerNode) Type() eventlogger.NodeType { // testMetricsCounter is for testing and implements the event.Labeler interface. type testMetricsCounter struct{} -func (m *testMetricsCounter) Label(_ *eventlogger.Event, err error) string { - return "" +func (m *testMetricsCounter) Labels(_ *eventlogger.Event, err error) []string { + return []string{""} }