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

Include spawn metrics in the execution log. #15413

Closed
wants to merge 1 commit into from

Conversation

tjgq
Copy link
Contributor

@tjgq tjgq commented May 6, 2022

The SpawnMetrics proto is intended to match the existing Java class of the
same name. Note that the SpawnExec.walltime field becomes redundant, but it
is kept to ensure backwards compatibility.

This change makes it easier to correlate the metrics for corresponding actions
in two different builds.

@tjgq tjgq force-pushed the execlog-spawnmetrics branch 3 times, most recently from 9f3c412 to 876bb49 Compare May 7, 2022 07:30
@sgowroji sgowroji added team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer labels May 7, 2022
@coeuvre
Copy link
Member

coeuvre commented May 9, 2022

One thing we usually do is diffing execution logs from two builds to troubleshoot cache hits. Adding spawn metrics could probably add noise to the diff. But I do think we need include spawn metrics somewhere.

@meisterT WDYT?

@tjgq tjgq force-pushed the execlog-spawnmetrics branch from 876bb49 to 26b6406 Compare May 9, 2022 11:31
The SpawnMetrics proto is intended to match the existing Java class of the
same name. Note that the SpawnExec.walltime field becomes redundant, but it
is kept to ensure backwards compatibility.

This change makes it easier to correlate the metrics for corresponding actions
in two different builds.
@tjgq tjgq force-pushed the execlog-spawnmetrics branch from 26b6406 to e114471 Compare May 9, 2022 12:04
@tjgq
Copy link
Contributor Author

tjgq commented May 9, 2022

Put the inclusion of spawn metrics in the log behind a flag, per offline discussion.

@bazel-io bazel-io closed this in b4b8b26 May 11, 2022
@brentleyjones
Copy link
Contributor

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label May 11, 2022
@ckolli5
Copy link

ckolli5 commented May 11, 2022

@bazel-io fork 5.2.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label May 11, 2022
@ShreeM01 ShreeM01 removed the awaiting-review PR is awaiting review from an assigned reviewer label Sep 15, 2022
@tjgq tjgq deleted the execlog-spawnmetrics branch December 8, 2022 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants