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 calculated build time in custom data by using System.currentTimeM… #52

Merged
merged 3 commits into from
Dec 14, 2018

Conversation

valtaroth
Copy link
Contributor

The calculated build time for custom data is terribly wrong for pipeline builds where the build is still running and the duration cannot be fetched from Jenkins itself. The timestamp provided to CustomDataPointGenerator used for the calculation is in nanoseconds while it is required to be in milliseconds. Since JenkinsBasePointGenerator is calculating the timestamp using System.currentTimeMillis(), I added that calculation here too. Alternatively, if the provided timestamp is supposed to be used although it isn't in the base point generation, the start time needs to be converted to nanoseconds before calculating the duration and the result converted back to milliseconds.

@valtaroth valtaroth changed the base branch from master to development November 27, 2018 09:36
@asimell
Copy link
Contributor

asimell commented Dec 4, 2018

The reason why it uses the timestamp is, because there is an option to use build scheduled time as the timestamp instead of the time the point is generated. So, the solution should be to convert to nanoseconds and then back to milliseconds.

@asimell
Copy link
Contributor

asimell commented Dec 13, 2018

Actually, the timestamp / 1000000 is what is supposed to be in startTime and currTime should be System.currentTimeMillis(). Otherwise, having the start time as job scheduled time will result in the build_time to always be 0 (build.getTimeInMillis() - build.getTimeInMillis())

@valtaroth
Copy link
Contributor Author

Sorry, I totally misunderstood you then. I flipped it now, should hopefully be fine.

@asimell asimell merged commit b201da8 into jenkinsci:development Dec 14, 2018
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.

3 participants