Skip to content

only prefix lines with the elapsed time in the streams.log#33

Merged
dirk-thomas merged 1 commit intomasterfrom
dirk-thomas/only-prefix-lines-with-ts-in-streams-log
Jun 19, 2020
Merged

only prefix lines with the elapsed time in the streams.log#33
dirk-thomas merged 1 commit intomasterfrom
dirk-thomas/only-prefix-lines-with-ts-in-streams-log

Conversation

@dirk-thomas
Copy link
Copy Markdown
Member

Follow up of #25.

The elapsed time prefix to each line in the log files is helpful for the user to know the point in time a line was printed. But in some cases the unaltered output is needed, e.g. to extract compiler warnings in Jenkins. In those cases the extra prefix is unexpected and confuses out-of-the-box software.

Therefore this PR removes the prefix from all log files but the streams.log. That allows to feed logs like stdout_stderr.log to tools like the Jenkins warnings-ng plugin while still providing users a complete log with timestamps on each line to introspect when certain output appeared and certain commands started / finished.

Copy link
Copy Markdown
Member

@cottsay cottsay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to merge, one thought though.

Comment on lines +119 to +125
if filename == ALL_STREAMS_LOG_FILENAME:
# prefix line with relative time
relative_time = time.monotonic() - self._start_times[job]
# use the same encoding as the default for the opened file
prefix = ('[%.3fs] ' % relative_time).encode(
encoding=locale.getpreferredencoding(False))
h.write(prefix)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fine, but I'm curious if you could write the prefix to the streams log explicitly outside of the loop. Just a thought.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be possible. But then I would need to get the file handle explicitly. So not sure if that would reduce any logic.

@dirk-thomas dirk-thomas merged commit bc19b6d into master Jun 19, 2020
@delete-merged-branch delete-merged-branch bot deleted the dirk-thomas/only-prefix-lines-with-ts-in-streams-log branch June 19, 2020 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Development

Successfully merging this pull request may close these issues.

2 participants