Merge pull request #2156 from josephschorr/missing-label-check

Add missing service label in metrics for consistency

pkg/cmd/server/defaults.go

@@ -218,6 +218,7 @@ func (m MiddlewareOption) WithDatastoreMiddleware(middleware Middleware) Middlew
 		EnableRequestLog:          m.EnableRequestLog,
 		EnableResponseLog:         m.EnableResponseLog,
 		DisableGRPCHistogram:      m.DisableGRPCHistogram,
+		MiddlewareServiceLabel:    m.MiddlewareServiceLabel,
 		unaryDatastoreMiddleware:  &unary,
 		streamDatastoreMiddleware: &stream,
 	}
@@ -244,6 +245,7 @@ func (m MiddlewareOption) WithDatastore(ds datastore.Datastore) MiddlewareOption
 		EnableRequestLog:          m.EnableRequestLog,
 		EnableResponseLog:         m.EnableResponseLog,
 		DisableGRPCHistogram:      m.DisableGRPCHistogram,
+		MiddlewareServiceLabel:    m.MiddlewareServiceLabel,
 		unaryDatastoreMiddleware:  &unary,
 		streamDatastoreMiddleware: &stream,
 	}

pkg/cmd/server/defaults_test.go

@@ -0,0 +1,94 @@
+package server
+
+import (
+	"context"
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/rs/zerolog"
+	"github.com/stretchr/testify/require"
+
+	"github.com/authzed/spicedb/internal/datastore/memdb"
+	"github.com/authzed/spicedb/internal/dispatch"
+	"github.com/authzed/spicedb/internal/middleware/pertoken"
+)
+
+func TestWithDatastore(t *testing.T) {
+	someLogger := zerolog.Nop()
+	someAuthFunc := func(ctx context.Context) (context.Context, error) {
+		return nil, fmt.Errorf("expected auth error")
+	}
+	var someDispatcher dispatch.Dispatcher
+
+	opts := MiddlewareOption{
+		someLogger,
+		someAuthFunc,
+		true,
+		someDispatcher,
+		true,
+		true,
+		false,
+		"service",
+		nil,
+		nil,
+	}
+
+	someDS, err := memdb.NewMemdbDatastore(0, time.Hour, time.Hour)
+	require.NoError(t, err)
+
+	withDS := opts.WithDatastore(someDS)
+	require.NotNil(t, withDS)
+	require.NotNil(t, withDS.unaryDatastoreMiddleware)
+	require.NotNil(t, withDS.streamDatastoreMiddleware)
+
+	require.Equal(t, opts.Logger, withDS.Logger)
+	require.Equal(t, opts.DispatcherForMiddleware, withDS.DispatcherForMiddleware)
+	require.Equal(t, opts.EnableRequestLog, withDS.EnableRequestLog)
+	require.Equal(t, opts.EnableResponseLog, withDS.EnableResponseLog)
+	require.Equal(t, opts.DisableGRPCHistogram, withDS.DisableGRPCHistogram)
+	require.Equal(t, opts.MiddlewareServiceLabel, withDS.MiddlewareServiceLabel)
+
+	_, authError := withDS.AuthFunc(context.Background())
+	require.Error(t, authError)
+	require.ErrorContains(t, authError, "expected auth error")
+}
+
+func TestWithDatastoreMiddleware(t *testing.T) {
+	someLogger := zerolog.Nop()
+	someAuthFunc := func(ctx context.Context) (context.Context, error) {
+		return nil, fmt.Errorf("expected auth error")
+	}
+	var someDispatcher dispatch.Dispatcher
+
+	opts := MiddlewareOption{
+		someLogger,
+		someAuthFunc,
+		true,
+		someDispatcher,
+		true,
+		true,
+		false,
+		"anotherservice",
+		nil,
+		nil,
+	}
+
+	someMiddleware := pertoken.NewMiddleware(nil)
+
+	withDS := opts.WithDatastoreMiddleware(someMiddleware)
+	require.NotNil(t, withDS)
+	require.NotNil(t, withDS.unaryDatastoreMiddleware)
+	require.NotNil(t, withDS.streamDatastoreMiddleware)
+
+	require.Equal(t, opts.Logger, withDS.Logger)
+	require.Equal(t, opts.DispatcherForMiddleware, withDS.DispatcherForMiddleware)
+	require.Equal(t, opts.EnableRequestLog, withDS.EnableRequestLog)
+	require.Equal(t, opts.EnableResponseLog, withDS.EnableResponseLog)
+	require.Equal(t, opts.DisableGRPCHistogram, withDS.DisableGRPCHistogram)
+	require.Equal(t, opts.MiddlewareServiceLabel, withDS.MiddlewareServiceLabel)
+
+	_, authError := withDS.AuthFunc(context.Background())
+	require.Error(t, authError)
+	require.ErrorContains(t, authError, "expected auth error")
+}

pkg/middleware/consistency/consistency.go

@@ -146,7 +146,10 @@ func addRevisionToContextFromConsistency(ctx context.Context, req hasConsistency
 		if pickedRequest {
 			source = "request"
 		}
-		ConsistencyCounter.WithLabelValues("atleast", source, serviceLabel).Inc()
+
+		if serviceLabel != "" {
+			ConsistencyCounter.WithLabelValues("atleast", source, serviceLabel).Inc()
+		}
 
 		revision = picked