WIP: Feature/value on counter

[Skeen]

This PR introduces adds an optional configuration field to the counter metric type, namely the value template field, also seen on the gauge.

It enables counting up with more than one, a motivating example could be for instance counting time spent or bytes ingested, both of which are monotonically increasing, and thus not will fit as gauges.

The PR has several commits;

• 050c2e5 renames two callbacks from cb to callback for consistency.
• 877a386 prepares for support for the counter, by allowing the callbacks to reject samples, specifically to reject floats which are negative for the counter. The change is made throughout to support future needs for late sample rejection.
• cdba743 changes the counter into an observeMetric and changes configuration to allow for the value key.
• 25314da updates the documentation, such that the optional field of value is reflected for the counter metric.

[Skeen]

I have tested the change using:

global:
  config_version: 3
input:
  type: webhook
  webhook_path: /webhook
  webhook_format: json_single
  webhook_json_selector: .message
  webhook_text_bulk_separator: "\n\n"
imports:
- type: grok_patterns
  dir: ./logstash-patterns-core/patterns
metrics:
- type: counter
  name: bytes_processed
  help: Total number of bytes processed
  match: '%{NUMBER:bytes} bytes processed'
  value: '{{ .bytes }}'
server:
  host: "[::]"
  port: 9144

Started as:

go run grok_exporter.go -config example/config_counter_val.yml

With the following log messages being posted:

curl localhost:9144/webhook --data '{"message": "5 bytes processed"}'

And this as output:

# HELP bytes_processed Total number of bytes processed
# TYPE bytes_processed counter
bytes_processed 5

Leaving out value: '{{ .bytes }}', in the configuration and running the rest the same way, yields:

# HELP bytes_processed Total number of bytes processed
# TYPE bytes_processed counter
bytes_processed 1

As expected for backwards compatability.

[Skeen]

Assuming this is something you'll consider merging, I will fix the broken tests, get the build to succeed, and then remove the WIP: prefix on the PR.

[fstab]

Thanks a lot for the PR! Yes, aligning counter and gauge sounds like a good idea. Please go ahead with fixing the tests.

Just a heads up: Some tests in tailer are flaky, so you don't need to fix them if their failure has nothing to do with your changes.

[coveralls]

Coverage decreased (-0.1%) to 67.135% when pulling 4eddad3 on magenta-aps:feature/value_on_counter into 1c7e020 on fstab:master.

[Skeen]

Hi,

Do you want me to duplicate the value test from the guage metric for the counter metric, to test out the value configuration key-word and default of 1? - or do you think it is good as is?

[fstab]

I added a few minor comments, apart from that it's looking good.

[fstab]

Hi,

Do you want me to duplicate the value test from the guage metric for the counter metric, to test out the value configuration key-word and default of 1? - or do you think it is good as is?

Yes, that would be good. Sounds like overkill because it's just copy-and-paste, but on the other hand it's an easy addition and it might prevent a bug in the future if someone refactors the counter but forgets that it might have a value.

[Skeen]

Hi,
Do you want me to duplicate the value test from the guage metric for the counter metric, to test out the value configuration key-word and default of 1? - or do you think it is good as is?

Yes, that would be good. Sounds like overkill because it's just copy-and-paste, but on the other hand it's an easy addition and it might prevent a bug in the future if someone refactors the counter but forgets that it might have a value.

I added a test, in 487bcf4.

[Skeen]

@fstab will you take a last look at this, and see if you deem it okay, or if there's more you'd like me to do?

[fstab]

Thanks a lot!