diff --git a/internal/storage/git/metrics.go b/internal/storage/git/metrics.go index 0f4fe6abea..4cebaa0a7a 100644 --- a/internal/storage/git/metrics.go +++ b/internal/storage/git/metrics.go @@ -17,8 +17,11 @@ const ( ) var ( - attrRemote = attribute.Key("remote") - attrBranch = attribute.Key("branch") + attrRemote = attribute.Key("remote") + attrBranch = attribute.Key("branch") + attrStatus = attribute.Key("status") + attrErrorType = attribute.Key("error_type") + attrOperation = attribute.Key("operation") ) type repoMetrics struct { @@ -38,6 +41,14 @@ type repoMetrics struct { branchesTotal metric.Int64UpDownCounter branchesErrorsTotal metric.Int64Counter + + // Sync operation metrics + syncTotal metric.Int64Counter + syncErrorsTotal metric.Int64Counter + syncDuration metric.Float64Histogram + + // Change tracking metrics + filesChanged metric.Int64Counter } func withRemote(name string) containers.Option[repoMetrics] { @@ -90,6 +101,23 @@ func newRepoMetrics(opts ...containers.Option[repoMetrics]) repoMetrics { prometheus.BuildFQName(namespace, subsystem, "branches_errors_total"), metric.WithDescription("The total number of errors observed creating or deleting branches"), ), + syncTotal: metrics.MustInt64().Counter( + prometheus.BuildFQName(namespace, subsystem, "sync_total"), + metric.WithDescription("The total number of sync operations (fetch)"), + ), + syncErrorsTotal: metrics.MustInt64().Counter( + prometheus.BuildFQName(namespace, subsystem, "sync_errors_total"), + metric.WithDescription("The total number of sync operation errors"), + ), + syncDuration: metrics.MustFloat64().Histogram( + prometheus.BuildFQName(namespace, subsystem, "sync_duration"), + metric.WithDescription("The duration of sync operations in milliseconds"), + metric.WithUnit("ms"), + ), + filesChanged: metrics.MustInt64().Counter( + prometheus.BuildFQName(namespace, subsystem, "files_changed_total"), + metric.WithDescription("The total number of files changed during sync operations"), + ), } containers.ApplyAll(&m, opts...) @@ -152,3 +180,40 @@ func (r repoMetrics) recordBranchDeleted(ctx context.Context) func(error) { } } } + +func (r repoMetrics) recordSyncStart(ctx context.Context, branch string) func(error) { + start := time.Now().UTC() + return func(err error) { + duration := float64(time.Since(start).Milliseconds()) + branchAttrs := metric.WithAttributes(append(r.attrs, attrBranch.String(branch))...) + r.syncDuration.Record(ctx, duration, branchAttrs) + + if err != nil { + statusAttrs := metric.WithAttributes(append(r.attrs, + attrBranch.String(branch), + attrStatus.String("failure"))...) + r.syncTotal.Add(ctx, 1, statusAttrs) + } else { + statusAttrs := metric.WithAttributes(append(r.attrs, + attrBranch.String(branch), + attrStatus.String("success"))...) + r.syncTotal.Add(ctx, 1, statusAttrs) + } + } +} + +func (r repoMetrics) recordSyncError(ctx context.Context, branch, errorType string) { + attrs := metric.WithAttributes(append(r.attrs, + attrBranch.String(branch), + attrErrorType.String(errorType))...) + r.syncErrorsTotal.Add(ctx, 1, attrs) +} + +func (r repoMetrics) recordFilesChanged(ctx context.Context, branch, operation string, count int) { + if count > 0 { + attrs := metric.WithAttributes(append(r.attrs, + attrBranch.String(branch), + attrOperation.String(operation))...) + r.filesChanged.Add(ctx, int64(count), attrs) + } +} diff --git a/internal/storage/git/metrics_test.go b/internal/storage/git/metrics_test.go new file mode 100644 index 0000000000..a0001ca60b --- /dev/null +++ b/internal/storage/git/metrics_test.go @@ -0,0 +1,73 @@ +package git + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestCategorizeError(t *testing.T) { + tests := []struct { + name string + err error + expected string + }{ + { + name: "unknown error", + err: assert.AnError, + expected: "unknown", + }, + { + name: "nil error", + err: nil, + expected: "unknown", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := categorizeError(tt.err) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestRepoMetrics(t *testing.T) { + metrics := newRepoMetrics(withRemote("test-remote")) + ctx := context.Background() + + t.Run("recordSyncStart with success", func(t *testing.T) { + finished := metrics.recordSyncStart(ctx, "main") + // Should not panic when called with nil error (success) + finished(nil) + }) + + t.Run("recordSyncStart with error", func(t *testing.T) { + finished := metrics.recordSyncStart(ctx, "main") + // Should not panic when called with error + finished(assert.AnError) + }) + + t.Run("recordSyncError", func(t *testing.T) { + // Should not panic + metrics.recordSyncError(ctx, "main", "network") + }) + + t.Run("recordFilesChanged", func(t *testing.T) { + // Should not panic + metrics.recordFilesChanged(ctx, "main", "added", 5) + metrics.recordFilesChanged(ctx, "main", "modified", 3) + metrics.recordFilesChanged(ctx, "main", "deleted", 1) + }) +} + +func TestRepoMetricsAttributes(t *testing.T) { + metrics := newRepoMetrics(withRemote("github.com/test/repo")) + + // Test that metrics can be created with remote attribute + assert.NotNil(t, metrics.syncTotal) + assert.NotNil(t, metrics.syncErrorsTotal) + assert.NotNil(t, metrics.syncDuration) + assert.NotNil(t, metrics.filesChanged) +} diff --git a/internal/storage/git/repository.go b/internal/storage/git/repository.go index 39357617fd..f2186895ec 100644 --- a/internal/storage/git/repository.go +++ b/internal/storage/git/repository.go @@ -2,6 +2,7 @@ package git import ( "context" + stderrors "errors" "fmt" "maps" "os" @@ -27,6 +28,44 @@ import ( "go.uber.org/zap" ) +// categorizeError categorizes git operation errors using go-git error types +func categorizeError(err error) string { + // Check for transport-level authentication/authorization errors + if stderrors.Is(err, transport.ErrAuthenticationRequired) || + stderrors.Is(err, transport.ErrAuthorizationFailed) || + stderrors.Is(err, transport.ErrInvalidAuthMethod) { + return "auth" + } + + // Check for repository/network errors + if stderrors.Is(err, transport.ErrRepositoryNotFound) || + stderrors.Is(err, transport.ErrEmptyRemoteRepository) { + return "network" + } + + // Check for conflict/merge related errors + if stderrors.Is(err, git.ErrNonFastForwardUpdate) || + stderrors.Is(err, git.ErrWorktreeNotClean) || + stderrors.Is(err, git.ErrUnstagedChanges) { + return "conflict" + } + + // Check for repository structure errors + if stderrors.Is(err, git.ErrRepositoryNotExists) || + stderrors.Is(err, git.ErrBranchNotFound) || + stderrors.Is(err, git.ErrBranchExists) { + return "repository" + } + + // Check for fetch/network errors + if stderrors.Is(err, git.ErrRemoteRefNotFound) || + stderrors.Is(err, git.ErrFetching) { + return "network" + } + + return "unknown" +} + type Repository struct { *git.Repository @@ -164,7 +203,7 @@ func newRepository(ctx context.Context, logger *zap.Logger, opts ...containers.O // do an initial fetch to setup remote tracking branches if err := r.Fetch(ctx); err != nil { - if !errors.Is(err, transport.ErrEmptyRemoteRepository) && !errors.Is(err, git.ErrRemoteRefNotFound) { + if !stderrors.Is(err, transport.ErrEmptyRemoteRepository) && !stderrors.Is(err, git.ErrRemoteRefNotFound) { return nil, empty, fmt.Errorf("performing initial fetch: %w", err) } @@ -271,6 +310,43 @@ func (r *Repository) Fetch(ctx context.Context, specific ...string) (err error) return nil } + // Track sync operation for the default branch (or first head if specific) + branch := r.defaultBranch + if len(specific) > 0 { + branch = specific[0] + } + + // Record sync start and get completion callback + finished := r.metrics.recordSyncStart(ctx, branch) + + // Store initial HEAD for change detection + beforeRef, _ := r.Head() + var beforeHash plumbing.Hash + if beforeRef != nil { + beforeHash = beforeRef.Hash() + } + + defer func() { + // Always call finished to record metrics + finished(err) + + // Record error categorization if error occurred + if err != nil && !stderrors.Is(err, git.NoErrAlreadyUpToDate) { + errorType := categorizeError(err) + r.metrics.recordSyncError(ctx, branch, errorType) + } else { + // Track changes if HEAD moved + afterRef, _ := r.Head() + var afterHash plumbing.Hash + if afterRef != nil { + afterHash = afterRef.Hash() + } + if beforeHash != afterHash { + r.trackSyncChanges(ctx, branch, beforeHash, afterHash) + } + } + }() + updatedRefs := map[string]plumbing.Hash{} r.mu.Lock() defer func() { @@ -304,7 +380,7 @@ func (r *Repository) Fetch(ctx context.Context, specific ...string) (err error) CABundle: r.caBundle, InsecureSkipTLS: r.insecureSkipTLS, RefSpecs: refSpecs, - }); err != nil && !errors.Is(err, git.NoErrAlreadyUpToDate) { + }); err != nil && !stderrors.Is(err, git.NoErrAlreadyUpToDate) { return err } @@ -677,6 +753,59 @@ func (r *Repository) newFilesystem(hash plumbing.Hash) (_ *filesystem, err error ) } +// trackSyncChanges tracks file changes between two commits +func (r *Repository) trackSyncChanges(ctx context.Context, branch string, beforeHash, afterHash plumbing.Hash) { + if beforeHash == afterHash { + return + } + + beforeCommit, err := r.CommitObject(beforeHash) + if err != nil { + r.logger.Debug("failed to get before commit for change tracking", zap.Error(err)) + return + } + + afterCommit, err := r.CommitObject(afterHash) + if err != nil { + r.logger.Debug("failed to get after commit for change tracking", zap.Error(err)) + return + } + + beforeTree, err := beforeCommit.Tree() + if err != nil { + r.logger.Debug("failed to get before tree for change tracking", zap.Error(err)) + return + } + + afterTree, err := afterCommit.Tree() + if err != nil { + r.logger.Debug("failed to get after tree for change tracking", zap.Error(err)) + return + } + + changes, err := beforeTree.Diff(afterTree) + if err != nil { + r.logger.Debug("failed to diff trees for change tracking", zap.Error(err)) + return + } + + var added, modified, deleted int + for _, change := range changes { + switch { + case change.From.Name == "": + added++ + case change.To.Name == "": + deleted++ + default: + modified++ + } + } + + r.metrics.recordFilesChanged(ctx, branch, "added", added) + r.metrics.recordFilesChanged(ctx, branch, "modified", modified) + r.metrics.recordFilesChanged(ctx, branch, "deleted", deleted) +} + func WithRemote(name, url string) containers.Option[Repository] { return func(r *Repository) { r.remote = &config.RemoteConfig{ diff --git a/magefile.go b/magefile.go index 945f478e4b..a29852583e 100644 --- a/magefile.go +++ b/magefile.go @@ -392,8 +392,8 @@ func findFilesRecursive(match func(path string, info os.FileInfo) bool) ([]strin return err } - // Don't look for files in git directories - if info.Mode().IsDir() && filepath.Base(path) == ".git" { + // Don't look for files in hidden directories + if info.Mode().IsDir() && strings.HasPrefix(filepath.Base(path), ".") { return filepath.SkipDir }