feat(v2:telemetry): more metrics & grafana dashboards

[InKryption]

No description provided.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 201 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
v2/lib/gossip/node.zig	0.00%	88 Missing ⚠️
v2/lib/time.zig	0.00%	28 Missing ⚠️
v2/lib/snapshot/download.zig	0.00%	27 Missing ⚠️
v2/lib/telemetry/metric.zig	0.00%	23 Missing ⚠️
v2/init/main.zig	0.00%	16 Missing ⚠️
v2/lib/telemetry.zig	0.00%	11 Missing ⚠️
v2/services/net.zig	0.00%	6 Missing ⚠️
v2/services/gossip.zig	0.00%	2 Missing ⚠️

❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (85.00%). You can increase the patch coverage or adjust the target coverage.

Files with missing lines	Coverage Δ
v2/lib/gossip.zig	0.00% <ø> (ø)
v2/services/gossip.zig	0.00% <0.00%> (ø)
v2/services/net.zig	0.00% <0.00%> (ø)
v2/lib/telemetry.zig	57.83% <0.00%> (-2.75%)	⬇️
v2/init/main.zig	0.00% <0.00%> (ø)
v2/lib/telemetry/metric.zig	0.00% <0.00%> (ø)
v2/lib/snapshot/download.zig	10.46% <0.00%> (ø)
v2/lib/time.zig	0.00% <0.00%> (ø)
v2/lib/gossip/node.zig	0.00% <0.00%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[dnut]

Is this abstraction needed yet? I don't see any usages of AnyLogger outside this file.

My preference would be for a log message's scope to reflect the location where it is emitted. That matches how loggers commonly behave across languages, and it gives readers a stable, predictable signal about where the log originated.

I agree there can be value in adding tracing context, especially for general-purpose functions called from many places. I just would not overload scope to serve both purposes, since that makes the field ambiguous for people reading the logs. I would rather keep scope as the source location and add a separate contextual field for tracing or caller-specific information. That preserves a conventional and precise meaning for scope while still supporting the additional context this is aiming for.

Given that, I think it would be simpler to remove this abstraction for now and keep scope static. Supporting both static and dynamic scope adds another layer of complexity that callers and readers need to reason about. We can add dynamic context fields once there is a clear need for them.

[InKryption]

I originally did have a change that used this, but it was swept away after a rebase; the main reason I introduced it was because there was a general-purpose function in gossip which was called from many contexts, and in fact its behaviour depended explicitly on who the caller was (it had an enum that directly said who the caller was).

But yeah, given it isn't needed, I have no issue removing it, long live YAGNI.

[InKryption]

Dropped the commit

[prestonsn]

Not related to the changes in this PR directly, but while testing the new grafana setup locally I found that prometheus was rejecting/not decoding the histogram metrics (Explore tab was not showing any data for histogram metrics)

I think this has to do with a formatting issue here:

sig/v2/lib/telemetry/prometheus.zig

Line 85 in 004ff44

try w.print("le={f}", .{numberFmt(bucket.upper_bound)});

Changing that line to include the outer double-quotes in the payload seems to fix this:

try w.print("le=\"{f}\"", .{numberFmt(bucket.upper_bound)});

versions i'm using in case it has something to do with it:

Grafana: 13.0.1 
Prometheus: 3.12.0

Update: it seems like my prometheus setup can't scrape any metrics unless I make the above code change

Another update: I did sidestep the docker setup and just ran prometheus and grafana services directly for testing things on my end. Maybe this isn't an issue in proper setups, not something that is blocking for me 🫡

Another update after the other update: it seems like prometheus scrape fails without the double quote change. From the spec it seems like le=x is actually treated like a label, so it has to be in double quotes (le="x") in the payload

[prestonsn]

LGTM overall, just a few minor things I noticed

[prestonsn]

I like the Instant and Timer abstractions a lot. I've got a couple questions tho:

1. Is there a reason why we don't use .MONOTONIC here? last i checked, the std lib uses it too (but perhaps that's part of the reason why its getting overhauled?)
2. Could we integrate/use lib.clock here in some way? It has the warmup path already setup. Wondering if this separation was intended

[InKryption]

On point 1, not sure iiuc. Are you saying stdlib uses MONOTONIC? Because that may be true for something else, but the Instant abstraction uses BOOTTIME on linux. If you're suggesting replacing BOOTTIME to stray from what stdlib was using, then sure I suppose, I think it probably doesn't matter a huge amount in our case.
On point 2, yeah, I implemented this before lib.clock got implemented. I'll give this another pass and probably move this into there.

[prestonsn]

on 1, I see your point actually. I was mostly curious as why we chose BOOTTIME here. I checked the zig std lib and was totally wrong on stating it uses MONOTONIC, I thought it did for some reason. I'm cool with sticking with BOOTTIME for this