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

feat(ci): upload metrics to GitHub Artifacts ✨ #116517

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

meysam81
Copy link
Contributor

@meysam81 meysam81 commented Oct 7, 2023

@rustbot
Copy link
Collaborator

rustbot commented Oct 7, 2023

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Oct 7, 2023
@meysam81 meysam81 changed the title meysam/feat/upload ci metrics to artifacts feat(ci): upload metrics to GitHub Artifacts ✨ Oct 7, 2023
@meysam81
Copy link
Contributor Author

meysam81 commented Oct 7, 2023

r? @jdno

@rustbot rustbot assigned jdno and unassigned Mark-Simulacrum Oct 7, 2023
Copy link
Member

@jdno jdno left a comment

Choose a reason for hiding this comment

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

We also generate detailed metrics in x.py, which we should upload. Here's the relevant step in upload-artifacts.sh:

cp "${build_dir}/metrics.json" "${upload_dir}/metrics-${CI_JOB_NAME}.json"

I didn't notice before that build_dir is a variable that has two different values, based on the operating system:

build_dir=build
if isLinux; then
    build_dir=obj/build
fi

So simply invoking actions/upload-artifact might not be enough. 😬

I am wondering if it would make sense to create a script to gather the build metrics that we can call in both upload-artifacts.sh and in a step before actions/upload-artifact. In other words, move these instructions into a script and call it in both places:

# CPU usage statistics.
cp build/cpu-usage.csv "${upload_dir}/cpu-${CI_JOB_NAME}.csv"

# Build metrics generated by x.py.
cp "${build_dir}/metrics.json" "${upload_dir}/metrics-${CI_JOB_NAME}.json"

What do you think?

@meysam81 meysam81 force-pushed the meysam/feat/upload-ci-metrics-to-artifacts branch from 9170f03 to 58784f8 Compare October 13, 2023 11:56
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@Dylan-DPC
Copy link
Member

@jdno any updates on the review? thanks

@bors
Copy link
Contributor

bors commented Apr 24, 2024

☔ The latest upstream changes (presumably #124175) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upload metrics from PR CI as artifacts in GitHub Actions
7 participants