Fix a race caused by assignment to core.metricSink (#9560)

vault/core_metrics_test.go

@@ -8,7 +8,6 @@ import (
 
 	"github.com/armon/go-metrics"
 	logicalKv "github.com/hashicorp/vault-plugin-secrets-kv"
-	"github.com/hashicorp/vault/helper/metricsutil"
 	"github.com/hashicorp/vault/helper/namespace"
 	"github.com/hashicorp/vault/sdk/logical"
 )
@@ -142,15 +141,7 @@ func TestCoreMetrics_KvSecretGauge(t *testing.T) {
 }
 
 func TestCoreMetrics_KvSecretGaugeError(t *testing.T) {
-	core := TestCore(t)
-
-	// Replace metricSink before unsealing
-	inmemSink := metrics.NewInmemSink(
-		1000000*time.Hour,
-		2000000*time.Hour)
-	core.metricSink = metricsutil.NewClusterMetricSink("test-cluster", inmemSink)
-
-	testCoreUnsealed(t, core)
+	core, _, _, sink := TestCoreUnsealedWithMetrics(t)
 	ctx := namespace.RootContext(nil)
 
 	badKvMount := &kvMount{
@@ -162,7 +153,7 @@ func TestCoreMetrics_KvSecretGaugeError(t *testing.T) {
 
 	core.walkKvMountSecrets(ctx, badKvMount)
 
-	intervals := inmemSink.Data()
+	intervals := sink.Data()
 	// Test crossed an interval boundary, don't try to deal with it.
 	if len(intervals) > 1 {
 		t.Skip("Detected interval crossing.")

vault/identity_store_test.go

@@ -12,7 +12,6 @@ import (
 	uuid "github.com/hashicorp/go-uuid"
 	credGithub "github.com/hashicorp/vault/builtin/credential/github"
 	"github.com/hashicorp/vault/helper/identity"
-	"github.com/hashicorp/vault/helper/metricsutil"
 	"github.com/hashicorp/vault/helper/namespace"
 	"github.com/hashicorp/vault/helper/storagepacker"
 	"github.com/hashicorp/vault/sdk/logical"
@@ -664,16 +663,7 @@ func TestIdentityStore_NewEntityCounter(t *testing.T) {
 		t.Fatalf("err: %s", err)
 	}
 
-	c := TestCore(t)
-
-	// Replace metricSink before unsealing
-	// This results in some duplication from the normal test setup function.
-	inmemSink := metrics.NewInmemSink(
-		1000000*time.Hour,
-		2000000*time.Hour)
-	c.metricSink = metricsutil.NewClusterMetricSink("test-cluster", inmemSink)
-
-	testCoreUnsealed(t, c)
+	c, _, _, sink := TestCoreUnsealedWithMetrics(t)
 
 	meGH := &MountEntry{
 		Table:       credentialTableType,
@@ -705,12 +695,12 @@ func TestIdentityStore_NewEntityCounter(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	expectSingleCount(t, inmemSink, "identity.entity.creation")
+	expectSingleCount(t, sink, "identity.entity.creation")
 
 	_, err = is.CreateOrFetchEntity(ctx, alias)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	expectSingleCount(t, inmemSink, "identity.entity.creation")
+	expectSingleCount(t, sink, "identity.entity.creation")
 }

vault/request_handling_test.go

@@ -8,7 +8,6 @@ import (
 	"github.com/armon/go-metrics"
 	uuid "github.com/hashicorp/go-uuid"
 	credUserpass "github.com/hashicorp/vault/builtin/credential/userpass"
-	"github.com/hashicorp/vault/helper/metricsutil"
 	"github.com/hashicorp/vault/helper/namespace"
 	"github.com/hashicorp/vault/sdk/logical"
 )
@@ -206,15 +205,7 @@ func checkCounter(t *testing.T, inmemSink *metrics.InmemSink, keyPrefix string,
 }
 
 func TestRequestHandling_LoginMetric(t *testing.T) {
-	core := TestCore(t)
-
-	// Replace sink before unseal!
-	inmemSink := metrics.NewInmemSink(
-		1000000*time.Hour,
-		2000000*time.Hour)
-	core.metricSink = metricsutil.NewClusterMetricSink("test-cluster", inmemSink)
-
-	_, _, root := testCoreUnsealed(t, core)
+	core, _, root, sink := TestCoreUnsealedWithMetrics(t)
 
 	if err := core.loadMounts(namespace.RootContext(nil)); err != nil {
 		t.Fatalf("err: %v", err)
@@ -275,7 +266,7 @@ func TestRequestHandling_LoginMetric(t *testing.T) {
 	}
 
 	// There should be two counters
-	checkCounter(t, inmemSink, "token.creation",
+	checkCounter(t, sink, "token.creation",
 		map[string]string{
 			"cluster":      "test-cluster",
 			"namespace":    "root",
@@ -285,7 +276,7 @@ func TestRequestHandling_LoginMetric(t *testing.T) {
 			"token_type":   "service",
 		},
 	)
-	checkCounter(t, inmemSink, "token.creation",
+	checkCounter(t, sink, "token.creation",
 		map[string]string{
 			"cluster":      "test-cluster",
 			"namespace":    "root",
@@ -299,12 +290,7 @@ func TestRequestHandling_LoginMetric(t *testing.T) {
 }
 
 func TestRequestHandling_SecretLeaseMetric(t *testing.T) {
-	core, _, root := TestCoreUnsealed(t)
-
-	inmemSink := metrics.NewInmemSink(
-		1000000*time.Hour,
-		2000000*time.Hour)
-	core.metricSink = metricsutil.NewClusterMetricSink("test-cluster", inmemSink)
+	core, _, root, sink := TestCoreUnsealedWithMetrics(t)
 
 	// Create a key with a lease
 	req := logical.TestRequest(t, logical.UpdateOperation, "secret/foo")
@@ -330,7 +316,7 @@ func TestRequestHandling_SecretLeaseMetric(t *testing.T) {
 		t.Fatalf("bad: %#v", resp)
 	}
 
-	checkCounter(t, inmemSink, "secret.lease.creation",
+	checkCounter(t, sink, "secret.lease.creation",
 		map[string]string{
 			"cluster":       "test-cluster",
 			"namespace":     "root",

vault/testing.go

@@ -156,6 +156,7 @@ func TestCoreWithSealAndUI(t testing.T, opts *CoreConfig) *Core {
 	conf.LicensingConfig = opts.LicensingConfig
 	conf.DisableKeyEncodingChecks = opts.DisableKeyEncodingChecks
 	conf.MetricsHelper = opts.MetricsHelper
+	conf.MetricSink = opts.MetricSink
 
 	if opts.Logger != nil {
 		conf.Logger = opts.Logger
@@ -306,6 +307,17 @@ func TestCoreUnsealed(t testing.T) (*Core, [][]byte, string) {
 	return testCoreUnsealed(t, core)
 }
 
+func TestCoreUnsealedWithMetrics(t testing.T) (*Core, [][]byte, string, *metrics.InmemSink) {
+	t.Helper()
+	inmemSink := metrics.NewInmemSink(1000000*time.Hour, 2000000*time.Hour)
+	conf := &CoreConfig{
+		BuiltinRegistry: NewMockBuiltinRegistry(),
+		MetricSink:      metricsutil.NewClusterMetricSink("test-cluster", inmemSink),
+	}
+	core, keys, root := testCoreUnsealed(t, TestCoreWithSealAndUI(t, conf))
+	return core, keys, root, inmemSink
+}
+
 // TestCoreUnsealedRaw returns a pure in-memory core that is already
 // initialized, unsealed, and with raw endpoints enabled.
 func TestCoreUnsealedRaw(t testing.T) (*Core, [][]byte, string) {

vault/token_store_test.go

@@ -13,7 +13,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/armon/go-metrics"
 	"github.com/go-test/deep"
 	"github.com/hashicorp/errwrap"
 	"github.com/hashicorp/go-hclog"
@@ -2122,15 +2121,7 @@ func TestTokenStore_HandleRequest_CreateToken_TTL(t *testing.T) {
 }
 
 func TestTokenStore_HandleRequest_CreateToken_Metric(t *testing.T) {
-	c := TestCore(t)
-
-	// Replace metricSink before unsealing
-	inmemSink := metrics.NewInmemSink(
-		1000000*time.Hour,
-		2000000*time.Hour)
-	c.metricSink = metricsutil.NewClusterMetricSink("test-cluster", inmemSink)
-
-	_, _, root := testCoreUnsealed(t, c)
+	c, _, root, sink := TestCoreUnsealedWithMetrics(t)
 	ts := c.tokenStore
 
 	req := logical.TestRequest(t, logical.UpdateOperation, "create")
@@ -2144,7 +2135,7 @@ func TestTokenStore_HandleRequest_CreateToken_Metric(t *testing.T) {
 		t.Fatalf("bad: %#v", resp)
 	}
 
-	expectSingleCount(t, inmemSink, "token.creation")
+	expectSingleCount(t, sink, "token.creation")
 }
 
 func TestTokenStore_HandleRequest_Revoke(t *testing.T) {