diff options
| -rw-r--r-- | internal/git/command_factory.go | 9 | ||||
| -rw-r--r-- | internal/git/command_factory_test.go | 7 | ||||
| -rw-r--r-- | internal/git/trace2/manager.go | 21 | ||||
| -rw-r--r-- | internal/git/trace2/manager_test.go | 15 |
4 files changed, 29 insertions, 23 deletions
diff --git a/internal/git/command_factory.go b/internal/git/command_factory.go index dcf5d7c9d..1bcd3ac41 100644 --- a/internal/git/command_factory.go +++ b/internal/git/command_factory.go @@ -602,13 +602,16 @@ func (cf *ExecCommandFactory) GlobalConfiguration(ctx context.Context) ([]Config func (cf *ExecCommandFactory) trace2Finalizer(manager *trace2.Manager) func(context.Context, *command.Command) { return func(ctx context.Context, cmd *command.Command) { - manager.Finish(ctx) + trace, err := manager.Finish(ctx) stats := command.StatsFromContext(ctx) if stats != nil { stats.RecordMetadata("trace2.activated", "true") stats.RecordMetadata("trace2.hooks", strings.Join(manager.HookNames(), ",")) - if manager.Error() != nil { - stats.RecordMetadata("trace2.error", manager.Error().Error()) + if trace != nil { + stats.RecordMetadata("trace2.events", trace.Inspect(true)) + } + if err != nil { + stats.RecordMetadata("trace2.error", err.Error()) } } } diff --git a/internal/git/command_factory_test.go b/internal/git/command_factory_test.go index f234bbb8c..16df0b056 100644 --- a/internal/git/command_factory_test.go +++ b/internal/git/command_factory_test.go @@ -856,6 +856,13 @@ func TestWithTrace2Hooks(t *testing.T) { for key, value := range tc.expectedStats { require.Equal(t, value, fields[key]) } + if _, exists := tc.expectedStats["trace2.activated"]; exists { + loggedEvents := fields["trace2.events"] + require.NotNil(t, loggedEvents) + for _, event := range essentialEvents { + require.Contains(t, loggedEvents, event) + } + } } else { require.Nil(t, command.StatsFromContext(ctx)) } diff --git a/internal/git/trace2/manager.go b/internal/git/trace2/manager.go index 229a71dcd..2ab0db10e 100644 --- a/internal/git/trace2/manager.go +++ b/internal/git/trace2/manager.go @@ -35,11 +35,6 @@ func (m *Manager) HookNames() []string { return names } -// Error returns the error occurs after the manager finishes -func (m *Manager) Error() error { - return m.err -} - // Inject injects the path to the tempfile used to store trace2 events and conventional environment // variables to the input ENV list. func (m *Manager) Inject(env []string) []string { @@ -75,9 +70,9 @@ func (m *Manager) Inject(env []string) []string { } // Finish reads the events, parses them to a tree, triggers hook handlers, and clean up the fd. -func (m *Manager) Finish(ctx context.Context) { - if m.Error() != nil { - return +func (m *Manager) Finish(ctx context.Context) (*Trace, error) { + if m.err != nil { + return nil, m.err } defer func() { @@ -97,17 +92,15 @@ func (m *Manager) Finish(ctx context.Context) { trace, err := Parse(ctx, m.fd) if err != nil { - m.err = fmt.Errorf("trace2: parsing events: %w", err) - return + return nil, fmt.Errorf("trace2: parsing events: %w", err) } if trace == nil { - m.err = fmt.Errorf("trace2: no events to handle") - return + return nil, fmt.Errorf("trace2: no events to handle") } for _, hook := range m.hooks { if err := hook.Handle(ctx, trace); err != nil { - m.err = fmt.Errorf("trace2: executing %q handler: %w", hook.Name(), err) - return + return nil, fmt.Errorf("trace2: executing %q handler: %w", hook.Name(), err) } } + return trace, nil } diff --git a/internal/git/trace2/manager_test.go b/internal/git/trace2/manager_test.go index bdbfd25b0..d84edf627 100644 --- a/internal/git/trace2/manager_test.go +++ b/internal/git/trace2/manager_test.go @@ -124,14 +124,15 @@ func TestManager(t *testing.T) { require.Nil(t, manager.fd) } - manager.Finish(testhelper.Context(t)) - require.NoError(t, manager.Error()) + trace, err := manager.Finish(testhelper.Context(t)) + require.NoError(t, err) if activated { require.NoFileExists(t, manager.fd.Name()) } if assert != nil { assert(t, manager) } + require.Equal(t, expectedTrace, trace.Inspect(false)) }) } } @@ -181,10 +182,11 @@ func TestManager_tempfileFailures(t *testing.T) { _ = manager.Inject([]string{}) tc.setup(t, manager) - manager.Finish(testhelper.Context(t)) + trace, err := manager.Finish(testhelper.Context(t)) - require.Regexp(t, tc.expectedError, manager.Error().Error()) + require.Regexp(t, tc.expectedError, err.Error()) require.NoFileExists(t, manager.fd.Name()) + require.Nil(t, trace) }) } } @@ -215,8 +217,9 @@ func TestManager_handlerFailures(t *testing.T) { events := testhelper.MustReadFile(t, "testdata/git-pack-objects.event") require.NoError(t, os.WriteFile(manager.fd.Name(), events, os.ModeAppend)) - manager.Finish(testhelper.Context(t)) + trace, err := manager.Finish(testhelper.Context(t)) - require.Equal(t, `trace2: executing "dummy2" handler: something goes wrong`, manager.Error().Error()) + require.Equal(t, `trace2: executing "dummy2" handler: something goes wrong`, err.Error()) require.NoFileExists(t, manager.fd.Name()) + require.Nil(t, trace) } |