[ZEPPELIN-6322] Filter download messages in ProcessData error stream

[celinayk]

What is this PR for?

This PR improves error stream output filtering in the ProcessData class to exclude download-related messages. Currently, Maven/npm download progress information clutters the error stream during integration tests, making it difficult to identify actual errors.
This change filters out download messages (e.g., "Downloading:", "Progress: 45%", "1024/2048 KB") while preserving real error messages.

What type of PR is it?

Improvement

Todos

• - Add DOWNLOAD_PATTERNS array with 6 regex patterns
• - Implement isDownloadMessage() method
• - Apply filtering logic in buildOutputAndErrorStreamData()

What is the Jira issue?

ZEPPELIN-6322

How should this be tested?

Screenshots (if appropriate)

Questions:

• Does the license files need to update? No
• Is there breaking changes for older versions? No
• Does this needs documentation? No

[tbonelee]

Thanks for tackling this. The direction makes sense, and pulling the patterns into a constant array with an isDownloadMessage() helper reads cleanly.

I do want to raise one concern up front, because I think it should drive the whole design: this change can silently hide real error messages, and in test output that's a costly failure mode. When an integration test fails, these error lines are often the only clue we have. A filter that removes the wrong line doesn't just add noise, it can make a real failure look like it never happened, or send someone debugging in the wrong direction for hours. So I'd argue we should approach this as conservatively as possible: decluttering is a nice-to-have, but never dropping a genuine error is a hard requirement. A few points through that lens:

1. The current patterns are broad enough to swallow real errors

Several patterns match anywhere in a line, so they will catch legitimate messages that merely happen to contain a size or percentage figure, for example:

• .*\d+/\d+\s*(KB|MB|GB|bytes).* would match Task failed after writing 1024/2048 MB
• .*progress:\s*\d+%.* would match Build failed at progress: 50%

Worse, the download check runs before the error/failed check, so a matched line is discarded before we ever get a chance to recognize it as an error. That's exactly the case we can't afford.

2. Prefer downgrading over dropping, so nothing is ever lost

Given the above, I'd suggest we never fully drop a line. Routing suspected download lines to LOGGER.trace() instead of removing them keeps the default output just as clean, while guaranteeing a misfire can never erase a real error, only move it to a quieter log level:

if (!temp.trim().isEmpty()) {
Stringlower = temp.toLowerCase();
if (lower.contains("error") || lower.contains("failed")) {
LOGGER.warn(temp.trim()); // real errors: always visible, never filtered
} elseif (isDownloadMessage(temp)) {
LOGGER.trace(temp.trim()); // hidden by default, but recoverable
} else {
LOGGER.debug(temp.trim());
}
}

This ordering also guarantees anything containing error/failed bypasses the filter entirely. With this in place, a wrong pattern match becomes "logged at the wrong level" rather than "lost", which is the safety margin I think this change needs.

3. The filter runs on buffer chunks, not lines

Separately, buildOutputAndErrorStreamData() reads the error stream in fixed 300-char chunks (BUFFER_LEN), so temp usually holds several lines and partial lines, not one message. Since Pattern.matches() must match the entire input and .* doesn't cross \n without DOTALL, any chunk containing a newline matches nothing, so most real download output slips through unfiltered anyway. Splitting temp on \n and filtering per line would make it actually work, and would let the patterns be anchored with ^ to further shrink accidental matches.

4. Minor: this affects console logging, not the returned error stream

sbErrorStream.append(tempSB) runs before the filter, so getErrorStream() still returns the full text. That's fine if the intent is purely to declutter CI console logs, but since the title says "error stream" it may be worth clarifying the wording or the intended target.

Once the approach settles, a unit test on isDownloadMessage() would be valuable, especially a case with a real error line that contains a size/percentage figure, since that's the exact scenario we're trying not to regress on.

[celinayk]

[ZEPPELIN-6322] Filter download messages in ProcessData error stream

Thanks for the thorough review, @tbonelee — really appreciate the depth here. Pushed a fix in c8225fa that addresses all four points:

1. Broad patterns swallowing real errors — anchored all DOWNLOAD_PATTERNS with ^ so they only match at the start of a line, instead of matching anywhere via unanchored .*. Task failed after writing 1024/2048 MB and Build failed at progress: 50% no longer match at all.

2. Downgrade instead of drop — reordered exactly as you suggested: error/failed content is checked first and always goes to LOGGER.warn(...), bypassing the download filter entirely. Lines that look like
   download noise are routed to LOGGER.trace(...) instead of being dropped, so a misclassification can only make a line quieter, never lose it.

3. Filtering ran on buffer chunks, not lines — went a bit further than splitting temp on \n: added a pendingErrorLine buffer so a line split across two BUFFER_LEN reads (or even two outer-loop
   iterations) gets reassembled into a complete line before classification. Any trailing unterminated content is flushed once the stream ends, so nothing at the tail gets silently swallowed either.

4. Wording clarification — added a comment above sbErrorStream.append(tempSB) noting that filtering only affects console log verbosity; getErrorStream() always returns the full, unfiltered content.

Also added ProcessDataTest#doesNotFilterOutRealErrorLines(), which includes both of your examples (Task failed after writing 1024/2048 MB, Build failed at progress: 50%) as regression cases, plus coverage for
the download-pattern matches and the chunk-boundary line-splitting scenario.

Let me know if anything still looks off

[tbonelee]

Merged into master