Fix queue time calculation for builds and pipeline steps#406
Conversation
nikita-tkachenko-datadog
added
the
changelog/Fixed
0f47c16 to
d2537c8
Compare
drodriguezhdez
left a comment
drodriguezhdez
left a comment
There was a problem hiding this comment.
Dropped some comments.
| // At this point, the current node is traceable. | ||
| final TraceSpan.TraceSpanContext spanContext = new TraceSpan.TraceSpanContext(current.getTraceId(), current.getParentSpanId(), current.getSpanId()); | ||
| final TraceSpan span = new TraceSpan(buildOperationName(current), fixedStartTimeNanos + current.getNanosInQueue(), spanContext); | ||
| final TraceSpan span = new TraceSpan(buildOperationName(current), startTimeNanos + current.getQueueTimeMillis(), spanContext); |
There was a problem hiding this comment.
Are we removing the queue time twice? We're doing the same in this part of the code and we're not doing it for webhooks protocol.
There was a problem hiding this comment.
Good catch, fixed!
| * This queue time goes AFTER timestamp that is reported by {@link Run#getStartTimeInMillis()} | ||
| * and is INCLUDED into the duration reported by {@link Run#getDuration()}. | ||
| */ | ||
| private long propagatedMillisInQueue; |
There was a problem hiding this comment.
Do we know how does this work when the pipeline uses multiple agents?
https://www.jenkins.io/doc/book/pipeline/jenkinsfile/#using-multiple-agents
There was a problem hiding this comment.
As BuildData represents the entire build, only the time spent waiting for the agent specified in the top-level agent {} section is propagated, which I think is the correct behaviour.
For individual stages the time spent waiting for their respective agents will be accounted for in the PipelineStepData instances
| queueInfoAction.setQueueTimeMillis(waitingMs); | ||
| if (waitingMs != null) { | ||
| try (Metrics metrics = client.metrics()) { | ||
| metrics.gauge("jenkins.job.waiting", TimeUnit.MILLISECONDS.toSeconds(waitingMs), hostname, tags); |
There was a problem hiding this comment.
Not sure if this metric has the same meaning than CI Visibility Queue Time, but here we're not taking into account the "propagated" queue time (for Jenkins Declarative Pipelines using agent)
There was a problem hiding this comment.
I'm not sure either, the documentation just says "Time spent waiting for job to run".
I'd leave it as is, at least for now, given that there haven't been any customer complaints
This reverts commit 26f09a7.
2 participants
Requirements for Contributing to this repository
What does this PR do?
Fixes the logic that calculates queue time for pipelines and pipeline steps.
The following problems are fixed:
Description of the Change
Alternate Designs
Possible Drawbacks
Verification Process
Additional Notes
Release Notes
Review checklist (to be filled by reviewers)
changelog/label attached. If applicable it should have thebackward-incompatiblelabel attached.do-not-merge/label attached.kind/andseverity/labels attached at least.