Conversation
Dennis-Mircea
left a comment
There was a problem hiding this comment.
I also took a closer look at JobStatusObserver and I see that the recordJobErrorIfPresent method is not covered by this feature, and currently it can emit event messages of any size. I'd say will great to extend the max length support for this method as well.
Additionally, the observeJobManagerExceptions has a TODO in place as of now which brings into light another hot spot for this max cap feature. Currently, the history of exceptions is being retrieved via REST and can have a big size as well, so I'd say it is worth considering fixing that place as well and to wrap everything at once for JobStatusObserver.
| operatorConfig.get(KubernetesOperatorConfigOptions.OPERATOR_EVENT_EXCEPTION_LIMIT); | ||
| int reportedExceptionEventsMaxStackTraceLength = | ||
| operatorConfig.get( | ||
| KubernetesOperatorConfigOptions.OPERATOR_EVENT_EXCEPTION_STACKTRACE_LINES); |
There was a problem hiding this comment.
Wouldn't be better/safer to first deprecate this option that was introduced as part of 1.12.0 operator release and remove it in further releases?
| public static final ConfigOption<Integer> OPERATOR_EVENT_EXCEPTION_STACKTRACE_LINES = | ||
| operatorConfig("events.exceptions.stacktrace-lines") | ||
| public static final ConfigOption<Integer> OPERATOR_EVENT_EXCEPTION_STACKTRACE_MAX_LENGTH = | ||
| operatorConfig("events.exceptions.stacktrace.max.length") |
There was a problem hiding this comment.
I'd say it's good to add a defensive support for negative values, and maybe consider -1 value as unlimited length.
| .append(stacktrace.length() - maxStackTraceLength) | ||
| .append(" more characters)"); | ||
| } else { | ||
| eventMessage.append(stacktrace); |
There was a problem hiding this comment.
nit optional: we can simply this to
if (stacktrace != null && !stacktrace.isBlank()) {
getSubstringWithMaxLength(stacktrace, maxStackTraceLength).ifPresent(eventMessage::append);
if (stacktrace.length() > maxStackTraceLength) {
eventMessage
.append("... (")
.append(stacktrace.length() - maxStackTraceLength)
.append(" more characters)");
}
}
JobStatusObserver capped JobManager exception text in K8s events byline count (stacktrace.split("\n"), keep first N lines). A single unbroken line of any size produces one array element, so it bypassed the cap entirely and could fill the operator heap.
Approach
events.exceptions.stacktrace-lines(line count, default 5)with
events.exceptions.stacktrace.max.length(character count, default2048 — matches the two existing sibling options
exception.stacktrace.max.length/exception.field.max.length; noconfig in the repo defaults to 4096, so 2048 was kept rather than
raised).
maxStackTraceLength + bounded substring, instead of splitting on \n.
Also removes an unbounded 3-4x string-copy chain downstream of the old
cap.
between line-count and character-count semantics).
updated existing tests, generated config docs, and hand-written
event-format docs (both locales).
Testing
Updated existing unit test and added a new test to explicitly cover the bug scenario reported in FlINK-40452.
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 5)