feat: declare seconds unit on duration histograms#177
Merged
Conversation
Adds metric.WithUnit("s") to the four Float64Histograms in MpcMetrics
(SessionTime, InitiateTime, CommSendTime, CommDnsResolveTime). Without
the unit, the sub-second bucket view registered by sygma-core's
observability.InitMetricProvider does not match these instruments and
the SDK falls back to default boundaries tuned for milliseconds, so
sub-second values collapse into the le=5 bucket and quantile queries
return useless results.
Co-Authored-By: Claude
|
Go Test coverage is 37.0 %\ ✨ ✨ ✨ |
5 tasks
|
Go Test coverage is 37.0 %\ ✨ ✨ ✨ |
mpetrun5
approved these changes
Apr 28, 2026
MakMuftic
approved these changes
Apr 28, 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
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.
Summary
Adds
metric.WithUnit("s")to the fourFloat64Histograms inMpcMetrics:relayer.SessionTime(PR feat: session time metrics #143)relayer.InitiateTime(PR feat: instrument mpc signing phase durations #171)relayer.CommSendTime(PR feat: instrument mpc signing phase durations #171)relayer.CommDnsResolveTime(PR feat: instrument mpc signing phase durations #171)Why
sygma-coreregisters a sub-second bucket view inobservability.InitMetricProvider:The view is keyed by
Instrument{Unit: "s"}. The histograms inmetrics/mpc.godeclared a description but no unit, so the view never matched and the SDK fell back to OTel's default histogram boundaries[5, 10, 25, 50, ..., 10000]— which are tuned for milliseconds. Signing-phase durations (sub-second to a few seconds) collapse into thele=5bucket, makinghistogram_quantilereturn values pinned to bucket boundaries rather than real percentiles.Values are already recorded in seconds via
d.Seconds(), so no math changes — only bucketing.Grafana query change
The OTLP→Prometheus exporter appends a
_secondssuffix when the instrument carriesUnit: "s". After this PR, dashboard queries change:relayer_SessionTime_bucketrelayer_SessionTime_seconds_bucketrelayer_InitiateTime_bucketrelayer_InitiateTime_seconds_bucketrelayer_CommSendTime_bucketrelayer_CommSendTime_seconds_bucketrelayer_CommDnsResolveTime_bucketrelayer_CommDnsResolveTime_seconds_bucketIn practice no dashboards depend on the old names yet — the OTel collector URL was only wired into staging in #174, so historical data is empty.
Test plan
go build ./...cleango test ./metrics/... ./tss/... ./comm/p2p/...passrelayer_SessionTime_seconds_countand the three new_seconds_countseries are non-empty in Grafanahistogram_quantile(0.95, ...)returns values that vary with workload rather than snapping to bucket boundaries