Merge pull request #1754 from Permify/ufuk/monitoring-histograms

refactor: update OTLP Histogram Records

Author: tolgaOzen

docs/api-reference/apidocs.swagger.json

@@ -3,7 +3,7 @@
   "info": {
     "title": "Permify API",
     "description": "Permify is an open source authorization service for creating fine-grained and scalable authorization systems.",
-    "version": "v1.1.9",
+    "version": "v1.2.0",
     "contact": {
       "name": "API Support",
       "url": "https://github.com/Permify/permify/issues",

docs/api-reference/openapiv2/apidocs.swagger.json

@@ -3,7 +3,7 @@
   "info": {
     "title": "Permify API",
     "description": "Permify is an open source authorization service for creating fine-grained and scalable authorization systems.",
-    "version": "v1.1.9",
+    "version": "v1.2.0",
     "contact": {
       "name": "API Support",
       "url": "https://github.com/Permify/permify/issues",

internal/engines/cache/check.go

@@ -3,7 +3,6 @@ package cache
 import (
 	"context"
 	"encoding/hex"
-	"time"
 
 	api "go.opentelemetry.io/otel/metric"
 
@@ -26,8 +25,7 @@ type CheckEngineWithCache struct {
 	cache        cache.Cache
 
 	// Metrics
-	cacheCounter              api.Int64Counter
-	cacheHitDurationHistogram api.Int64Histogram
+	cacheHitHistogram api.Int64Histogram
 }
 
 // NewCheckEngineWithCache creates a new instance of EngineKeyManager by initializing an EngineKeys
@@ -38,11 +36,10 @@ func NewCheckEngineWithCache(
 	cache cache.Cache,
 ) invoke.Check {
 	return &CheckEngineWithCache{
-		schemaReader:              schemaReader,
-		checker:                   checker,
-		cache:                     cache,
-		cacheCounter:              telemetry.NewCounter(internal.Meter, "cache_check_count", "Number of permission cached checks performed"),
-		cacheHitDurationHistogram: telemetry.NewHistogram(internal.Meter, "cache_hit_duration", "microseconds", "Duration of cache hits in microseconds"),
+		schemaReader:      schemaReader,
+		checker:           checker,
+		cache:             cache,
+		cacheHitHistogram: telemetry.NewHistogram(internal.Meter, "cache_hit", "amount", "Number of cache hits"),
 	}
 }
 
@@ -67,15 +64,8 @@ func (c *CheckEngineWithCache) Check(ctx context.Context, request *base.Permissi
 
 	// If a cached result is found, handle exclusion and return the result.
 	if found {
-		ctx, span := internal.Tracer.Start(ctx, "hit")
-		defer span.End()
-		start := time.Now()
-
-		// Increase the check count in the metrics.
-		c.cacheCounter.Add(ctx, 1)
-
-		duration := time.Now().Sub(start)
-		c.cacheHitDurationHistogram.Record(ctx, duration.Microseconds())
+		// Increase the hit count in the metrics.
+		c.cacheHitHistogram.Record(ctx, 1)
 
 		// If the request doesn't have the exclusion flag set, return the cached result.
 		return &base.PermissionCheckResponse{

internal/engines/cache/check_test.go

@@ -41,7 +41,7 @@ var _ = Describe("cache", func() {
 			Expect(err).ShouldNot(HaveOccurred())
 
 			// Initialize a new EngineKeys struct with a new cache.Cache instance
-			engineKeys := CheckEngineWithCache{nil, nil, cache, nil, nil}
+			engineKeys := CheckEngineWithCache{nil, nil, cache, nil}
 
 			// Create a new PermissionCheckRequest and PermissionCheckResponse
 			checkReq := &base.PermissionCheckRequest{
@@ -91,7 +91,7 @@ var _ = Describe("cache", func() {
 			Expect(err).ShouldNot(HaveOccurred())
 
 			// Initialize a new EngineKeys struct with a new cache.Cache instance
-			engineKeys := CheckEngineWithCache{nil, nil, cache, nil, nil}
+			engineKeys := CheckEngineWithCache{nil, nil, cache, nil}
 
 			// Create a new PermissionCheckRequest and PermissionCheckResponse
 			checkReq := &base.PermissionCheckRequest{
@@ -141,7 +141,7 @@ var _ = Describe("cache", func() {
 			Expect(err).ShouldNot(HaveOccurred())
 
 			// Initialize a new EngineKeys struct with a new cache.Cache instance
-			engineKeys := CheckEngineWithCache{nil, nil, cache, nil, nil}
+			engineKeys := CheckEngineWithCache{nil, nil, cache, nil}
 
 			// Create a new PermissionCheckRequest and PermissionCheckResponse
 			checkReq := &base.PermissionCheckRequest{
@@ -285,7 +285,7 @@ var _ = Describe("cache", func() {
 			Expect(err).ShouldNot(HaveOccurred())
 
 			// Initialize a new EngineKeys struct with a new cache.Cache instance
-			engineKeys := CheckEngineWithCache{nil, nil, cache, nil, nil}
+			engineKeys := CheckEngineWithCache{nil, nil, cache, nil}
 
 			// Create a new PermissionCheckRequest
 			checkReq := &base.PermissionCheckRequest{
@@ -320,7 +320,7 @@ var _ = Describe("cache", func() {
 			Expect(err).ShouldNot(HaveOccurred())
 
 			// Initialize a new EngineKeys struct with a new cache.Cache instance
-			engineKeys := CheckEngineWithCache{nil, nil, cache, nil, nil}
+			engineKeys := CheckEngineWithCache{nil, nil, cache, nil}
 
 			// Create some new PermissionCheckRequests and PermissionCheckResponses
 			checkReq1 := &base.PermissionCheckRequest{

internal/info.go

@@ -23,7 +23,7 @@ var Identifier = ""
 */
 const (
 	// Version is the last release of the Permify (e.g. v0.1.0)
-	Version = "v1.1.9"
+	Version = "v1.2.0"
 )
 
 // Function to create a single line of the ASCII art with centered content and color

internal/invoke/invoke.go

@@ -3,10 +3,10 @@ package invoke
 import (
 	"context"
 	"sync/atomic"
-	"time"
 
 	"go.opentelemetry.io/otel/attribute"
 	otelCodes "go.opentelemetry.io/otel/codes"
+	"go.opentelemetry.io/otel/metric"
 	api "go.opentelemetry.io/otel/metric"
 	"go.opentelemetry.io/otel/trace"
 
@@ -68,16 +68,10 @@ type DirectInvoker struct {
 	// LookupSubject
 	sp SubjectPermission
 
-	// Metrics
-	checkCounter             api.Int64Counter
-	lookupEntityCounter      api.Int64Counter
-	lookupSubjectCounter     api.Int64Counter
-	subjectPermissionCounter api.Int64Counter
-
-	checkDurationHistogram             api.Int64Histogram
-	lookupEntityDurationHistogram      api.Int64Histogram
-	lookupSubjectDurationHistogram     api.Int64Histogram
-	subjectPermissionDurationHistogram api.Int64Histogram
+	checkHistogram             api.Int64Histogram
+	lookupEntityHistogram      api.Int64Histogram
+	lookupSubjectHistogram     api.Int64Histogram
+	subjectPermissionHistogram api.Int64Histogram
 }
 
 // NewDirectInvoker is a constructor for DirectInvoker.
@@ -92,20 +86,17 @@ func NewDirectInvoker(
 	sp SubjectPermission,
 ) *DirectInvoker {
 	return &DirectInvoker{
-		schemaReader:                       schemaReader,
-		dataReader:                         dataReader,
-		cc:                                 cc,
-		ec:                                 ec,
-		lo:                                 lo,
-		sp:                                 sp,
-		checkCounter:                       telemetry.NewCounter(internal.Meter, "check_count", "Number of permission checks performed"),
-		lookupEntityCounter:                telemetry.NewCounter(internal.Meter, "lookup_entity_count", "Number of permission lookup entity performed"),
-		lookupSubjectCounter:               telemetry.NewCounter(internal.Meter, "lookup_subject_count", "Number of permission lookup subject performed"),
-		subjectPermissionCounter:           telemetry.NewCounter(internal.Meter, "subject_permission_count", "Number of subject permission performed"),
-		checkDurationHistogram:             telemetry.NewHistogram(internal.Meter, "check_duration", "microseconds", "Duration of checks in microseconds"),
-		lookupEntityDurationHistogram:      telemetry.NewHistogram(internal.Meter, "lookup_entity_duration", "microseconds", "Duration of lookup entity duration in microseconds"),
-		lookupSubjectDurationHistogram:     telemetry.NewHistogram(internal.Meter, "lookup_subject_duration", "microseconds", "Duration of lookup subject duration in microseconds"),
-		subjectPermissionDurationHistogram: telemetry.NewHistogram(internal.Meter, "subject_permission_duration", "microseconds", "Duration of subject permission duration in microseconds"),
+		schemaReader:   schemaReader,
+		dataReader:     dataReader,
+		cc:             cc,
+		ec:             ec,
+		lo:             lo,
+		sp:             sp,
+		checkHistogram: telemetry.NewHistogram(internal.Meter, "check", "amount", "Number of checks"),
+
+		lookupEntityHistogram:      telemetry.NewHistogram(internal.Meter, "lookup_entity", "amount", "Number of lookup entity"),
+		lookupSubjectHistogram:     telemetry.NewHistogram(internal.Meter, "lookup_subject", "amount", "Number of lookup subject"),
+		subjectPermissionHistogram: telemetry.NewHistogram(internal.Meter, "subject_permission", "amount", "Number of subject permission"),
 	}
 }
 
@@ -120,8 +111,13 @@ func (invoker *DirectInvoker) Check(ctx context.Context, request *base.Permissio
 		attribute.KeyValue{Key: "subject", Value: attribute.StringValue(tuple.SubjectToString(request.GetSubject()))},
 	))
 	defer span.End()
-
-	start := time.Now()
+	invoker.checkHistogram.Record(ctx, 1,
+		metric.WithAttributeSet(
+			attribute.NewSet(
+				attribute.KeyValue{Key: "subject_id", Value: attribute.StringValue(request.GetSubject().GetId())},
+				attribute.KeyValue{Key: "subject_type", Value: attribute.StringValue(request.GetSubject().GetType())},
+			)),
+	)
 
 	// Validate the depth of the request.
 	err = checkDepth(request)
@@ -186,15 +182,10 @@ func (invoker *DirectInvoker) Check(ctx context.Context, request *base.Permissio
 			},
 		}, err
 	}
-	duration := time.Since(start)
-	invoker.checkDurationHistogram.Record(ctx, duration.Microseconds())
 
 	// increaseCheckCount increments the CheckCount value in the response metadata by 1.
 	atomic.AddInt32(&response.GetMetadata().CheckCount, +1)
 
-	// Increase the check count in the metrics.
-	invoker.checkCounter.Add(ctx, 1)
-
 	span.SetAttributes(attribute.KeyValue{Key: "can", Value: attribute.StringValue(response.GetCan().String())})
 	return
 }
@@ -245,8 +236,6 @@ func (invoker *DirectInvoker) LookupEntity(ctx context.Context, request *base.Pe
 	))
 	defer span.End()
 
-	start := time.Now()
-
 	// Set SnapToken if not provided
 	if request.GetMetadata().GetSnapToken() == "" { // Check if the request has a SnapToken.
 		var st token.SnapToken
@@ -271,11 +260,7 @@ func (invoker *DirectInvoker) LookupEntity(ctx context.Context, request *base.Pe
 
 	resp, err := invoker.lo.LookupEntity(ctx, request)
 
-	duration := time.Since(start)
-	invoker.lookupEntityDurationHistogram.Record(ctx, duration.Microseconds())
-
-	// Increase the lookup entity count in the metrics.
-	invoker.lookupEntityCounter.Add(ctx, 1)
+	invoker.lookupEntityHistogram.Record(ctx, 1)
 
 	return resp, err
 }
@@ -292,8 +277,6 @@ func (invoker *DirectInvoker) LookupEntityStream(ctx context.Context, request *b
 	))
 	defer span.End()
 
-	start := time.Now()
-
 	// Set SnapToken if not provided
 	if request.GetMetadata().GetSnapToken() == "" { // Check if the request has a SnapToken.
 		var st token.SnapToken
@@ -318,11 +301,7 @@ func (invoker *DirectInvoker) LookupEntityStream(ctx context.Context, request *b
 
 	resp := invoker.lo.LookupEntityStream(ctx, request, server)
 
-	duration := time.Since(start)
-	invoker.lookupEntityDurationHistogram.Record(ctx, duration.Microseconds())
-
-	// Increase the lookup entity count in the metrics.
-	invoker.lookupEntityCounter.Add(ctx, 1)
+	invoker.lookupEntityHistogram.Record(ctx, 1)
 
 	return resp
 }
@@ -338,8 +317,6 @@ func (invoker *DirectInvoker) LookupSubject(ctx context.Context, request *base.P
 	))
 	defer span.End()
 
-	start := time.Now()
-
 	// Check if the request has a SnapToken. If not, a SnapToken is set.
 	if request.GetMetadata().GetSnapToken() == "" {
 		// Create an instance of SnapToken
@@ -370,11 +347,7 @@ func (invoker *DirectInvoker) LookupSubject(ctx context.Context, request *base.P
 
 	resp, err := invoker.lo.LookupSubject(ctx, request)
 
-	duration := time.Now().Sub(start)
-	invoker.lookupSubjectDurationHistogram.Record(ctx, duration.Microseconds())
-
-	// Increase the lookup subject count in the metrics.
-	invoker.lookupSubjectCounter.Add(ctx, 1)
+	invoker.lookupSubjectHistogram.Record(ctx, 1)
 
 	// Call the LookupSubject function of the ls field in the invoker, pass the context and request,
 	// and return its response and error
@@ -391,8 +364,6 @@ func (invoker *DirectInvoker) SubjectPermission(ctx context.Context, request *ba
 	))
 	defer span.End()
 
-	start := time.Now()
-
 	// Check if the request has a SnapToken. If not, a SnapToken is set.
 	if request.GetMetadata().GetSnapToken() == "" {
 		// Create an instance of SnapToken
@@ -422,11 +393,7 @@ func (invoker *DirectInvoker) SubjectPermission(ctx context.Context, request *ba
 	}
 	resp, err := invoker.sp.SubjectPermission(ctx, request)
 
-	duration := time.Now().Sub(start)
-	invoker.subjectPermissionDurationHistogram.Record(ctx, duration.Microseconds())
-
-	// Increase the subject permission count in the metrics.
-	invoker.subjectPermissionCounter.Add(ctx, 1)
+	invoker.subjectPermissionHistogram.Record(ctx, 1)
 
 	// Call the SubjectPermission function of the ls field in the invoker, pass the context and request,
 	// and return its response and error