feat: add metrics for gcs payload cleanup on failed db transactions

[taddes]

Description

STOR-623 added a best-effort GCS delete for payloads that were offloaded before a
write's database transaction failed, but it was fire and forget, so there's no way
to tell from prod whether those deletes actually land. This adds a counter around
them.

The metric is storage.gcs.payload.cleanup, tagged with:

• handler: put_bso or post_collection
• result: success or failure
• reason on failures: gcs_error if GCS rejected the delete, invalid_url if the
  gs:// URL didn't parse (a code bug rather than an infra problem)

Both are emitted inside delete_payload, so any future call site gets them for free.

Tests use cadence's SpyMetricSink over the existing StorageControl stubs, one per
branch. Commits are split so each addition is reviewable on its own.

Note on local verification: cargo fmt --check is clean, clippy is clean for
syncserver, and the web::payload_offload unit tests pass. The full
make clippy_mysql can't run on arm64 here because grpcio-sys fails to build, and
it also trips two pre-existing needless_as_bytes errors in tokenserver-auth that
are already on master, so CI is the real check for those.

Review round

Rebased onto master, so the merge commit is gone and the history is linear.

• Dropped the skipped result entirely. Both GCS clients are built together at
  startup when a payload bucket is configured, so there is no state where the control
  client is missing but the upload succeeded. The counter could only ever read zero.
  Back to if let Ok(client) with a comment recording why it holds.
• Swapped the hand-rolled recording sink for cadence's SpyMetricSink, which needed
  crossbeam-channel as a dev-dependency to name the receiver. It was already in the
  lock via cadence.
• Dropped the .config/nextest.toml retry commit as out of scope. Worth noting for
  whenever it gets picked up separately: nextest profiles inherit from
  [profile.default], so test-threads = 1 does apply to profile.ci, which means
  parallel tests contending on shared rows can't be the cause of that abort.

I did not take the gcs_control_client()? variant. At the cleanup site the ? would
return the GCS client error in place of the database error that actually caused the
rollback, so the real cause would be lost from both the response and Sentry.

Testing

No behaviour change beyond the metric. cargo test -p syncserver --lib web::payload_offload
covers success, GCS failure, and unparseable URL.

Issue(s)

Closes STOR-653.

[chenba]

You don't need to break up the chain for the side effect. You can do

      .inspect(|_| metrics.incr_with_tags(CLEANUP_METRIC, cleanup_tags(handler, "success")))
      .inspect_err(|e| {
          warn!("gcs payload cleanup failed for {gs_url}: {e}");
          metrics.incr_with_tags(CLEANUP_METRIC, cleanup_failure_tags(handler, "gcs_error"));
      })

[taddes]

ok cool, good suggestion

[chenba]

Since you are returning the Result as is (parsed -> parsed, Err(e) -> Err(e)) you can use inspect_err for the side effect.

[chenba]

Maybe make an enum instead of allowing any string.

[chenba]

This is a bit out of scope. But also, line 6 above is test-threads = 1 so I'm not sure the lock contention is even possible?

[taddes]

It was a bug I was running into locally and that was the suggested fix 🤷 I can look a bit more closely at that part

[pjenvey]

I'd argue we don't need these Err blocks and their metrics at all because we should never have anything offloaded in the first place if there's no gcs_control_client (there should never be a lack of gcs_control_client if there is a gcs_client).

Or at least we could do this more simpler by just propagating the error (gcs_control_client()?) -- similarly to the above state.gcs_client()? -- if we're set to offload and there's no gcs clients, just fail with an error.

[pjenvey]

FYI cadence ships a similar sink for testing -- pretty similar but it gives you a https://docs.rs/crossbeam-channel/0.5.15/crossbeam_channel/struct.Receiver.html you can iterate over (here's an e.g. of it used in contile)