diff --git a/internal/controlplane/handlers_artifacts.go b/internal/controlplane/handlers_artifacts.go index a8e4a357c4..1b05911dd1 100644 --- a/internal/controlplane/handlers_artifacts.go +++ b/internal/controlplane/handlers_artifacts.go @@ -30,6 +30,7 @@ import ( "github.com/stacklok/minder/internal/db" "github.com/stacklok/minder/internal/engine" + "github.com/stacklok/minder/internal/logger" "github.com/stacklok/minder/internal/util" pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1" ) @@ -60,6 +61,10 @@ func (s *Server) ListArtifacts(ctx context.Context, in *pb.ListArtifactsRequest) return nil, fmt.Errorf("failed to list artifacts: %w", err) } + // Telemetry logging + logger.BusinessRecord(ctx).Provider = provider.Name + logger.BusinessRecord(ctx).Project = projectID + return &pb.ListArtifactsResponse{Results: results}, nil } @@ -120,6 +125,12 @@ func (s *Server) GetArtifactByName(ctx context.Context, in *pb.GetArtifactByName return nil, status.Errorf(codes.Unknown, "failed to get artifact versions: %s", err) } + // Telemetry logging + logger.BusinessRecord(ctx).Provider = artifact.Provider + logger.BusinessRecord(ctx).Project = artifact.ProjectID + logger.BusinessRecord(ctx).Artifact = artifact.ID + logger.BusinessRecord(ctx).Repository = artifact.RepositoryID + return &pb.GetArtifactByNameResponse{Artifact: &pb.Artifact{ ArtifactPk: artifact.ID.String(), Owner: artifact.RepoOwner, @@ -174,6 +185,12 @@ func (s *Server) GetArtifactById(ctx context.Context, in *pb.GetArtifactByIdRequ return nil, status.Errorf(codes.Unknown, "failed to get artifact versions: %s", err) } + // Telemetry logging + logger.BusinessRecord(ctx).Provider = artifact.Provider + logger.BusinessRecord(ctx).Project = artifact.ProjectID + logger.BusinessRecord(ctx).Artifact = artifact.ID + logger.BusinessRecord(ctx).Repository = artifact.RepositoryID + return &pb.GetArtifactByIdResponse{Artifact: &pb.Artifact{ ArtifactPk: artifact.ID.String(), Owner: artifact.RepoOwner, diff --git a/internal/controlplane/handlers_authz.go b/internal/controlplane/handlers_authz.go index eba7462d0b..05f25cb15c 100644 --- a/internal/controlplane/handlers_authz.go +++ b/internal/controlplane/handlers_authz.go @@ -16,6 +16,8 @@ package controlplane import ( "context" + "crypto/sha256" + "encoding/hex" "fmt" "github.com/google/uuid" @@ -28,6 +30,7 @@ import ( "github.com/stacklok/minder/internal/auth" "github.com/stacklok/minder/internal/db" "github.com/stacklok/minder/internal/engine" + "github.com/stacklok/minder/internal/logger" "github.com/stacklok/minder/internal/util" minder "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1" ) @@ -51,6 +54,10 @@ func lookupUserPermissions(ctx context.Context, store db.Store) auth.UserPermiss subject := auth.GetUserSubjectFromContext(ctx) + // Attach the login sha for telemetry usage (hash of the user subject from the JWT) + loginSHA := sha256.Sum256([]byte(subject)) + logger.BusinessRecord(ctx).LoginHash = hex.EncodeToString(loginSHA[:]) + // read all information for user claims userInfo, err := store.GetUserBySubject(ctx, subject) if err != nil { diff --git a/internal/controlplane/handlers_oauth.go b/internal/controlplane/handlers_oauth.go index 1fdaec1147..f4d272e64d 100644 --- a/internal/controlplane/handlers_oauth.go +++ b/internal/controlplane/handlers_oauth.go @@ -33,6 +33,7 @@ import ( mcrypto "github.com/stacklok/minder/internal/crypto" "github.com/stacklok/minder/internal/db" "github.com/stacklok/minder/internal/engine" + "github.com/stacklok/minder/internal/logger" "github.com/stacklok/minder/internal/util" pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1" ) @@ -108,13 +109,14 @@ func (s *Server) GetAuthorizationURL(ctx context.Context, return nil, status.Errorf(codes.Unknown, "error inserting session state: %s", err) } - // Return the authorization URL and state - url := oauthConfig.AuthCodeURL(state, oauth2.AccessTypeOffline) + // Telemetry logging + logger.BusinessRecord(ctx).Provider = provider.Name + logger.BusinessRecord(ctx).Project = projectID - response := &pb.GetAuthorizationURLResponse{ - Url: url, - } - return response, nil + // Return the authorization URL and state + return &pb.GetAuthorizationURLResponse{ + Url: oauthConfig.AuthCodeURL(state, oauth2.AccessTypeOffline), + }, nil } // ExchangeCodeForTokenCLI exchanges an OAuth2 code for a token @@ -211,6 +213,10 @@ func (s *Server) ExchangeCodeForTokenCLI(ctx context.Context, return nil, status.Errorf(codes.Unknown, "error inserting access token: %s", err) } + // Telemetry logging + logger.BusinessRecord(ctx).Provider = provider.Name + logger.BusinessRecord(ctx).Project = stateData.ProjectID + return &httpbody.HttpBody{ ContentType: "text/html", Data: auth.OAuthSuccessHtml, @@ -283,7 +289,7 @@ func (s *Server) StoreProviderToken(ctx context.Context, } encodedToken := base64.StdEncoding.EncodeToString(encryptedToken) - // additionally add owner + // additionally, add an owner var owner sql.NullString if in.Owner == nil { owner = sql.NullString{Valid: false} @@ -299,6 +305,11 @@ func (s *Server) StoreProviderToken(ctx context.Context, } else if err != nil { return nil, status.Errorf(codes.Internal, "error storing access token: %v", err) } + + // Telemetry logging + logger.BusinessRecord(ctx).Provider = provider.Name + logger.BusinessRecord(ctx).Project = projectID + return &pb.StoreProviderTokenResponse{}, nil } @@ -327,5 +338,10 @@ func (s *Server) VerifyProviderTokenFrom(ctx context.Context, } return nil, status.Errorf(codes.Internal, "error getting access token: %v", err) } + + // Telemetry logging + logger.BusinessRecord(ctx).Provider = provider.Name + logger.BusinessRecord(ctx).Project = projectID + return &pb.VerifyProviderTokenFromResponse{Status: "OK"}, nil } diff --git a/internal/controlplane/handlers_profile.go b/internal/controlplane/handlers_profile.go index cb06dfaa8e..d61b06fc59 100644 --- a/internal/controlplane/handlers_profile.go +++ b/internal/controlplane/handlers_profile.go @@ -31,6 +31,7 @@ import ( "github.com/stacklok/minder/internal/db" "github.com/stacklok/minder/internal/engine" "github.com/stacklok/minder/internal/engine/entities" + "github.com/stacklok/minder/internal/logger" "github.com/stacklok/minder/internal/reconcilers" "github.com/stacklok/minder/internal/util" minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1" @@ -168,6 +169,11 @@ func (s *Server) CreateProfile(ctx context.Context, log.Printf("error publishing reconciler event: %v", err) } + // Telemetry logging + logger.BusinessRecord(ctx).Provider = profile.Provider + logger.BusinessRecord(ctx).Project = profile.ProjectID + logger.BusinessRecord(ctx).Profile = logger.Profile{Name: profile.Name, ID: profile.ID} + return resp, nil } @@ -240,7 +246,7 @@ func (s *Server) DeleteProfile(ctx context.Context, return nil, util.UserVisibleError(codes.InvalidArgument, "invalid profile ID") } - _, err = s.store.GetProfileByID(ctx, parsedProfileID) + profile, err := s.store.GetProfileByID(ctx, parsedProfileID) if err != nil { if errors.Is(err, sql.ErrNoRows) { return nil, status.Error(codes.NotFound, "profile not found") @@ -253,6 +259,11 @@ func (s *Server) DeleteProfile(ctx context.Context, return nil, status.Errorf(codes.Internal, "failed to delete profile: %s", err) } + // Telemetry logging + logger.BusinessRecord(ctx).Provider = profile.Provider + logger.BusinessRecord(ctx).Project = profile.ProjectID + logger.BusinessRecord(ctx).Profile = logger.Profile{Name: profile.Name, ID: profile.ID} + return &minderv1.DeleteProfileResponse{}, nil } @@ -282,6 +293,10 @@ func (s *Server) ListProfiles(ctx context.Context, resp.Profiles = append(resp.Profiles, profile) } + // Telemetry logging + logger.BusinessRecord(ctx).Provider = entityCtx.Provider.Name + logger.BusinessRecord(ctx).Project = entityCtx.Project.ID + return &resp, nil } @@ -315,6 +330,11 @@ func (s *Server) GetProfileById(ctx context.Context, return nil, status.Errorf(codes.Internal, "failed to get profile: %s", err) } + // Telemetry logging + logger.BusinessRecord(ctx).Provider = entityCtx.Provider.Name + logger.BusinessRecord(ctx).Project = entityCtx.Project.ID + logger.BusinessRecord(ctx).Profile = logger.Profile{Name: prof.Name, ID: parsedProfileID} + return &minderv1.GetProfileByIdResponse{ Profile: prof, }, nil @@ -498,6 +518,11 @@ func (s *Server) GetProfileStatusByName(ctx context.Context, // TODO: Add other entities once we have database entries for them } + // Telemetry logging + logger.BusinessRecord(ctx).Provider = entityCtx.Provider.Name + logger.BusinessRecord(ctx).Project = entityCtx.Project.ID + logger.BusinessRecord(ctx).Profile = logger.Profile{Name: dbstat.Name, ID: dbstat.ID} + return &minderv1.GetProfileStatusByNameResponse{ ProfileStatus: &minderv1.ProfileStatus{ ProfileId: dbstat.ID.String(), @@ -546,6 +571,10 @@ func (s *Server) GetProfileStatusByProject(ctx context.Context, }) } + // Telemetry logging + logger.BusinessRecord(ctx).Provider = entityCtx.Provider.Name + logger.BusinessRecord(ctx).Project = entityCtx.Project.ID + return res, nil } @@ -685,6 +714,11 @@ func (s *Server) UpdateProfile(ctx context.Context, log.Printf("error publishing reconciler event: %v", err) } + // Telemetry logging + logger.BusinessRecord(ctx).Provider = profile.Provider + logger.BusinessRecord(ctx).Project = profile.ProjectID + logger.BusinessRecord(ctx).Profile = logger.Profile{Name: profile.Name, ID: profile.ID} + return resp, nil } diff --git a/internal/controlplane/handlers_repositories.go b/internal/controlplane/handlers_repositories.go index 497ec59e3b..9ddeb3979f 100644 --- a/internal/controlplane/handlers_repositories.go +++ b/internal/controlplane/handlers_repositories.go @@ -28,6 +28,7 @@ import ( "github.com/stacklok/minder/internal/db" "github.com/stacklok/minder/internal/engine" + "github.com/stacklok/minder/internal/logger" "github.com/stacklok/minder/internal/providers" github "github.com/stacklok/minder/internal/providers/github" "github.com/stacklok/minder/internal/reconcilers" @@ -41,7 +42,7 @@ const maxFetchLimit = 100 // RegisterRepository adds repositories to the database and registers a webhook // Once a user had enrolled in a project (they have a valid token), they can register // repositories to be monitored by the minder by provisioning a webhook on the -// repositor(ies). +// repository(ies). func (s *Server) RegisterRepository(ctx context.Context, in *pb.RegisterRepositoryRequest) (*pb.RegisterRepositoryResponse, error) { entityCtx := engine.EntityFromContext(ctx) @@ -124,7 +125,7 @@ func (s *Server) RegisterRepository(ctx context.Context, repoDBID := dbRepo.ID.String() r.Id = &repoDBID - // publish a reconcile event for the registered repositories + // publish a reconciling event for the registered repositories log.Printf("publishing register event for repository: %s/%s", r.Owner, r.Name) msg, err := reconcilers.NewRepoReconcilerMessage(provider.Name, r.RepoId, projectID) @@ -138,6 +139,11 @@ func (s *Server) RegisterRepository(ctx context.Context, log.Printf("error publishing reconciler event: %v", err) } + // Telemetry logging + logger.BusinessRecord(ctx).Provider = provider.Name + logger.BusinessRecord(ctx).Project = projectID + logger.BusinessRecord(ctx).Repository = dbRepo.ID + return response, nil } @@ -220,6 +226,10 @@ func (s *Server) ListRepositories(ctx context.Context, resp.Results = results resp.Cursor = respRepoCursor.String() + // Telemetry logging + logger.BusinessRecord(ctx).Provider = provider.Name + logger.BusinessRecord(ctx).Project = projectID + return &resp, nil } @@ -250,6 +260,12 @@ func (s *Server) GetRepositoryById(ctx context.Context, Project: &projID, Provider: &repo.Provider, } + + // Telemetry logging + logger.BusinessRecord(ctx).Provider = repo.Provider + logger.BusinessRecord(ctx).Project = repo.ProjectID + logger.BusinessRecord(ctx).Repository = repo.ID + return &pb.GetRepositoryByIdResponse{Repository: r}, nil } @@ -297,6 +313,12 @@ func (s *Server) GetRepositoryByName(ctx context.Context, Project: &projID, Provider: &repo.Provider, } + + // Telemetry logging + logger.BusinessRecord(ctx).Provider = repo.Provider + logger.BusinessRecord(ctx).Project = repo.ProjectID + logger.BusinessRecord(ctx).Repository = repo.ID + return &pb.GetRepositoryByNameResponse{Repository: r}, nil } @@ -331,6 +353,11 @@ func (s *Server) DeleteRepositoryById(ctx context.Context, return nil, err } + // Telemetry logging + logger.BusinessRecord(ctx).Provider = repo.Provider + logger.BusinessRecord(ctx).Project = repo.ProjectID + logger.BusinessRecord(ctx).Repository = repo.ID + // return the response with the id of the deleted repository return &pb.DeleteRepositoryByIdResponse{ RepositoryId: in.RepositoryId, @@ -376,6 +403,11 @@ func (s *Server) DeleteRepositoryByName(ctx context.Context, return nil, err } + // Telemetry logging + logger.BusinessRecord(ctx).Provider = repo.Provider + logger.BusinessRecord(ctx).Project = repo.ProjectID + logger.BusinessRecord(ctx).Repository = repo.ID + // return the response with the name of the deleted repository return &pb.DeleteRepositoryByNameResponse{ Name: in.Name, @@ -474,6 +506,10 @@ func (s *Server) ListRemoteRepositoriesFromProvider( out.Results = append(out.Results, repo) } + // Telemetry logging + logger.BusinessRecord(ctx).Provider = provider.Name + logger.BusinessRecord(ctx).Project = projectID + return out, nil } diff --git a/internal/controlplane/handlers_ruletype.go b/internal/controlplane/handlers_ruletype.go index ae21a21208..c4cea2025c 100644 --- a/internal/controlplane/handlers_ruletype.go +++ b/internal/controlplane/handlers_ruletype.go @@ -27,6 +27,7 @@ import ( "github.com/stacklok/minder/internal/db" "github.com/stacklok/minder/internal/engine" + "github.com/stacklok/minder/internal/logger" "github.com/stacklok/minder/internal/util" "github.com/stacklok/minder/internal/util/schemaupdate" minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1" @@ -69,6 +70,10 @@ func (s *Server) ListRuleTypes( resp.RuleTypes = append(resp.RuleTypes, rtpb) } + // Telemetry logging + logger.BusinessRecord(ctx).Provider = entityCtx.Provider.Name + logger.BusinessRecord(ctx).Project = entityCtx.Project.ID + return resp, nil } @@ -107,6 +112,11 @@ func (s *Server) GetRuleTypeByName( resp.RuleType = rt + // Telemetry logging + logger.BusinessRecord(ctx).Provider = rtdb.Provider + logger.BusinessRecord(ctx).Project = rtdb.ProjectID + logger.BusinessRecord(ctx).RuleType = logger.RuleType{Name: rtdb.Name, ID: rtdb.ID} + return resp, nil } @@ -146,6 +156,11 @@ func (s *Server) GetRuleTypeById( resp.RuleType = rt + // Telemetry logging + logger.BusinessRecord(ctx).Provider = rtdb.Provider + logger.BusinessRecord(ctx).Project = rtdb.ProjectID + logger.BusinessRecord(ctx).RuleType = logger.RuleType{Name: rtdb.Name, ID: rtdb.ID} + return resp, nil } @@ -193,7 +208,7 @@ func (s *Server) CreateRuleType( return nil, fmt.Errorf("cannot convert rule definition to db: %v", err) } - dbrtyp, err := s.store.CreateRuleType(ctx, db.CreateRuleTypeParams{ + rtdb, err := s.store.CreateRuleType(ctx, db.CreateRuleTypeParams{ Name: in.GetName(), Provider: entityCtx.Provider.Name, ProjectID: entityCtx.Project.ID, @@ -205,11 +220,16 @@ func (s *Server) CreateRuleType( return nil, status.Errorf(codes.Unknown, "failed to create rule type: %s", err) } - rt, err := engine.RuleTypePBFromDB(&dbrtyp) + rt, err := engine.RuleTypePBFromDB(&rtdb) if err != nil { - return nil, fmt.Errorf("cannot convert rule type %s to pb: %v", dbrtyp.Name, err) + return nil, fmt.Errorf("cannot convert rule type %s to pb: %v", rtdb.Name, err) } + // Telemetry logging + logger.BusinessRecord(ctx).Provider = rtdb.Provider + logger.BusinessRecord(ctx).Project = rtdb.ProjectID + logger.BusinessRecord(ctx).RuleType = logger.RuleType{Name: rtdb.Name, ID: rtdb.ID} + return &minderv1.CreateRuleTypeResponse{ RuleType: rt, }, nil @@ -297,6 +317,11 @@ func (s *Server) UpdateRuleType( return nil, fmt.Errorf("cannot convert rule type %s to pb: %v", rtdb.Name, err) } + // Telemetry logging + logger.BusinessRecord(ctx).Provider = rtdb.Provider + logger.BusinessRecord(ctx).Project = rtdb.ProjectID + logger.BusinessRecord(ctx).RuleType = logger.RuleType{Name: rtdb.Name, ID: rtdb.ID} + return &minderv1.UpdateRuleTypeResponse{ RuleType: rt, }, nil @@ -313,7 +338,7 @@ func (s *Server) DeleteRuleType( } // first read rule type by id, so we can get provider - ruletype, err := s.store.GetRuleTypeByID(ctx, parsedRuleTypeID) + rtdb, err := s.store.GetRuleTypeByID(ctx, parsedRuleTypeID) if err != nil { if errors.Is(err, sql.ErrNoRows) { return nil, util.UserVisibleError(codes.NotFound, "rule type %s not found", in.GetId()) @@ -322,8 +347,8 @@ func (s *Server) DeleteRuleType( } prov, err := s.store.GetProviderByName(ctx, db.GetProviderByNameParams{ - Name: ruletype.Provider, - ProjectID: ruletype.ProjectID, + Name: rtdb.Provider, + ProjectID: rtdb.ProjectID, }) if err != nil { return nil, status.Errorf(codes.Unknown, "failed to get provider: %s", err) @@ -343,7 +368,7 @@ func (s *Server) DeleteRuleType( return nil, err } - profileInfo, err := s.store.ListProfilesInstantiatingRuleType(ctx, ruletype.ID) + profileInfo, err := s.store.ListProfilesInstantiatingRuleType(ctx, rtdb.ID) // We have profiles that use this rule type, so we can't delete it if err == nil { if len(profileInfo) > 0 { @@ -370,5 +395,10 @@ func (s *Server) DeleteRuleType( return nil, status.Errorf(codes.Unknown, "failed to delete rule type: %s", err) } + // Telemetry logging + logger.BusinessRecord(ctx).Provider = rtdb.Provider + logger.BusinessRecord(ctx).Project = rtdb.ProjectID + logger.BusinessRecord(ctx).RuleType = logger.RuleType{Name: rtdb.Name, ID: rtdb.ID} + return &minderv1.DeleteRuleTypeResponse{}, nil } diff --git a/internal/controlplane/handlers_user.go b/internal/controlplane/handlers_user.go index e7f86b39c1..041c35a7a3 100644 --- a/internal/controlplane/handlers_user.go +++ b/internal/controlplane/handlers_user.go @@ -31,6 +31,7 @@ import ( "google.golang.org/protobuf/types/known/timestamppb" "github.com/stacklok/minder/internal/db" + "github.com/stacklok/minder/internal/logger" pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1" ) @@ -116,6 +117,9 @@ func (s *Server) CreateUser(ctx context.Context, return nil, status.Errorf(codes.Internal, "failed to commit transaction: %s", err) } + // Telemetry logging + logger.BusinessRecord(ctx).Project = userProject + return &pb.CreateUserResponse{ Id: user.ID, OrganizationId: user.OrganizationID.String(), diff --git a/internal/engine/executor_test.go b/internal/engine/executor_test.go index d4ff28114c..bfdb7a8fc0 100644 --- a/internal/engine/executor_test.go +++ b/internal/engine/executor_test.go @@ -18,7 +18,6 @@ import ( "context" "encoding/base64" "encoding/json" - "fmt" "os" "testing" "time" @@ -309,9 +308,9 @@ default allow = true`, require.NoError(t, err, "expected no error") ts := &logger.TelemetryStore{ - Project: projectID.String(), - Provider: providerName, - Resource: fmt.Sprintf("repository/%s", repositoryID), + Project: projectID, + Provider: providerName, + Repository: repositoryID, } ctx = ts.WithTelemetry(ctx) msg.SetContext(ctx) @@ -333,9 +332,9 @@ default allow = true`, require.Len(t, ts.Evals, 1, "expected one eval to be logged") requredEval := ts.Evals[0] - require.Equal(t, "test-profile", requredEval.ProfileName) + require.Equal(t, "test-profile", requredEval.Profile.Name) require.Equal(t, "success", requredEval.EvalResult) - require.Equal(t, "passthrough", requredEval.RuleName) + require.Equal(t, "passthrough", requredEval.RuleType.Name) require.Equal(t, "off", requredEval.Actions[alert.ActionType].State) require.Equal(t, "off", requredEval.Actions[remediate.ActionType].State) } diff --git a/internal/logger/telemetry_store.go b/internal/logger/telemetry_store.go index fdde307a74..b789b83e06 100644 --- a/internal/logger/telemetry_store.go +++ b/internal/logger/telemetry_store.go @@ -17,6 +17,7 @@ package logger import ( "context" + "github.com/google/uuid" "github.com/rs/zerolog" "github.com/stacklok/minder/internal/engine/actions/alert" @@ -32,6 +33,18 @@ const ( telemetryContextKey key = iota ) +// RuleType is a struct describing a rule type for telemetry purposes +type RuleType struct { + Name string `json:"name"` + ID uuid.UUID `json:"id"` +} + +// Profile is a struct describing a Profile for telemetry purposes +type Profile struct { + Name string `json:"name"` + ID uuid.UUID `json:"id"` +} + // ActionEvalData reports type ActionEvalData struct { // how was the action configured - on, off, ... @@ -42,8 +55,8 @@ type ActionEvalData struct { // RuleEvalData reports type RuleEvalData struct { - RuleName string `json:"rule_name"` - ProfileName string `json:"profile_name"` + RuleType RuleType `json:"ruletype"` + Profile Profile `json:"profile"` EvalResult string `json:"eval_result"` Actions map[interfaces.ActionType]ActionEvalData `json:"actions"` @@ -53,14 +66,26 @@ type RuleEvalData struct { // TelemetryStore is a struct that can be used to store telemetry data in the context. type TelemetryStore struct { - // Project records the project that the request was associated with. - Project string `json:"project"` + // Project records the project ID that the request was associated with. + Project uuid.UUID `json:"project"` - // Provider records the provider that the request was associated with. + // Provider records the provider name that the request was associated with. Provider string `json:"provider"` - // The resource processed by the request, for example, a repository or a profile. - Resource string `json:"resource"` + // Repository is the repository ID that the request was associated with. + Repository uuid.UUID `json:"repository"` + + // Artifact is the artifact ID that the request was associated with. + Artifact uuid.UUID `json:"artifact"` + + // PullRequest is the pull request ID that the request was associated with. + PullRequest uuid.UUID `json:"pr"` + + // Profile is the profile that the request was associated with. + Profile Profile `json:"profile"` + + // RuleType is the rule type that the request was associated with. + RuleType RuleType `json:"ruletype"` // Data from RPCs @@ -82,10 +107,21 @@ func (ts *TelemetryStore) AddRuleEval( return } + // Get rule type ID + ruleTypeID, err := uuid.Parse(evalInfo.GetRuleType().GetId()) + if err != nil { + return + } + // Get profile ID + profileID, err := uuid.Parse(evalInfo.GetProfile().GetId()) + if err != nil { + return + } + red := RuleEvalData{ - RuleName: evalInfo.GetRule().GetType(), - ProfileName: evalInfo.GetProfile().GetName(), - EvalResult: errors.EvalErrorAsString(evalInfo.GetEvalErr()), + RuleType: RuleType{Name: evalInfo.GetRuleType().GetName(), ID: ruleTypeID}, + Profile: Profile{Name: evalInfo.GetProfile().GetName(), ID: profileID}, + EvalResult: errors.EvalErrorAsString(evalInfo.GetEvalErr()), Actions: map[interfaces.ActionType]ActionEvalData{ remediate.ActionType: { State: evalInfo.GetActionsOnOff()[remediate.ActionType].String(), @@ -134,18 +170,30 @@ func (ts *TelemetryStore) Record(e *zerolog.Event) *zerolog.Event { } // We could use reflection here like json.Marshal, but given // the small number of fields, we'll just add them explicitly. - if ts.Project != "" { - e.Str("project", ts.Project) + if ts.Project != uuid.Nil { + e.Str("project", ts.Project.String()) } if ts.Provider != "" { e.Str("provider", ts.Provider) } - if ts.Resource != "" { - e.Str("resource", ts.Resource) - } if ts.LoginHash != "" { e.Str("login_sha", ts.LoginHash) } + if ts.Repository != uuid.Nil { + e.Str("repository", ts.Repository.String()) + } + if ts.Artifact != uuid.Nil { + e.Str("artifact", ts.Artifact.String()) + } + if ts.PullRequest != uuid.Nil { + e.Str("pr", ts.PullRequest.String()) + } + if ts.Profile != (Profile{}) { + e.Any("profile", ts.Profile) + } + if ts.RuleType != (RuleType{}) { + e.Any("ruletype", ts.RuleType) + } if len(ts.Evals) > 0 { e.Any("rules", ts.Evals) } diff --git a/internal/logger/telemetry_store_test.go b/internal/logger/telemetry_store_test.go index e99589ae78..25e6700203 100644 --- a/internal/logger/telemetry_store_test.go +++ b/internal/logger/telemetry_store_test.go @@ -21,6 +21,7 @@ import ( "reflect" "testing" + "github.com/google/uuid" "github.com/rs/zerolog" "github.com/stacklok/minder/internal/engine/actions/alert" @@ -32,6 +33,9 @@ import ( ) func TestTelemetryStore_Record(t *testing.T) { + testUUIDString := "00000000-0000-0000-0000-000000000001" + testUUID := uuid.MustParse(testUUIDString) + t.Parallel() cases := []struct { name string @@ -45,11 +49,13 @@ func TestTelemetryStore_Record(t *testing.T) { evalParamsFunc: func() *engif.EvalStatusParams { ep := &engif.EvalStatusParams{} - ep.Rule = &minderv1.Profile_Rule{ - Type: "artifact_signature", - } ep.Profile = &minderv1.Profile{ Name: "artifact_profile", + Id: &testUUIDString, + } + ep.RuleType = &minderv1.RuleType{ + Name: "artifact_signature", + Id: &testUUIDString, } ep.SetEvalErr(enginerr.NewErrEvaluationFailed("evaluation failure reason")) ep.SetActionsOnOff(map[engif.ActionType]engif.ActionOpt{ @@ -63,9 +69,8 @@ func TestTelemetryStore_Record(t *testing.T) { return ep }, recordFunc: func(ctx context.Context, evalParams engif.ActionsParams) { - logger.BusinessRecord(ctx).Project = "bar" - - logger.BusinessRecord(ctx).Resource = "foo/repo" + logger.BusinessRecord(ctx).Project = testUUID + logger.BusinessRecord(ctx).Repository = testUUID logger.BusinessRecord(ctx).AddRuleEval(evalParams) }, }, { @@ -74,11 +79,13 @@ func TestTelemetryStore_Record(t *testing.T) { evalParamsFunc: func() *engif.EvalStatusParams { ep := &engif.EvalStatusParams{} - ep.Rule = &minderv1.Profile_Rule{ - Type: "artifact_signature", - } ep.Profile = &minderv1.Profile{ Name: "artifact_profile", + Id: &testUUIDString, + } + ep.RuleType = &minderv1.RuleType{ + Name: "artifact_signature", + Id: &testUUIDString, } ep.SetEvalErr(enginerr.NewErrEvaluationFailed("evaluation failure reason")) ep.SetActionsOnOff(map[engif.ActionType]engif.ActionOpt{ @@ -92,29 +99,34 @@ func TestTelemetryStore_Record(t *testing.T) { return ep }, recordFunc: func(ctx context.Context, evalParams engif.ActionsParams) { - logger.BusinessRecord(ctx).Project = "bar" - - logger.BusinessRecord(ctx).Resource = "foo/repo" + logger.BusinessRecord(ctx).Project = testUUID + logger.BusinessRecord(ctx).Repository = testUUID logger.BusinessRecord(ctx).AddRuleEval(evalParams) }, expected: `{ - "project": "bar", - "resource": "foo/repo", + "project": "00000000-0000-0000-0000-000000000001", + "repository": "00000000-0000-0000-0000-000000000001", "rules": [ { - "rule_name": "artifact_signature", - "profile_name": "artifact_profile", - "eval_result": "failure", - "actions": { - "alert": { - "state": "off", - "result": "skipped" - }, - "remediate": { - "state": "on", - "result": "success" - } - } + "ruletype": { + "name": "artifact_signature", + "id": "00000000-0000-0000-0000-000000000001" + }, + "profile": { + "name": "artifact_profile", + "id": "00000000-0000-0000-0000-000000000001" + }, + "eval_result": "failure", + "actions": { + "alert": { + "state": "off", + "result": "skipped" + }, + "remediate": { + "state": "on", + "result": "success" + } + } } ] }`, @@ -127,7 +139,7 @@ func TestTelemetryStore_Record(t *testing.T) { recordFunc: func(_ context.Context, _ engif.ActionsParams) { }, expected: `{"telemetry": true}`, - notPresent: []string{"project", "resource", "rules", "login_sha"}, + notPresent: []string{"project", "rules", "login_sha", "repository", "provider", "profile", "ruletype", "artifact", "pr"}, }} count := len(cases) diff --git a/internal/logger/telemetry_store_watermill.go b/internal/logger/telemetry_store_watermill.go index 78aded73ea..893b8f42b2 100644 --- a/internal/logger/telemetry_store_watermill.go +++ b/internal/logger/telemetry_store_watermill.go @@ -18,6 +18,7 @@ import ( "fmt" "github.com/ThreeDotsLabs/watermill/message" + "github.com/google/uuid" "github.com/rs/zerolog" "github.com/stacklok/minder/internal/engine/entities" @@ -44,16 +45,12 @@ func (m *TelemetryStoreWMMiddleware) TelemetryStoreMiddleware(h message.HandlerF return nil, fmt.Errorf("error unmarshalling payload: %w", err) } - typ := inf.Type - ent, err := getEntityID(inf) + // Create a new telemetry store from entity + ts, err := newTelemetryStoreFromEntity(inf) if err != nil { - return nil, fmt.Errorf("error getting entity ID: %w", err) - } - - ts := &TelemetryStore{ - Project: inf.ProjectID.String(), - Provider: inf.Provider, - Resource: resourceFromEntity(typ, ent), + // Log the error but don't fail the event processing, use the returned empty telemetry store instead + logger := zerolog.Ctx(msg.Context()) + logger.Info().Msg("error creating telemetry store from entity") } // Store telemetry data in the context @@ -73,28 +70,57 @@ func (m *TelemetryStoreWMMiddleware) TelemetryStoreMiddleware(h message.HandlerF } } -func resourceFromEntity(typ minderv1.Entity, ent string) string { - return fmt.Sprintf("%s/%s", typ.ToString(), ent) +// newTelemetryStoreFromEntity creates a new telemetry store from an entity. +func newTelemetryStoreFromEntity(inf *entities.EntityInfoWrapper) (*TelemetryStore, error) { + // Create a new telemetry store + ts := &TelemetryStore{} + + // Get the entity UUID - this is the entity we are processing + ent, err := getEntityID(inf) + if err != nil { + // Return an error but also return the telemetry store so we don't fail the event + return ts, fmt.Errorf("error getting entity ID: %w", err) + } + + // Set the provider name and project ID + ts.Provider = inf.Provider + ts.Project = *inf.ProjectID + + // Set the entity telemetry field based on the entity type + switch inf.Type { + case minderv1.Entity_ENTITY_REPOSITORIES: + ts.Repository = ent + case minderv1.Entity_ENTITY_ARTIFACTS: + ts.Artifact = ent + case minderv1.Entity_ENTITY_PULL_REQUESTS: + ts.PullRequest = ent + case minderv1.Entity_ENTITY_BUILD_ENVIRONMENTS: + case minderv1.Entity_ENTITY_UNSPECIFIED: + // Do nothing + } + + return ts, nil } -func getEntityID(inf *entities.EntityInfoWrapper) (string, error) { +// getEntityID returns the entity ID from the entity info wrapper based on its type. +func getEntityID(inf *entities.EntityInfoWrapper) (uuid.UUID, error) { repoID, artID, prID := inf.GetEntityDBIDs() - var ent string + var ent uuid.UUID // In the case of this middleware, we receive entities // to process by the executor. switch inf.Type { case minderv1.Entity_ENTITY_UNSPECIFIED: - return "", fmt.Errorf("unspecified entity type") + return uuid.Nil, fmt.Errorf("unspecified entity type") case minderv1.Entity_ENTITY_BUILD_ENVIRONMENTS: - return "", fmt.Errorf("build environments not supported") + return uuid.Nil, fmt.Errorf("build environments not supported") case minderv1.Entity_ENTITY_REPOSITORIES: - ent = repoID.String() + ent = repoID case minderv1.Entity_ENTITY_ARTIFACTS: - ent = artID.UUID.String() + ent = artID.UUID case minderv1.Entity_ENTITY_PULL_REQUESTS: - ent = prID.UUID.String() + ent = prID.UUID } return ent, nil diff --git a/internal/logger/telemetry_store_watermill_test.go b/internal/logger/telemetry_store_watermill_test.go index f7594bfd6e..0ea15d8f34 100644 --- a/internal/logger/telemetry_store_watermill_test.go +++ b/internal/logger/telemetry_store_watermill_test.go @@ -18,7 +18,6 @@ import ( "bytes" "context" "encoding/json" - "fmt" "testing" "github.com/google/uuid" @@ -90,7 +89,6 @@ func TestTelemetryStoreWMMiddlewareLogsRepositoryInfo(t *testing.T) { require.Equal(t, projectID.String(), logged["project"], "expected project ID to be logged") require.Equal(t, providerName, logged["provider"], "expected provider to be logged") - resourceString := fmt.Sprintf("%s/%s", minderv1.RepositoryEntity.String(), repositoryID.String()) - require.Equal(t, resourceString, logged["resource"], "expected repository ID to be logged") + require.Equal(t, repositoryID.String(), logged["repository"], "expected repository ID to be logged") require.Equal(t, true, logged["telemetry"], "expected telemetry to be logged") }