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

Add a metric recording the overall time spent building each crate #2186

Merged
merged 1 commit into from
Aug 9, 2023

Conversation

Nemo157
Copy link
Member

@Nemo157 Nemo157 commented Aug 7, 2023

In some ways this is simply an inverse of the existing "builds/hour" derived metric, but I think having a histogram will help visualize what proportion of fast vs slow crates we have.

@github-actions github-actions bot added the S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed label Aug 7, 2023
Copy link
Member

@GuillaumeGomez GuillaumeGomez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! (Please wait for someone else than me to approve before merging as I'm not much knowledgeable about this area in the code)

Copy link
Member

@syphar syphar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT in general.

I'm not 100% certain how useful this metric will be around build failures, this probably depends on the kind of failure / when it fails.

But in my mind #1011 would add more visibility, presumably even recording per-target data / logs / metrics.

@Nemo157
Copy link
Member Author

Nemo157 commented Aug 8, 2023

Yeah, per-target logs were the main reason I was working on #1011. I'm not sure if there's very useful metrics we can extract to prometheus after it, but I do think we can record per-target times and surface those on the builds page, and I have some thoughts on trying to show warnings to crate owners where we could potentially inform them if they're approaching the timeout.

@Nemo157
Copy link
Member Author

Nemo157 commented Aug 8, 2023

One thought on failures, I could partition this based on whether the build succeeded, do you think that would be useful?

@syphar
Copy link
Member

syphar commented Aug 9, 2023

One thought on failures, I could partition this based on whether the build succeeded, do you think that would be useful?

It seems like the intent of this metric is more or less "build time capacity" or something like that, or how much time builds by crate consume. And for the time consumption in general it doesn't matter if it's a build failure or not.

The real interesting information will come when we start recording per-target build information in addition to the logs, like the duration, if the build did timeout or OOM, ...

@Nemo157 Nemo157 merged commit a4e1030 into rust-lang:master Aug 9, 2023
7 checks passed
@Nemo157 Nemo157 deleted the build-timing branch August 9, 2023 09:38
@github-actions github-actions bot added S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it and removed S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed labels Aug 9, 2023
@syphar syphar removed the S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it label Aug 22, 2023
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