feat: CP-1062 Remove excessive information returned from reading log files #3710
Conversation
| msg := logLine.Body.Msg | ||
|
|
||
| if !startPrinting { | ||
| if msg == "Dependencies downloaded and unpacked." { |
There was a problem hiding this comment.
This feels very brittle, due to how implicit it is. Now we have downstream automation logic depending on a localization on some other API that has zero awareness of this automation.
Is there something else we can use, something less prone to change?
Could you instead filter out any log messages that are not errors or warnings? ie. filter out DEBUG and INFO.
There was a problem hiding this comment.
The errors are logged as INFO:
{"body": {"facility": "INFO", "msg": "ModuleNotFoundError: No module named 'poetry'"}, "artifact_id": "0fc44508-4e7e-58f6-8553-58592a52b04c", "timestamp": "2025-08-12T19:24:19.211884", "type": "artifact_progress", "source": "python", "pid": 124, "pipe_name": "stderr"}
We should do a better job classifying these log levels. Additionally, there could be something in the logs that tell where in the process it is (downloading, unpacking, building...)? Or perhaps these strings like "Dependencies downloaded and unpacked." are already intended for that and we just want to make it more explicit?
There was a problem hiding this comment.
We are empowered to make changes throughout the whole codebase, and this feels like a relatively simple one. Perhaps you could modify the log line where "Dependencies downloaded and unpacked" and add some form of context to make the relationship explicit? At the very least this could be a comment stating "If you change this also change this file here: X". But ideally we'd add something specifically meant for programmatic interpretation, ie. some sort of status code or something that explicitly communicates "this is for programmatic use".
Then again I imagine you might be looking at failed builds that predate this change? If that's the case I suppose a comment is the best we can do for now, though I'd challenge you to think through the problem and see if there's a clean solution.
samueld-activator
force-pushed
the
samueld/CP-1062
branch
from
f2fd2c3 to
24aa177
Compare
|
We should be fine looking for "error" +- 10 lines, at least for now that we are only fixing missing dependencies. My concern is about missing important information that were "farther" in the logs, but we are already risking to lose when we only return what comes after "Dependencies downloaded and unpacked.". Additionally, 10 seems to be a good default. |
| // Check what lines contain the keyword "error" and print the previous 10 and next 10 lines | ||
| printedLines := make(map[int]bool) | ||
| for i, line := range lines { | ||
| if strings.Contains(strings.ToLower(line), "error") { |
There was a problem hiding this comment.
Should perhaps include some more synonyms here, eg.
fail
critical
panic
exception
…files