fix(timeline,cobuilds): cobuilt operations should reflect the cobuild time and not cache restore time #4680
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
OperationExecutionRecord
is currently only tracking cache restore time for cobuilt operations that are marked asRemoteOperating
and are then restored from cache. This is a confusing UX and causes developers + maintainers to have to search across multiple machine logs to determine operation run times. This PR adjusts that to use thenonCachedDurationMs
from the state file. This felt like something that #3649 was intending to do or is in a similar vein.Before:
After:
Details
This overwrites the existing stopwatch for operations that were not cobuilt on the specific agent. It adds a new
beforeResult
method toOperationExecutionRecord#executeAsync
to handle theafterExecuteOperation
hook instead of passing that work into theonResult
method which ended up receiving running stopwatches and had other assumptions of state (collated terminal not closed). This does add possibly breaking behavior where cobuilds were showing cache times which might be useful, but I don't think those are as useful as having the cobuild time available across all agents. That also has the secondary effect of making the timeline views of all agents much more cohesive - though as we see above they're not the exact same across agents.How it was tested
I tested this locally using the
build-tests/rush-redis-cobuild-plugin-integration-test
sandbox repo with 2 runners, confirmed that timings generally matched across the instances. There's about 10ms (rounded up) of difference between the agents, but this already seems much more useful.Impacted documentation
None