grafana / grafana/walqueue

Ensure values from stats used for calculations stay in sync with those that are used for metrics

Open
#50 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
9
Forks
0
PR merge metrics
No merged PRs in 30d

Description

**Context**

One the biggest challenges we had with finding the root cause to https://github.com/grafana/walqueue/pull/49 was that according to all the metrics everything was fine. It was only after we enabled debug logging were we able to see that `parallelism.go`'s `timestampNetworkSeconds` https://github.com/grafana/walqueue/blob/0639a4d8854fd222fc5609f065ea6594e4df2bf5/network/parallelism.go#L35 was 2x what was expected.

The values are allowed to drift because there are two different components reading the same stats updates, one acts on the value to adjust parallelism and the other exposes the value as a metric. If the logic between these two components diverges the values can get out of sync and our visibility is completely gone.

**What needs done**

- [ ] A quick win would be exposing the all [ParralelismStats](https://github.com/grafana/walqueue/blob/f5820e845c289a296582aa931b3a8dfe10262541/types/stats.go#L22-L30) (it's okay to fix the spelling too) as metrics
- We currently only expose `MinConnections`, `MaxConnections`, and `DesiredConnection` which are unique to parallelism
- Exposing `TimestampDriftSeconds`, `TimestampNetworkSeconds`, `TimestampSerializerSeconds` would give us parallelism's view of important metrics that exist elsewhere which is incredibly useful for debugging
- `NetworkErrorRate` is also unique to parallemism, exists in the stats but is not currently set, and is another blind spot that would be really useful for debugging
- [ ] Attempt to unify the stats updating process so that different components cannot get out of sync in this manner. Ideally, a metrics component subscribes to the stats updates to update metrics and other components can be given accessors to the current state of the metric value (perhaps through a combination of `prometheus.Func` being tied to an `atomic.`).

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.