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

Unclear meaning of DateTime for Gauges #103

Closed
dv00d00 opened this issue Sep 5, 2018 · 3 comments
Closed

Unclear meaning of DateTime for Gauges #103

dv00d00 opened this issue Sep 5, 2018 · 3 comments

Comments

@dv00d00
Copy link
Contributor

dv00d00 commented Sep 5, 2018

There are two methods in StatsDMessageFormatter class which are using DateTime parameter

public string Gauge(double magnitude, string statBucket, DateTime timestamp)
{
    var stat = string.Format(InvariantCulture, "{0}{1}:{2}|g|@{3}", _prefix, statBucket, magnitude, timestamp.AsUnixTime());
    return Format(DefaultSampleRate, stat);
}

public string Gauge(long magnitude, string statBucket, DateTime timestamp)
{
    var stat = string.Format(InvariantCulture, "{0}{1}:{2}|g|@{3}", _prefix, statBucket, magnitude, timestamp.AsUnixTime());
    return Format(DefaultSampleRate, stat);
}

I was not able to find any reference to Unix timestamp in statsd docs

https://github.com/b/statsd_spec#gauges
https://statsd.readthedocs.io/en/v3.3/types.html#gauges
https://github.com/etsy/statsd/blob/master/docs/metric_types.md#gauges

Also this paragraph

The StatsClient.gauge() method also support the rate parameter to sample data back to the statsd server, but use it with care, especially with gauges that may not be updated very often.

Am I missing something?

@slang25
Copy link
Member

slang25 commented Sep 5, 2018

Agreed, it looks weird. Can't see any real justification in the (very old) commit that added it: 63c1a45

@AnthonySteele
Copy link
Contributor

This is almost certainly unused, and not matched by similar methods for increment / timer etc.
Suggest that we remove it for the next major version?

@martincostello
Copy link
Member

I'd concur that we could drop it as part of 4.0.0.

AnthonySteele referenced this issue in AnthonySteele/JustEat.StatsD Oct 3, 2018
fix for #103
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

4 participants