[faucet] Improving observability and throughput

[amnn]

A number of changes to improve the quality of metrics we receive from the faucet, and also reduce bottlenecking at the core of the service, to achieve a higher transaction throughput:

Fixes to SimpleFaucet::prepare_gas_coin

This function had a confusing control-flow that hid a few bugs:

• The consumer lock was held for its entire body, when it was only needed at the beginning (to recieve a Coin ID). Fixed by factoring out the part of the function that needs to hold that consumer lock into pop_gas_coin.
• If the consumer lock timed out, the function produced an error that was never used. Fixed by deleting this unused error message.
• The rest of the body is contained within an unconditional loop which always runs exactly once. Fixed by removing the loop and simplifying the control-flow.
• The control-flow path that discards a coin because it has insufficient balance did not update the total_discarded_coins metric. Fixed by moving the logic for updating this metric to transfer_gases which is responsible for handling the return from prepare_gas_coin -- this is also the function that is responsible for recycling coins that should be kept.

Standardise format for Coin IDs in log messages

• Output them as a coin_id label that can be parsed and queried against, rather than inline in the message.

Improvements to RequestMetrics middleware

• Detect and count disconnects, printing their elapsed time in the logs.
• Treat non-successful (2xx) HTTP Responses as failed requests

Test Plan:

Unit tests:

sui$ cargo nextest run

Test the various new features on a local instance of the faucet, by:

 # Starting validators
sui$ cargo build
$ rm -rf ~/.sui                        \
  && ~/sui/target/debug/sui genesis -f \
  && ~/sui/target/debug/sui start

 # Starting faucet (new terminal session)
$ ~/sui/target/debug/sui-faucet \
  --request-buffer-size 2       \
  --max-request-per-second 1

 # Sending a faucet request (excuse fish shell syntax)
$ set -l ADDRESS (yq -r .active_address ~/.sui/sui_config/client.yaml | sed 's/0x//' | string upper)
  curl --location --request POST 'http://127.0.0.1:5003/gas' \
      --header 'Content-Type: application/json'              \
      --data-raw "{ \"FixedAmountRequest\": { \"recipient\": \"$ADDRESS\" } }"

• Test detecting request failure by replacing the body of SimpleFaucet::pop_gas_coin with None
• Test load shedding by running multiple faucet requests in while loops (three is sufficient)
• Test disconnection metrics by adding a sleep command within SimpleFaucet::prepare_gas_coin and interrupting the curl command before it finishes.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments

Name	Status	Preview	Comments	Updated
explorer-storybook	⬜️ Ignored (Inspect)			Jan 3, 2023 at 6:14PM (UTC)
wallet-adapter	⬜️ Ignored (Inspect)			Jan 3, 2023 at 6:14PM (UTC)

[tnowacki]

Faucet code looks good to me. Though I'm not very familiar with it. And I'm even less familiar with the metrics layer stuff.

Approving as it looks like a good improvement on the faucet side, and mostly harmless on the metrics side, though you might want another set of eyes

[amnn]

Thanks for the review @tnowacki! I wrote the metrics layer stuff previously, so it should be okay (at least in the sense that the change isn't entirely without context).