Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix wrong offset in large logs when some are skipped. #190

Merged
merged 1 commit into from
Sep 2, 2022

Conversation

thesvistun
Copy link
Contributor

@thesvistun thesvistun commented Aug 16, 2022

Timestamps in large logs written to console have wrong
positive offset when logs are partly skipped. So you never
see the last timestamp in the situation.

Changes made in commit 5a5e7b9 touch Parse From Finish logic
and began the offset.
They were partly reverted.

@thesvistun thesvistun requested a review from a team as a code owner August 16, 2022 19:20
Timestamps in large logs written to console have wrong
positive offset when logs are partly skipped. So you never
see the last timestamp in the situation.

Changes made in commit 5a5e7b9 touch Parse From Finish logic
and began the offset.
They were partly reverted.
Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

Have you been running this in production, and has it fixed your problem without causing any regressions?

@thesvistun
Copy link
Contributor Author

Hi, basil.
I have been running this in test instance. The change has fixed my problem. The plagin of v1.17 was used for the purpose.. No regressions were noticed. It is inconvenient to use plugins of custom versions on production, so I'm going to wait for the next release.

@basil
Copy link
Member

basil commented Aug 23, 2022

Hi @thesvistun, please install the incremental build from this PR

https://repo.jenkins-ci.org/incrementals/org/jenkins-ci/plugins/timestamper/1.19-rc755.45f101ddd78a/

on your production server and run it for a week or so. Instructions on how to do so are here:

https://www.jenkins.io/doc/book/managing/plugins/#advanced-installation

Once this change has been verified on a production instance without regressions I will perform a release.

@thesvistun
Copy link
Contributor Author

Deal.

@thesvistun
Copy link
Contributor Author

thesvistun commented Sep 2, 2022

Hi, @basil.
No regression has been noticed for more then requested term.

Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@basil basil merged commit 9351b1d into jenkinsci:master Sep 2, 2022
@basil
Copy link
Member

basil commented Sep 2, 2022

Released in 1.19.

@thesvistun
Copy link
Contributor Author

Thank you too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants