rpc/system_metrics.rs: Added rustversion label to garage_build_info metric. #524

jpds · 2023-03-10T11:44:42Z

jpds commented

2023-03-10 11:44:42 +00:00

No description provided.

jpds force-pushed rustversion-label from 25e90d2bf9 to 531cd017ab

2023-03-10 11:48:40 +00:00

Compare

quentin commented

2023-03-10 12:23:09 +00:00

Hi @jpds, thanks for your PR, can you elaborate a bit more around the use cases you see for it?

jpds commented

2023-03-10 12:24:14 +00:00

@quentin Knowing when to rebuild all the Rust binaries because of a vulnerability at the compiler level.

👍 1

quentin commented

2023-03-10 12:36:43 +00:00

Makes sense. Before accepting it, I want to be sure that it does not break our NixOS-based build system, as you introduce a build.rs file. Otherwise, looks good to me.

jpds commented

2023-03-10 13:53:09 +00:00

Someone will need to push up a Cargo.nix change for the new package (which is why I allowed edits from maintainers) - but I think it should work fine after that.

Someone will need to push up a `Cargo.nix` change for the new package (which is why I allowed edits from maintainers) - but I think it should work fine after that.

lx requested changes 2023-03-10 14:05:08 +00:00

src/rpc/build.rs Outdated

					
				@ -0,0 +1,8 @@

				use rustc_version::version;

				fn main() {

lx commented

2023-03-10 14:05:05 +00:00

Can we move this to the garage_util crate, and make the rustc version available in the version module (as garage_util::version::rustc_version(), similarly to garage_util::version::garage_version())

This way we can also show the rustc version in garage stats (here) and in the status admin endpoint (here)

Can we move this to the `garage_util` crate, and make the rustc version available in the `version` module (as `garage_util::version::rustc_version()`, similarly to `garage_util::version::garage_version()`) This way we can also show the rustc version in `garage stats` ([here](https://git.deuxfleurs.fr/Deuxfleurs/garage/src/branch/main/src/garage/admin.rs#L821)) and in the status admin endpoint ([here](https://git.deuxfleurs.fr/Deuxfleurs/garage/src/branch/main/src/api/admin/cluster.rs#L21))

lx commented

2023-03-10 14:07:13 +00:00

I can regen the cargo.nix file once the dependency on rustc_version is moved to the correct crate

jpds marked this conversation as resolved

jpds force-pushed rustversion-label from 531cd017ab to 9ea154ae9c

2023-03-10 14:47:23 +00:00

Compare

lx added 1 commit 2023-03-10 17:15:17 +00:00

continuous-integration/drone/pr Build is passing Details

40fa1242f0 update cargo.nix

quentin commented

2023-03-13 09:52:47 +00:00

I restarted the CI for the last commit.

It failed on thread 'k2v::poll::test_poll_range' panicked at 'Command exited with code 1: Could not get node ID', tests/common/ext/process.rs:46:31. Running the tests (with nix-build) on my computer worked. I suspect a race condition (we statically wait for only 2sec at garage startup).

I think I will open a separate PR to fix this issue, and the other one with the parallel tests. In the mean time, we should restart the CI and/or increase the duration in the fn setup() sleep, inside the src/garage/tests/common/garage.rs file.

I restarted the CI for the last commit. It failed on `thread 'k2v::poll::test_poll_range' panicked at 'Command exited with code 1: Could not get node ID', tests/common/ext/process.rs:46:31`. Running the tests (with nix-build) on my computer worked. I suspect a race condition (we statically wait for only 2sec at garage startup). I think I will open a separate PR to fix this issue, and the other one with the parallel tests. In the mean time, we should restart the CI and/or increase the duration in the `fn setup()` sleep, inside the `src/garage/tests/common/garage.rs` file.

lx added 1 commit 2023-03-13 15:14:19 +00:00