Repository navigation
Fix skipped-target and cache-hit span timing - #7
Merged
Merged
Conversation
The group span opened at the invocation start and closed at the build end, while every skipped target inside it is zero-length, so a build with skipped targets reported them as taking the whole build. Skipped targets take no time, but Bazel reports them both during analysis and after execution, so a first-to-last window would still span the build. Put every skipped target and the group at the first skip's timestamp, kept inside the invocation. Remove the uncalled on_aborted_label, which built the same group the old way.
Bazel 9 reports targets that fail analysis through aborted TargetConfigured events, often without a matching ConfiguredLabel event, so some skipped targets never got a span. Route those events to on_target_skipped and ignore a second report for a target that is already skipped. Refs #6
Bazel reports a cache hit with the start and end time of the run that filled the cache. Conduit kept that duration and shifted it to the build start, so cache hits showed minutes of work they never did and won the lightweight span budget over real slow actions. A cache hit (cached == true, or, before Bazel 10, an action starting before the build) is now a zero-length span at the time Bazel reported it, and ranks as zero duration in the budget. An explicit cache miss keeps its timing. Refs #6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #6.
Skipped targets and cache-hit actions reported durations they never took (trace
4a1d125a3ec94e75baa10a7a1daa2628:skipped targets173 s, GoCompilePkg cache hits 88–161 s).TargetConfiguredget a span. Bazel 9 reports some analysis failures without aConfiguredLabelevent; a second report for the same target is ignored.cached == true, or before Bazel 10 an action starting before the build) is now a zero-length span at the time Bazel reported it and ranks as zero duration in the lightweight span budget. An explicit cache miss keeps its timing.Testing
bazel test -c opt //...: pass (121 unit tests, new ones for each fix).conduit serveinto Datadog. Bazel reported the cache hits starting ~12.4 s before the build with build 1's 6.42 s / 4.46 s durations; the trace shows them as 0 ms at their report time inside their targets. The executed action keeps its 3.19 s, and both skipped targets sit in a 0 ms group.Risk
cached_spawn_chain_is_nested_inside_root_windowno longer expects cache-hit actions to have nonzero width; that expectation encoded the duration-preserving behavior removed here.