Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explicitly set counter values #89

Open
rauhul opened this issue Dec 14, 2020 · 1 comment
Open

Explicitly set counter values #89

rauhul opened this issue Dec 14, 2020 · 1 comment

Comments

@rauhul
Copy link
Contributor

rauhul commented Dec 14, 2020

swift-metrics doesn't currently provide a Gauge.record api for Counters and this probably makes as Counters are supposed to be strictly incrementing. However this behavior doesn't compose if the metric source already accumulates the value.

let total = GetTotalNetworkPackets()
Counter(label: "network_packets_total")
  .increment(total) // <-- increment is wrong here (double counting)

Instead you have to use a Gauge to directly set this value. However consumers now see the value as a gauge and not a counter which is the real behavior.

I'm not sure if this is actually something we want to support, but it probably warrants some thought.


Prometheus in go provides an escape hatch for this using NewConstMetric

@ktoso
Copy link
Member

ktoso commented Dec 14, 2020

Thanks for reporting, I have somewhat mixed feelings about it.

On one hand, a Counter is really defined as being "grow only", so allowing a record API means we would probably need to enforce that grown-only nature...

  • Should we crash if someone reports 10 and then 1 though? Seems a bit too "intense", for a metrics library.
  • Should we just pass through to the underlying library but warn implementations about this?

The prom API you mention also does not allow for directly recording on a Counter but offers this escape hatch... Unless we really have to, I think we should avoid doing this in this API package; Actual metrics libs can offer those hatches if necessary.

Let me know though if this would be a huge blocker for you -- so far I understand you can get away with the Gauge approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants