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

add delete-gauges flag to support re-sending a value when there was no u... #62

Merged
merged 1 commit into from
Apr 23, 2015

Conversation

markrechler
Copy link
Contributor

This is adding a flag for a config option that statsd normally supports:
https://github.com/etsy/statsd/blob/master/docs/metric_types.md#gauges

@markrechler
Copy link
Contributor Author

Did not notice there was an existing pull, #58. This has tests for the case where the flag is set to false as well.

@jehiah
Copy link
Member

jehiah commented Apr 22, 2015

I guess the difference being we don't wan't "unchanged form last value" we want "any update or not" even if it's the same value.

@markrechler
Copy link
Contributor Author

Updated.

continue
} else if ok && lastValue > 0 && c == math.MaxUint64 && *deleteGauges == false {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is functional, but the length of these if statements is getting hard to parse. Any ideas for simplifying? Typically idiomatic Go moves longer if/else if/else if/else into a switch block to simplify.... (my attempt below)

currentValue := c
lastValue, hasLastValue := trackedGauges[g]
var hasChanged bool
if c != math.MaxUint64 {
    hasChanged = true
}
switch {
    case hasChanged:
        fmt.Fprintf(buffer, "%s %d %d\n", g, currentValue, now)
        trackedGauges[g] = c
        gauges[g] = math.MaxUint64
        num++
    case !hasChanged && hasLastValue &&  !*deleteGauges:
        fmt.Fprintf(buffer, "%s %d %d\n", g, lastValue, now)
        num++
    default:
        continue
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also i'm thinking that s/trackedGauges/lastGaugeValue might be a better variable name.

@markrechler
Copy link
Contributor Author

Thanks for the tips, modified per comments.

jehiah added a commit that referenced this pull request Apr 23, 2015
add delete-gauges flag to support re-sending a value when there was no u...
@jehiah jehiah merged commit 0367a6e into bitly:master Apr 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants