feat(controlplane): add the organization name to Sentry errors

app/artifact-cas/internal/service/bytestream.go

@@ -138,7 +138,7 @@ func (s *ByteStreamService) Write(stream bytestream.ByteStream_WriteServer) erro
 		if backendErr, ok := errors.AsType[*backendUploadError](err); ok {
 			// the reused client may hold stale credentials
 			s.dropUploadBackend(info)
-			return sl.LogAndMaskErr(backendErr.err, s.log)
+			return sl.LogAndMaskErr(ctx, backendErr.err, s.log)
 		}
 		if isClientDisconnect(err) {
 			s.log.Infow("msg", "upload canceled", "digest", req.resource.Digest, "name", req.resource.FileName)
@@ -147,7 +147,7 @@ func (s *ByteStreamService) Write(stream bytestream.ByteStream_WriteServer) erro
 		if backend.IsUploadSizeExceeded(err) {
 			return status.Error(codes.ResourceExhausted, err.Error())
 		}
-		return sl.LogAndMaskErr(err, s.log)
+		return sl.LogAndMaskErr(ctx, err, s.log)
 	}
 
 	s.log.Infow("msg", "upload finished", "name", req.resource.FileName, "digest", req.resource.Digest, "size", committedSize)
@@ -174,7 +174,7 @@ func (s *ByteStreamService) backendExists(ctx context.Context, info *casJWT.Clai
 		if err != nil && kerrors.IsNotFound(err) {
 			return nil, false, err
 		} else if err != nil {
-			return nil, false, sl.LogAndMaskErr(err, s.log)
+			return nil, false, sl.LogAndMaskErr(ctx, err, s.log)
 		}
 
 		exists, err := storageBackend.Exists(ctx, digest)
@@ -184,7 +184,7 @@ func (s *ByteStreamService) backendExists(ctx context.Context, info *casJWT.Clai
 
 		s.dropUploadBackend(info)
 		if !cached || attempt > 0 {
-			return nil, false, sl.LogAndMaskErr(err, s.log)
+			return nil, false, sl.LogAndMaskErr(ctx, err, s.log)
 		}
 		s.log.Warnw("msg", "reused backend client failed, loading it again", "digest", digest, "error", err)
 	}
@@ -331,7 +331,7 @@ func (s *ByteStreamService) Read(req *bytestream.ReadRequest, stream bytestream.
 	if err != nil && kerrors.IsNotFound(err) {
 		return err
 	} else if err != nil {
-		return sl.LogAndMaskErr(err, s.log)
+		return sl.LogAndMaskErr(ctx, err, s.log)
 	}
 
 	// Stage the backend egress on local disk and verify it against the requested
@@ -340,12 +340,12 @@ func (s *ByteStreamService) Read(req *bytestream.ReadRequest, stream bytestream.
 	// because the artifact lives on disk.
 	f, size, err := s.stageDownload(ctx, backend, req.ResourceName)
 	if err != nil {
-		return s.downloadError(err, req.ResourceName)
+		return s.downloadError(ctx, err, req.ResourceName)
 	}
 	defer s.closeStagingFile(f)
 
 	if err := copyStaged(sendWriter{stream}, f); err != nil {
-		return s.downloadError(err, req.ResourceName)
+		return s.downloadError(ctx, err, req.ResourceName)
 	}
 
 	s.log.Infow("msg", "download finished", "digest", req.ResourceName, "size", size)
@@ -365,7 +365,7 @@ func (s *ByteStreamService) Read(req *bytestream.ReadRequest, stream bytestream.
 // key, corrupt or tampered, so it is reported as DataLoss with both digests
 // rather than blamed on the caller. A client that went away is not an error.
 // Anything else is masked.
-func (s *ByteStreamService) downloadError(err error, digest string) error {
+func (s *ByteStreamService) downloadError(ctx context.Context, err error, digest string) error {
 	if _, ok := errors.AsType[*digestMismatchError](err); ok {
 		return status.Error(codes.DataLoss, err.Error())
 	}
@@ -374,7 +374,7 @@ func (s *ByteStreamService) downloadError(err error, digest string) error {
 		return nil
 	}
 
-	return sl.LogAndMaskErr(err, s.log)
+	return sl.LogAndMaskErr(ctx, err, s.log)
 }
 
 // checkUploadSize returns an ErrUploadSizeExceeded when total exceeds maxSize.

app/artifact-cas/internal/service/download.go

@@ -16,6 +16,7 @@
 package service
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"io"
@@ -82,7 +83,7 @@ func (s *DownloadService) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "backend not found", http.StatusNotFound)
 		return
 	} else if err != nil {
-		http.Error(w, sl.LogAndMaskErr(err, s.log).Error(), http.StatusInternalServerError)
+		http.Error(w, sl.LogAndMaskErr(ctx, err, s.log).Error(), http.StatusInternalServerError)
 		return
 	}
 
@@ -91,7 +92,7 @@ func (s *DownloadService) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "artifact not found", http.StatusNotFound)
 		return
 	} else if err != nil {
-		http.Error(w, sl.LogAndMaskErr(err, s.log).Error(), http.StatusInternalServerError)
+		http.Error(w, sl.LogAndMaskErr(ctx, err, s.log).Error(), http.StatusInternalServerError)
 		return
 	}
 
@@ -118,7 +119,7 @@ func (s *DownloadService) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	// memory stays bounded because the artifact lives on disk, not in a buffer.
 	f, size, err := s.stageDownload(ctx, b, wantChecksum.Hex)
 	if err != nil {
-		s.writeDownloadError(w, err, wantChecksum.Hex)
+		s.writeDownloadError(ctx, w, err, wantChecksum.Hex)
 		return
 	}
 	defer s.closeStagingFile(f)
@@ -131,7 +132,7 @@ func (s *DownloadService) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	// A plain io.Copy lets the response writer pull the file with sendfile, so
 	// the verified bytes go kernel-to-kernel without a user-space buffer.
 	if _, err := io.Copy(w, f); err != nil {
-		s.writeDownloadError(w, err, wantChecksum.Hex)
+		s.writeDownloadError(ctx, w, err, wantChecksum.Hex)
 		return
 	}
 
@@ -171,7 +172,7 @@ func etagMatches(ifNoneMatch, etag string) bool {
 // to its key, corrupt or tampered, so it is a 500 carrying both digests rather
 // than blamed on the caller. A client that went away gets nothing. Anything
 // else is masked.
-func (s *DownloadService) writeDownloadError(w http.ResponseWriter, err error, digest string) {
+func (s *DownloadService) writeDownloadError(ctx context.Context, w http.ResponseWriter, err error, digest string) {
 	if _, ok := errors.AsType[*digestMismatchError](err); ok {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
@@ -181,5 +182,5 @@ func (s *DownloadService) writeDownloadError(w http.ResponseWriter, err error, d
 		return
 	}
 
-	http.Error(w, sl.LogAndMaskErr(err, s.log).Error(), http.StatusInternalServerError)
+	http.Error(w, sl.LogAndMaskErr(ctx, err, s.log).Error(), http.StatusInternalServerError)
 }

app/artifact-cas/internal/service/resource.go

@@ -53,14 +53,14 @@ func (s *ResourceService) Describe(ctx context.Context, req *v1.ResourceServiceD
 	if err != nil && errors.IsNotFound(err) {
 		return nil, err
 	} else if err != nil {
-		return nil, sl.LogAndMaskErr(err, s.log)
+		return nil, sl.LogAndMaskErr(ctx, err, s.log)
 	}
 
 	res, err := b.Describe(ctx, req.Digest)
 	if err != nil && backend.IsNotFound(err) {
 		return nil, errors.NotFound("not found", err.Error())
 	} else if err != nil {
-		return nil, sl.LogAndMaskErr(err, s.log)
+		return nil, sl.LogAndMaskErr(ctx, err, s.log)
 	}
 
 	return &v1.ResourceServiceDescribeResponse{

app/controlplane/internal/sentrycontext/sentry_context.go

@@ -33,29 +33,55 @@ import (
 // defaultTracingHeaders is a list of headers that are used to extract tracing information.
 var defaultTracingHeaders = []string{"X-Request-ID", "X-Correlation-ID", "X-Trace-ID"}
 
+// Sentry tags that identify the organization of the current request
+const (
+	tagOrgID   = "org.id"
+	tagOrgName = "org.name"
+)
+
+// NewSentryHub returns a middleware that gives each request its own Sentry hub on its context,
+// so concurrent requests do not share a scope. Put it before any middleware that reports to Sentry.
+func NewSentryHub() middleware.Middleware {
+	return func(handler middleware.Handler) middleware.Handler {
+		return func(ctx context.Context, req interface{}) (interface{}, error) {
+			return handler(sentry.SetHubOnContext(ctx, sentry.CurrentHub().Clone()), req)
+		}
+	}
+}
+
 // NewSentryContext returns a middleware that adds context to Sentry for the current request
-// that will be sent along with the error if one occurs
+// that will be sent along with the error if one occurs.
+// It sets the scope of the request hub that NewSentryHub added, and adds that hub if it is missing.
 func NewSentryContext() middleware.Middleware {
 	return func(handler middleware.Handler) middleware.Handler {
 		return func(ctx context.Context, req interface{}) (interface{}, error) {
-			addSentryContext(ctx, req)
+			hub := sentry.GetHubFromContext(ctx)
+			if hub == nil {
+				hub = sentry.CurrentHub().Clone()
+				ctx = sentry.SetHubOnContext(ctx, hub)
+			}
+
+			configureScope(ctx, hub.Scope(), req)
 			return handler(ctx, req)
 		}
 	}
 }
 
-// addSentryContext adds context to Sentry for the current request
-func addSentryContext(ctx context.Context, req interface{}) {
+// configureScope sets the account and request information of the current request on the scope
+func configureScope(ctx context.Context, scope *sentry.Scope, req interface{}) {
 	org := entities.CurrentOrg(ctx)
 	user := entities.CurrentUser(ctx)
 	apiToken := entities.CurrentAPIToken(ctx)
 	role := usercontext.CurrentAuthzSubject(ctx)
 
-	// ConfigureScope allows to set context that will be sent along with the error if one occurs
-	sentry.ConfigureScope(func(scope *sentry.Scope) {
-		scope.SetContext("Account", buildAuthContext(user, apiToken, org, role))
-		scope.SetContext("Request", buildRequestContext(ctx, req))
-	})
+	scope.SetContext("Account", buildAuthContext(user, apiToken, org, role))
+	scope.SetContext("Request", buildRequestContext(ctx, req))
+
+	// Tags make the organization searchable in Sentry. They do not take part in issue grouping.
+	if org != nil {
+		scope.SetTag(tagOrgID, org.ID)
+		scope.SetTag(tagOrgName, org.Name)
+	}
 }
 
 // buildAuthContext creates a map of the user and membership information

app/controlplane/internal/sentrycontext/sentry_context_test.go

@@ -21,8 +21,11 @@ import (
 
 	"github.com/chainloop-dev/chainloop/app/controlplane/pkg/usercontext/entities"
 
+	"github.com/getsentry/sentry-go"
+	"github.com/go-kratos/kratos/v2/middleware"
 	"github.com/go-kratos/kratos/v2/transport"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"go.opentelemetry.io/otel/trace"
 	"google.golang.org/grpc/metadata"
 )
@@ -65,6 +68,100 @@ func TestNewSentryContext(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+// scopeTags returns the tags that the scope of the hub in ctx adds to an event
+func scopeTags(ctx context.Context) map[string]string {
+	hub := sentry.GetHubFromContext(ctx)
+	if hub == nil {
+		return nil
+	}
+
+	return hub.Scope().ApplyToEvent(sentry.NewEvent(), nil, nil).Tags
+}
+
+func TestNewSentryContextOrgTags(t *testing.T) {
+	org := &entities.Org{ID: "org-id", Name: "my-org"}
+	orgTags := map[string]string{tagOrgID: "org-id", tagOrgName: "my-org"}
+
+	testCases := []struct {
+		name        string
+		middlewares []middleware.Middleware
+		org         *entities.Org
+		wantTags    map[string]string
+	}{
+		{
+			name:        "request with an org",
+			middlewares: []middleware.Middleware{NewSentryHub(), NewSentryContext()},
+			org:         org,
+			wantTags:    orgTags,
+		},
+		{
+			name:        "request without an org",
+			middlewares: []middleware.Middleware{NewSentryHub(), NewSentryContext()},
+			wantTags:    map[string]string{},
+		},
+		{
+			name:        "request without a hub from NewSentryHub",
+			middlewares: []middleware.Middleware{NewSentryContext()},
+			org:         org,
+			wantTags:    orgTags,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := context.Background()
+			if tc.org != nil {
+				ctx = entities.WithCurrentOrg(ctx, tc.org)
+			}
+
+			var gotTags map[string]string
+			handler := func(ctx context.Context, _ interface{}) (interface{}, error) {
+				gotTags = scopeTags(ctx)
+				return nil, nil
+			}
+
+			_, err := middleware.Chain(tc.middlewares...)(handler)(ctx, "request")
+			require.NoError(t, err)
+			assert.Equal(t, tc.wantTags, gotTags)
+			assert.Empty(t, scopeTags(sentry.SetHubOnContext(context.Background(), sentry.CurrentHub())), "the global scope must not change")
+		})
+	}
+}
+
+// Two requests of different orgs overlap: the first one is still running when the second one
+// sets its scope. Each request must keep the tags of its own org.
+func TestNewSentryContextConcurrentRequests(t *testing.T) {
+	chain := middleware.Chain(NewSentryHub(), NewSentryContext())
+	orgA := &entities.Org{ID: "org-a", Name: "org-a-name"}
+	orgB := &entities.Org{ID: "org-b", Name: "org-b-name"}
+
+	started, resume := make(chan struct{}), make(chan struct{})
+	tagsA := make(chan map[string]string, 1)
+	errA := make(chan error, 1)
+	go func() {
+		_, err := chain(func(ctx context.Context, _ interface{}) (interface{}, error) {
+			close(started)
+			<-resume
+			tagsA <- scopeTags(ctx)
+			return nil, nil
+		})(entities.WithCurrentOrg(context.Background(), orgA), "request-a")
+		errA <- err
+	}()
+
+	<-started
+	var tagsB map[string]string
+	_, err := chain(func(ctx context.Context, _ interface{}) (interface{}, error) {
+		tagsB = scopeTags(ctx)
+		return nil, nil
+	})(entities.WithCurrentOrg(context.Background(), orgB), "request-b")
+	require.NoError(t, err)
+	close(resume)
+	require.NoError(t, <-errA)
+
+	assert.Equal(t, map[string]string{tagOrgID: "org-a", tagOrgName: "org-a-name"}, <-tagsA)
+	assert.Equal(t, map[string]string{tagOrgID: "org-b", tagOrgName: "org-b-name"}, tagsB)
+}
+
 func TestBuildAuthContext(t *testing.T) {
 	org := &entities.Org{ID: "org1", Name: "OrgName"}
 	user := &entities.User{ID: "user1"}

app/controlplane/internal/server/grpc.go

@@ -35,7 +35,7 @@ import (
 	"github.com/chainloop-dev/chainloop/app/controlplane/internal/service"
 	"github.com/chainloop-dev/chainloop/app/controlplane/internal/usercontext"
 	"github.com/chainloop-dev/chainloop/pkg/credentials"
-	"github.com/getsentry/sentry-go"
+	"github.com/chainloop-dev/chainloop/pkg/servicelogger"
 	"github.com/golang-jwt/jwt/v5"
 
 	"github.com/go-kratos/kratos/v2/errors"
@@ -187,9 +187,12 @@ func NewGRPCServer(opts *Opts) (*grpc.Server, error) {
 
 func craftMiddleware(opts *Opts) []middleware.Middleware {
 	middlewares := []middleware.Middleware{
+		// Give each request its own Sentry hub before anything can report to Sentry
+		sentrycontext.NewSentryHub(),
 		recovery.Recovery(
-			recovery.WithHandler(func(_ context.Context, req, err interface{}) error {
-				sentry.CaptureMessage(fmt.Sprintf("%v", err))
+			recovery.WithHandler(func(ctx context.Context, _, err interface{}) error {
+				// The hub is set by NewSentryHub, so the panic carries the scope of its request
+				servicelogger.SentryHub(ctx).CaptureMessage(fmt.Sprintf("%v", err))
 				return errors.InternalServer("internal error", "there was an internal error")
 			}),
 		),

app/controlplane/internal/service/apitoken.go

@@ -79,7 +79,7 @@ func (s *APITokenService) Create(ctx context.Context, req *pb.APITokenServiceCre
 
 	token, err := s.APITokenUseCase.Create(ctx, req.Name, req.Description, expiresIn, &currentOrg.ID, biz.APITokenWithProject(project))
 	if err != nil {
-		return nil, handleUseCaseErr(err, s.log)
+		return nil, handleUseCaseErr(ctx, err, s.log)
 	}
 
 	return &pb.APITokenServiceCreateResponse{
@@ -117,7 +117,7 @@ func (s *APITokenService) List(ctx context.Context, req *pb.APITokenServiceListR
 
 	tokens, err := s.APITokenUseCase.List(ctx, currentOrg.ID, biz.WithAPITokenStatusFilter(mapTokenStatusFilter(req.GetStatusFilter())), biz.WithAPITokenProjectFilter(defaultProjectFilter), biz.WithAPITokenScope(scope))
 	if err != nil {
-		return nil, handleUseCaseErr(err, s.log)
+		return nil, handleUseCaseErr(ctx, err, s.log)
 	}
 
 	result := make([]*pb.APITokenItem, 0, len(tokens))
@@ -158,7 +158,7 @@ func (s *APITokenService) Revoke(ctx context.Context, req *pb.APITokenServiceRev
 
 	t, err := s.APITokenUseCase.FindByIDInOrg(ctx, currentOrg.ID, req.GetId())
 	if err != nil {
-		return nil, handleUseCaseErr(err, s.log)
+		return nil, handleUseCaseErr(ctx, err, s.log)
 	}
 
 	// System tokens are internal and must not be reachable through the public API.
@@ -195,7 +195,7 @@ func (s *APITokenService) Revoke(ctx context.Context, req *pb.APITokenServiceRev
 	}
 
 	if err := s.APITokenUseCase.Revoke(ctx, currentOrg.ID, t.ID.String()); err != nil {
-		return nil, handleUseCaseErr(err, s.log)
+		return nil, handleUseCaseErr(ctx, err, s.log)
 	}
 
 	return &pb.APITokenServiceRevokeResponse{}, nil