mirror of
https://gitea.com/gitea/act_runner.git
synced 2026-07-22 02:37:45 +00:00
fix(reporter): respect configured log level for job log forwarding (#989)
## Summary - Non-raw_output log entries above the globally configured `log.level` are no longer forwarded to the Gitea job log output - Step output (`raw_output=true`) is always forwarded regardless of level — it is actual job stdout/stderr, not runner internals - State-machine fields (`stepResult`, `jobResult`) are always processed regardless of level, preserving correct tracking for skipped steps (whose `stepResult` is emitted at `DebugLevel` in `step.go`) - Extracts a `shouldAppendLogRow` helper to avoid repeating the combined `!duringSteps() && entry.Level <= log.GetLevel()` guard in three places ## Why not the approach in #677 PR #677 adds `if entry.Level != log.GetLevel() { return nil }` at the top of `Fire()`. That has two bugs: 1. Uses `!=` instead of `>`, so `Error`/`Fatal` entries are dropped when the configured level is `Warn` 2. Returns early before processing `stepResult`/`jobResult` state fields — skipped steps (whose `stepResult` is logged at `DebugLevel`) would never be marked complete This fix instead applies the level guard only at the `r.logRows` append sites, leaving state tracking unconditional. Relates to #409. Reviewed-on: https://gitea.com/gitea/runner/pulls/989 Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com>
This commit is contained in:
@@ -205,7 +205,7 @@ func (r *Reporter) Fire(entry *log.Entry) error {
|
|||||||
urgentState = true
|
urgentState = true
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if !r.duringSteps() {
|
if r.shouldAppendLogRow(entry) {
|
||||||
r.logRows = appendIfNotNil(r.logRows, r.parseLogRow(entry))
|
r.logRows = appendIfNotNil(r.logRows, r.parseLogRow(entry))
|
||||||
}
|
}
|
||||||
r.unlockAndNotify(urgentState)
|
r.unlockAndNotify(urgentState)
|
||||||
@@ -219,7 +219,7 @@ func (r *Reporter) Fire(entry *log.Entry) error {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
if step == nil {
|
if step == nil {
|
||||||
if !r.duringSteps() {
|
if r.shouldAppendLogRow(entry) {
|
||||||
r.logRows = appendIfNotNil(r.logRows, r.parseLogRow(entry))
|
r.logRows = appendIfNotNil(r.logRows, r.parseLogRow(entry))
|
||||||
}
|
}
|
||||||
r.unlockAndNotify(false)
|
r.unlockAndNotify(false)
|
||||||
@@ -246,7 +246,7 @@ func (r *Reporter) Fire(entry *log.Entry) error {
|
|||||||
r.logRows = append(r.logRows, row)
|
r.logRows = append(r.logRows, row)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else if !r.duringSteps() {
|
} else if r.shouldAppendLogRow(entry) {
|
||||||
r.logRows = appendIfNotNil(r.logRows, r.parseLogRow(entry))
|
r.logRows = appendIfNotNil(r.logRows, r.parseLogRow(entry))
|
||||||
}
|
}
|
||||||
if v, ok := entry.Data["stepResult"]; ok && isJobStepEntry(entry) {
|
if v, ok := entry.Data["stepResult"]; ok && isJobStepEntry(entry) {
|
||||||
@@ -576,6 +576,13 @@ func (r *Reporter) duringSteps() bool {
|
|||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// shouldAppendLogRow reports whether a non-raw_output entry should be written
|
||||||
|
// to the job log: only when we are between steps and the entry's level is
|
||||||
|
// within the globally configured log level.
|
||||||
|
func (r *Reporter) shouldAppendLogRow(entry *log.Entry) bool {
|
||||||
|
return !r.duringSteps() && entry.Level <= log.GetLevel()
|
||||||
|
}
|
||||||
|
|
||||||
var stringToResult = map[string]runnerv1.Result{
|
var stringToResult = map[string]runnerv1.Result{
|
||||||
"success": runnerv1.Result_RESULT_SUCCESS,
|
"success": runnerv1.Result_RESULT_SUCCESS,
|
||||||
"failure": runnerv1.Result_RESULT_FAILURE,
|
"failure": runnerv1.Result_RESULT_FAILURE,
|
||||||
|
|||||||
@@ -219,6 +219,59 @@ func TestReporter_Fire(t *testing.T) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestReporter_LogLevelFiltering(t *testing.T) {
|
||||||
|
// Set global level to Info so Debug entries should be filtered.
|
||||||
|
origLevel := log.GetLevel()
|
||||||
|
log.SetLevel(log.InfoLevel)
|
||||||
|
defer log.SetLevel(origLevel)
|
||||||
|
|
||||||
|
client := mocks.NewClient(t)
|
||||||
|
client.On("UpdateLog", mock.Anything, mock.Anything).Return(func(_ context.Context, req *connect_go.Request[runnerv1.UpdateLogRequest]) (*connect_go.Response[runnerv1.UpdateLogResponse], error) {
|
||||||
|
return connect_go.NewResponse(&runnerv1.UpdateLogResponse{
|
||||||
|
AckIndex: req.Msg.Index + int64(len(req.Msg.Rows)),
|
||||||
|
}), nil
|
||||||
|
})
|
||||||
|
client.On("UpdateTask", mock.Anything, mock.Anything).Return(func(_ context.Context, req *connect_go.Request[runnerv1.UpdateTaskRequest]) (*connect_go.Response[runnerv1.UpdateTaskResponse], error) {
|
||||||
|
return connect_go.NewResponse(&runnerv1.UpdateTaskResponse{}), nil
|
||||||
|
})
|
||||||
|
|
||||||
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
|
taskCtx, err := structpb.NewStruct(map[string]any{})
|
||||||
|
require.NoError(t, err)
|
||||||
|
cfg, _ := config.LoadDefault("")
|
||||||
|
reporter := NewReporter(ctx, cancel, client, &runnerv1.Task{Context: taskCtx}, cfg)
|
||||||
|
reporter.RunDaemon()
|
||||||
|
defer func() {
|
||||||
|
require.NoError(t, reporter.Close(""))
|
||||||
|
}()
|
||||||
|
reporter.ResetSteps(2)
|
||||||
|
|
||||||
|
dataStep0 := log.Fields{"stage": "Main", "stepNumber": 0, "raw_output": true}
|
||||||
|
dataStep0Internal := log.Fields{"stage": "Main", "stepNumber": 0}
|
||||||
|
|
||||||
|
// raw_output entries always appear in job log regardless of level.
|
||||||
|
require.NoError(t, reporter.Fire(&log.Entry{Message: "step output", Data: dataStep0, Level: log.InfoLevel}))
|
||||||
|
require.NoError(t, reporter.Fire(&log.Entry{Message: "step debug output", Data: dataStep0, Level: log.DebugLevel}))
|
||||||
|
assert.Equal(t, int64(2), reporter.state.Steps[0].LogLength, "raw_output entries must always be forwarded")
|
||||||
|
|
||||||
|
// Non-raw_output entries during steps are not added to logRows regardless of level.
|
||||||
|
require.NoError(t, reporter.Fire(&log.Entry{Message: "internal info", Data: dataStep0Internal, Level: log.InfoLevel}))
|
||||||
|
require.NoError(t, reporter.Fire(&log.Entry{Message: "internal debug", Data: dataStep0Internal, Level: log.DebugLevel}))
|
||||||
|
|
||||||
|
// stepResult at DebugLevel (skipped step) must still update state even when filtered from log.
|
||||||
|
require.NoError(t, reporter.Fire(&log.Entry{
|
||||||
|
Message: "Skipping step",
|
||||||
|
Data: log.Fields{
|
||||||
|
"stage": "Main",
|
||||||
|
"stepNumber": 1,
|
||||||
|
"stepResult": "skipped",
|
||||||
|
},
|
||||||
|
Level: log.DebugLevel,
|
||||||
|
}))
|
||||||
|
assert.Equal(t, runnerv1.Result_RESULT_SKIPPED, reporter.state.Steps[1].Result,
|
||||||
|
"stepResult at DebugLevel must update step state even when log entry is filtered from job log output")
|
||||||
|
}
|
||||||
|
|
||||||
// TestReporter_EphemeralRunnerDeletion reproduces the exact scenario from
|
// TestReporter_EphemeralRunnerDeletion reproduces the exact scenario from
|
||||||
// https://gitea.com/gitea/runner/issues/793:
|
// https://gitea.com/gitea/runner/issues/793:
|
||||||
//
|
//
|
||||||
|
|||||||
Reference in New Issue
Block a user