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

Counter should not have decrement method #176

Closed
unkusr007 opened this issue Apr 17, 2019 · 7 comments
Closed

Counter should not have decrement method #176

unkusr007 opened this issue Apr 17, 2019 · 7 comments
Labels
Milestone

Comments

@unkusr007
Copy link

Hello,

a counter should always goes up according to the documentation, there's no decrement.

https://github.com/statsd/statsd/blob/master/docs/metric_types.md#counting

Thanks

@unkusr007 unkusr007 changed the title Counter should not have decrement Counter should not have decrement method Apr 17, 2019
@AnthonySteele
Copy link
Contributor

I'm sure that I have seen decrements, e.g. https://python-statsd.readthedocs.io/en/latest/statsd.counter.html

It's really an alias for increment by a negative number.

@unkusr007
Copy link
Author

This is an implementation of the statd specification, but I can't see anywhere that a counter can be decrement: otherwise it's a gauge.

@AnthonySteele
Copy link
Contributor

AnthonySteele commented Apr 18, 2019

You might be right. "increment" in Statsd is usually used to say that "this thing happened".
e.g. "a request to this endpoint was handled". And so "increment by three" means that "this thing happened three times".

Decrement(3), i.e. "increment by -3" means ... what? This thing happened less than zero times? It might not have a meaning at all.

If we find an actual use of it in our code then we should keep it. Otherwise, it can go.
Decrement is only an extension method that flips the sign of the number, but its presence implies that this is a thing that you can and should do..

@martincostello
Copy link
Member

This doesn't particularly concern me.

Even if we didn't have a decrement method, you'd still be able to do that because the input type is still a signed number, so you could still do this via the increment method anyway.

The integration tests happily work against statsd itself and increment and decrement the counter bucket, so that implies it works, even if it isn't documented. Maybe it's supported as a way to "correct" a mistake in a count before the metric bucket ships somewhere?

If we did want to deprecate this, in the immediate term we'd just add an [Obsolete] attribute to the relevant method(s) so that the next release warns consumers and they can move away from it. Then at some future date when there's a genuine need to ship a breaking change as a v5, we could remove it entirely then.

@martincostello
Copy link
Member

@stale
Copy link

stale bot commented Jun 13, 2019

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jun 13, 2019
@stale
Copy link

stale bot commented Jun 20, 2019

Closed automatically due to inactivity.

@stale stale bot closed this as completed Jun 20, 2019
@martincostello martincostello linked a pull request Mar 14, 2020 that will close this issue
@martincostello martincostello added this to the v5.0.0 milestone Mar 14, 2020
@martincostello martincostello modified the milestone: v5.0.0 Aug 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants