rustls / rustls-bench-app Goto Github PK

When visiting https://bench.rustls.dev/info, this is the returned JSON:

{
  "git_commit_message": "VERGEN_IDEMPOTENT_OUTPUT",
  "git_commit_sha": "VERGEN_IDEMPOTENT_OUTPUT"
}

I have traced the problem to the fact that the vergen library is unable to use the git executable during the build step. @cpu any clue on how to solve this, based on your ansible experience? I tried explicitly setting the PATH variable as follows, but it didn't change anything:

- name: Build app
  become: true
  become_user: "{{ app_user }}"
  ansible.builtin.command: /home/{{ app_user }}/.cargo/bin/cargo build --release
  changed_when: true
  args:
    chdir: /home/{{ app_user }}/rustls-bench-app/ci-bench-runner
    # Note: omitting a 'creates' here - we want to build fresh even if the binary exists
    #       in order to pick up changes from the src git repo updating.
  environment:
    PATH: /usr/bin

Coming from rustls/rustls#1763 (comment), I think this is an interesting way to compare the performance of different providers.

eg, on rustls/rustls#1684 it goes to https://example.com/jobs/c6a595e9-6d60-42d2-9b88-ec237220a618

I notice CI is occasionally failing the unit tests with:

---- test::test_pr_opened_happy_path_with_failed_comment_reuse stdout ----
thread 'test::test_pr_opened_happy_path_with_failed_comment_reuse' panicked at src/test/mod.rs:331:10:
called `Result::unwrap()` on an `Err` value: Elapsed(())
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I've noticed two instances today. Probably the test is flaky?

• https://github.com/aochagavia/rustls-bench-app/actions/runs/6787181320/job/18449415008
• https://github.com/aochagavia/rustls-bench-app/actions/runs/6785631007/job/18444379135

There are a few scenarios where it makes sense to postpone running benchmarks, to avoid competing for resources and prevent noise in the results:

1. When manually running other performance benchmarks (like in this case).
2. When building and deploying a new version of the application (since the app is built on the server).

Currently, the only way to avoid running benchmarks in reaction to GitHub events is to shut down the application entirely. If we do so, however, GitHub events get lost, which is unfortunate. The current way to recover them is that someone goes to the rustls-benchmarking GitHub app's page and manually redelivers any relevant webhooks.

I'm tempted to leave this as wontfix, because I don't expect it will cause problems in practice. That way we avoid the extra complexity.

We need a way to check which version of the application is running at a given moment. Maybe we should use the vergen crate to embed the git hash at compile-time, and return it along with other relevant information upon GET /info.

An idea from djc in discord:

In fact it would be nice if we can relax it even more, like allow the bot to run if we previously merged a PR from the same author

This will probably be a little bit trickier than #32 - I'm not sure if GitHub exposes this state through the API. Some research will be required.

An idea from djc in discord:

would be nice if the benchmarking bot knew it doesn't need to wait for approval on PRs where the author is an org maintainer

I think this would be a fairly small lift: we already have the code in place to check when a PR is approved that the approver has an allowed account association. Likely we just need to adjust the event processing for a new PR to skip requiring an approval or a comment if the author has such an association.