diff --git a/events/event_recorder.go b/events/event_recorder.go index ff4908d5f..b07cc412b 100644 --- a/events/event_recorder.go +++ b/events/event_recorder.go @@ -14,6 +14,7 @@ import ( ) const maxErrorMessageLength = 104857600 //100KB +const truncationIndicator = "... ..." type recordingMetrics struct { EventRecordingFailure labeled.StopWatch @@ -85,7 +86,7 @@ func (r *eventRecorder) RecordWorkflowEvent(ctx context.Context, e *event.Workfl // the beginning and the end of the message to capture the most relevant information. func truncateErrorMessage(err *core.ExecutionError, length int) { if len(err.Message) > length { - err.Message = fmt.Sprintf("%s%s", err.Message[:length/2], err.Message[(len(err.Message)-length/2):]) + err.Message = fmt.Sprintf("%s\n%s\n%s", err.Message[:length/2], truncationIndicator, err.Message[(len(err.Message)-length/2):]) } } diff --git a/events/event_recorder_test.go b/events/event_recorder_test.go index 5850c0f33..cd4ab5a1d 100644 --- a/events/event_recorder_test.go +++ b/events/event_recorder_test.go @@ -96,6 +96,6 @@ func TestTruncateErrorMessage(t *testing.T) { } truncateErrorMessage(&executionError, length) - assert.True(t, len(executionError.Message) <= length) + assert.True(t, len(executionError.Message) <= length+len(truncationIndicator)+2) } } diff --git a/pkg/controller/nodes/task/config/config.go b/pkg/controller/nodes/task/config/config.go index 36bdfdb48..4bc2937c5 100644 --- a/pkg/controller/nodes/task/config/config.go +++ b/pkg/controller/nodes/task/config/config.go @@ -29,7 +29,6 @@ var ( BaseSecond: 2, MaxDuration: config.Duration{Duration: time.Second * 20}, }, - MaxErrorMessageLength: 2048, } section = config.MustRegisterSection(SectionKey, defaultConfig) @@ -40,7 +39,7 @@ type Config struct { MaxPluginPhaseVersions int32 `json:"max-plugin-phase-versions" pflag:",Maximum number of plugin phase versions allowed for one phase."` BarrierConfig BarrierConfig `json:"barrier" pflag:",Config for Barrier implementation"` BackOffConfig BackOffConfig `json:"backoff" pflag:",Config for Exponential BackOff implementation"` - MaxErrorMessageLength int `json:"maxLogMessageLength" pflag:",Max length of error message."` + MaxErrorMessageLength int `json:"maxLogMessageLength" pflag:",Deprecated!!! Max length of error message."` } type BarrierConfig struct { diff --git a/pkg/controller/nodes/task/config/config_flags.go b/pkg/controller/nodes/task/config/config_flags.go index 09a3465ed..a77a6f58e 100755 --- a/pkg/controller/nodes/task/config/config_flags.go +++ b/pkg/controller/nodes/task/config/config_flags.go @@ -57,6 +57,6 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "barrier.cache-ttl"), defaultConfig.BarrierConfig.CacheTTL.String(), " Max duration that a barrier would be respected if the process is not restarted. This should account for time required to store the record into persistent storage (across multiple rounds.") cmdFlags.Int(fmt.Sprintf("%v%v", prefix, "backoff.base-second"), defaultConfig.BackOffConfig.BaseSecond, "The number of seconds representing the base duration of the exponential backoff") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "backoff.max-duration"), defaultConfig.BackOffConfig.MaxDuration.String(), "The cap of the backoff duration") - cmdFlags.Int(fmt.Sprintf("%v%v", prefix, "maxLogMessageLength"), defaultConfig.MaxErrorMessageLength, "Max length of error message.") + cmdFlags.Int(fmt.Sprintf("%v%v", prefix, "maxLogMessageLength"), defaultConfig.MaxErrorMessageLength, "Deprecated!!! Max length of error message.") return cmdFlags } diff --git a/pkg/controller/nodes/task/pre_post_execution.go b/pkg/controller/nodes/task/pre_post_execution.go index 1a6903b76..7ee10700e 100644 --- a/pkg/controller/nodes/task/pre_post_execution.go +++ b/pkg/controller/nodes/task/pre_post_execution.go @@ -156,10 +156,6 @@ func (t *Handler) ValidateOutputAndCacheAdd(ctx context.Context, nodeID v1alpha1 if taskErr.ExecutionError == nil { taskErr.ExecutionError = &core.ExecutionError{Kind: core.ExecutionError_UNKNOWN, Code: "Unknown", Message: "Unknown"} } - // Errors can be arbitrary long since they are written by containers/potentially 3rd party plugins. This ensures - // the error message length will never be big enough to cause write failures to Etcd. or spam Admin DB with huge - // objects. - taskErr.Message = trimErrorMessage(taskErr.Message, t.cfg.MaxErrorMessageLength) return cacheDisabled, &taskErr, nil } diff --git a/pkg/controller/nodes/task/transformer.go b/pkg/controller/nodes/task/transformer.go index d8bbc2617..c668d1d49 100644 --- a/pkg/controller/nodes/task/transformer.go +++ b/pkg/controller/nodes/task/transformer.go @@ -48,14 +48,6 @@ func ToTaskEventPhase(p pluginCore.Phase) core.TaskExecution_Phase { } } -func trimErrorMessage(original string, maxLength int) string { - if len(original) <= maxLength { - return original - } - - return original[0:maxLength/2] + original[len(original)-maxLength/2:] -} - func getParentNodeExecIDForTask(taskExecID *core.TaskExecutionIdentifier, execContext executors.ExecutionContext) (*core.NodeExecutionIdentifier, error) { nodeExecutionID := &core.NodeExecutionIdentifier{ ExecutionId: taskExecID.NodeExecutionId.ExecutionId, diff --git a/pkg/controller/nodes/task/transformer_test.go b/pkg/controller/nodes/task/transformer_test.go index e1310921a..7b4a42b5b 100644 --- a/pkg/controller/nodes/task/transformer_test.go +++ b/pkg/controller/nodes/task/transformer_test.go @@ -37,29 +37,6 @@ func TestToTaskEventPhase(t *testing.T) { assert.Equal(t, core.TaskExecution_QUEUED, ToTaskEventPhase(pluginCore.PhaseQueued)) } -func Test_trimErrorMessage(t *testing.T) { - const inputStr = "0123456789" - t.Run("Length less or equal than max", func(t *testing.T) { - input := inputStr - assert.Equal(t, input, trimErrorMessage(input, 10)) - }) - - t.Run("Length > max", func(t *testing.T) { - input := inputStr - assert.Equal(t, "01236789", trimErrorMessage(input, 8)) - }) - - t.Run("Odd Max", func(t *testing.T) { - input := inputStr - assert.Equal(t, "01236789", trimErrorMessage(input, 9)) - }) - - t.Run("Odd input", func(t *testing.T) { - input := "012345678" - assert.Equal(t, "012345678", trimErrorMessage(input, 9)) - }) -} - func TestToTaskExecutionEvent(t *testing.T) { tkID := &core.Identifier{} nodeID := &core.NodeExecutionIdentifier{}