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

Valuecounter aggregator plugin #3523

Merged
merged 9 commits into from
Jun 19, 2018
Merged

Conversation

piotr1212
Copy link
Contributor

@piotr1212 piotr1212 commented Nov 29, 2017

A valuecounter aggregator plugin to count values in specified fields.
Emits the aggregated count of the values.

A usecase for the valuecounter plugin is when you are processing a HTTP access
log (with the logparser input) and want to count the HTTP status codes.

The fields which will be counted must be configured with the field_names
configuration directive. When no field_names is provided the plugin will not
count any fields. The results are emitted in fields in the format:
originalfieldname_fieldvalue = count.

See also: https://community.influxdata.com/t/telegraf-count-field-values-processing-http-response-codes/3214/5

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

@slogar
Copy link

slogar commented Jan 8, 2018

This looks like a feature that we need. Can you please merge this one so we can deploy this?

Copy link
Contributor

@danielnelson danielnelson left a comment

Choose a reason for hiding this comment

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

Can you make these tweaks?:


type ValueCounter struct {
cache map[uint64]aggregate
Field_names []string
Copy link
Contributor

Choose a reason for hiding this comment

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

Call this Fields, which will match with some other aggregator work.

Strictly speaking, we could probably use the normal filtering options, since this is essentially fieldpass, but I think it is important to require this to be opt in.

## aggregator and will not get sent to the output plugins.
drop_original = false
## The fields for which the values will be counted
field_names = ""
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be a list: fields = []


// Check if this metric has fields which we need to count, if so increment
// the count.
for fk, fv := range in.Fields() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should skip floats? One thing that would be bad is if there is a frequently changing float value, which could result in many new fields.

If we don't disallow this, maybe we should add a warning in the README.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good suggestion, will check if I can implement this

}
}

// Emmit the counters
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: Emit

Choose a reason for hiding this comment

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

Hi Daniel
My requirement is to filter the logs by response code. If the response code is 500 then only we have to insert in the influx db

Please suggest me the approach

# ValueCounter Aggregator Plugin

The valuecounter plugin counts the occurance of values in fields and emmits the
counter once every 'preriod' seconds.
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: period

@piotr1212
Copy link
Contributor Author

Sorry completely missed the notification about your review. Will check it asap.

@piotr1212
Copy link
Contributor Author

Apart from the float check the suggestions are implemented.

For some reason when running head I don't see events from inputs.logparser going through the aggregators at all. I do see events from other plugins passing by. I'm not sure if this is a bug or that I am doing something wrong here..

@danielnelson
Copy link
Contributor

Most likely the logs you are parsing are old, and only the metrics with a timestamp in the current period are added to the aggregators. There is also a minor bug with service inputs adding metrics too early right at startup, sometimes I run into it when working with very small test sets.

@piotr1212
Copy link
Contributor Author

The last commit implements filtering on type as suggested. I've also updated the docs.

"Must be an int, string or bool. Ignoring.")
continue
case uint8, uint16, uint32, uint64, int8,
int16, int32, int64, string, bool:
Copy link
Contributor

Choose a reason for hiding this comment

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

Only need to support int64, uint64, bool, and string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, thanks, updated.

@danielnelson danielnelson requested a review from glinton June 13, 2018 00:53
@piotr1212
Copy link
Contributor Author

I think the failing 'release' test is unrelated

@danielnelson
Copy link
Contributor

I think it will clear up if you merge or rebase master.

Copy link
Contributor

@glinton glinton left a comment

Choose a reason for hiding this comment

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

lgtm

@danielnelson danielnelson merged commit 3ad1028 into influxdata:master Jun 19, 2018
@piotr1212
Copy link
Contributor Author

thanks

mkboudreau added a commit to mkboudreau/telegraf that referenced this pull request Jun 22, 2018
* origin: (39 commits)
  Document path tag in tail input
  Update changelog
  Added path tag to tail input plugin (influxdata#4292)
  Run windows tests with -short
  Fix postfix input handling of multi-level queues (influxdata#4333)
  Update changelog
  Add support for comma in logparser timestamp format (influxdata#4311)
  Update vendoring to dep from gdm (influxdata#4314)
  Update changelog
  Add new measurement with results of pgrep lookup to procstat input (influxdata#4307)
  Update changelog
  Add valuecounter aggregator plugin (influxdata#3523)
  Update changelog
  Update docker input documentation for container status
  Add container status tag to docker input (influxdata#4259)
  Drop CI support for Go 1.8 (influxdata#4309)
  Update changelog
  Fix selection of tags under nested objects in the JSON parser (influxdata#4284)
  Update changelog
  Add owner tag on partitions in burrow input (influxdata#4281)
  ...
maxunt pushed a commit that referenced this pull request Jun 26, 2018
@dataviruset
Copy link

It would be great if there would only be one field and the counted values would be tagged instead of having multiple fields. Makes it easier to do GROUP BY etc.

@danielnelson
Copy link
Contributor

@dataviruset You might want to use the regex and/or converter processors instead. Open a new issue with more details about your use case or post on the InfluxData Community site if you want to discuss further.

@dataviruset
Copy link

@danielnelson Thanks for the reply. I created a post with more details on the community: https://community.influxdata.com/t/tail-and-count-log-file-with-tags/6484

rgitzel pushed a commit to rgitzel/telegraf that referenced this pull request Oct 17, 2018
otherpirate pushed a commit to otherpirate/telegraf that referenced this pull request Mar 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants