[telemetry] Add structured logging with span-based tracing#453
Open
0xsiddharthks wants to merge 6 commits intomainfrom
Open
[telemetry] Add structured logging with span-based tracing#4530xsiddharthks wants to merge 6 commits intomainfrom
0xsiddharthks wants to merge 6 commits intomainfrom
Conversation
0xsiddharthks
force-pushed
the
siddharth/telemetry-1
branch
2 times, most recently
from
2820309 to
92086db
Compare
bmwill
reviewed
Apr 15, 2026
bmwill
reviewed
Apr 15, 2026
Comment on lines
+100
to
+107
| TelemetryGuard { _private: () } | ||
| } | ||
| } | ||
|
|
||
| #[must_use = "dropping the guard immediately will lose buffered log output"] | ||
| pub struct TelemetryGuard { | ||
| _private: (), | ||
| } |
Contributor
There was a problem hiding this comment.
Why do we have this guard? I don't think that its actually doing what this says it is doing.
Replaces the per-binary init_tracing_subscriber() boilerplate with a shared hashi-telemetry crate that auto-detects the log format via std::io::IsTerminal and honors RUST_LOG / RUST_LOG_JSON: - stderr is a terminal (local dev) → human format - stderr is a pipe (K8s pod) → JSON (for Loki) - RUST_LOG_JSON=1 / =0 → hard override, wins over auto-detect CI workflows that want human-readable output for pipe'd GitHub Actions runner stderr set RUST_LOG_JSON=0 at the workflow env level. .github/workflows/ci.yml does this for the hashi CI job. Instruments the main unit-of-work boundaries across the daemon: the gRPC handlers in bridge_service, deposit/withdrawal validation, every leader task (deposit processing, withdrawal approval/commitment/signing/confirmation fan-outs), the Bitcoin and Sui indexers, MPC run loops and handle_reconfig, every execute_* method in sui_tx_executor (with sui_digest recorded on the generic execute), and mpc::signing::sign so presig-reassignment logs carry their parent span. Service identification (hashi vs hashi-screener vs hashi-guardian vs hashi-monitor) is done via Kubernetes pod labels already injected by Promtail at ingest, not via a log-body field.
The big module-level bullet list, per-field doc comments, per-method doc comments, and the inline NOTE block in init() were mostly describing what the code already says. Keep one short module-level summary and one line on with_env's RUST_LOG_JSON parsing (the only non-obvious bit — silent fall-through on unknown values).
Per code-review feedback: hashi-telemetry was small enough that the extra crate wasn't pulling its weight. Move the whole thing to hashi_types::telemetry and delete the standalone crate. - crates/hashi-telemetry/ → crates/hashi-types/src/telemetry.rs - hashi-types now depends on tracing-subscriber (already transitively present in every workspace consumer except internal-tools, which already has it too) - All 5 binaries drop `hashi-telemetry = ...` and swap `hashi_telemetry::TelemetryConfig` for `hashi_types::telemetry::TelemetryConfig` - Docker Containerfiles for hashi and hashi-screener drop the extra COPY/stub-source steps for the removed crate
0xsiddharthks
force-pushed
the
siddharth/telemetry-1
branch
from
16e1805 to
e4a9c97
Compare
… main The guard was a ZST marker with no Drop impl and no buffered I/O to flush, so it could not do what its #[must_use] message claimed. The hashi-localnet init sat inside cmd_start, which meant FaucetSui and Deposit silently dropped their spans; lift verbose to a global clap flag and initialize tracing at the top of main, matching every other binary.
bmwill
reviewed
Apr 15, 2026
`tracing_subscriber::fmt::layer()` defaults its writer to stdout, so log output was leaking onto stdout even though every other signal in this module (TTY autodetect, ANSI colorization) is keyed off stderr. Explicitly set the writer on both the JSON and TTY branches.
bmwill
approved these changes
Apr 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a new
telemetrymodule in hashi-types .additional changes:
std::io::IsTerminalto detect stderr (terminal -> ttl , pipe -> JSON)RUST_LOG_JSONtargetfield on every event. (tracing subscriber emits the module path in the field)Also add
#[tracing::instrument]at the main unit-of-work boundaries:bridge_servicehandle_reconfig, andmpc::signing::signexecute_*insui_tx_executor— the genericexecuterecordssui_digeston success so one span tree covers handler → validator → Sui submissiondeployment changes:
MystenLabs/sui-operations#7626