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.

Sign up for GitHub

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

Jump to bottom

Added filename tag to tail input plugin. #4292

Merged
merged 1 commit into from
Jun 22, 2018
Merged

Added filename tag to tail input plugin. #4292

merged 1 commit into from
Jun 22, 2018

Conversation

wingsof
Copy link
Contributor

@wingsof wingsof commented Jun 15, 2018

Required for all PRs:

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

Added filename tag for tail input plugin.
This information is needed when the case telegraf captures log from multiple files in the directory structure and there's a need to identify source.

danielnelson
danielnelson reviewed Jun 15, 2018
View reviewed changes
plugins/inputs/tail/tail.go Outdated
t.acc.AddFields(m.Name(), m.Fields(), m.Tags(), m.Time())
if m != nil {
tags := m.Tags()
tags["filename"] = tailer.Filename
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change the name of this tag to path? This will just be more consistent with the logparser input, and I hope to combine these plugins at some point.

@wingsof wingsof force-pushed the tail-add-filename branch from b6e0673 to daff426 Compare June 18, 2018 00:16
@wingsof
Copy link
Contributor Author

wingsof commented Jun 19, 2018

Hello @danielnelson , it seems that circleci deps check failed due to operational failure. Would you please re-run check or should I make additional commit to rerun this?

@danielnelson
Copy link
Contributor

@wingsof If you can merge/rebase against our master to get the new CircleCI build image it should resolve this issue.

@wingsof wingsof force-pushed the tail-add-filename branch from daff426 to a21251c Compare June 19, 2018 02:30
@wingsof
Copy link
Contributor Author

wingsof commented Jun 20, 2018

tag name changed to 'path' =-)

and would you share your plan about merging tail and logparser plugin? I was planning to implement capture log from newly created file while telegraf running, but if tail plugin will be merged eventually, maybe I can check logparser plugin first.

@danielnelson
Copy link
Contributor

What we would like to do is make the grok parser in the logparser plugin available as a first class parser available to all plugins that support the data_format option: #3479

Once this is done then tail will be able to do everything that logparser can do, and there will no longer be a need to have both plugins.

@danielnelson danielnelson added this to the 1.8.0 milestone Jun 22, 2018
@danielnelson danielnelson added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Jun 22, 2018
@danielnelson danielnelson merged commit 3f28add into influxdata:master Jun 22, 2018
mkboudreau added a commit to mkboudreau/telegraf that referenced this pull request Jun 22, 2018
@mkboudreau
Merge remote-tracking branch 'origin' into InputPlugin_2673_Traefik
29bb479 
* 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)
  ...
@mjf
Copy link

mjf commented Jun 25, 2018

Once this is done then tail will be able to do everything that logparser can do, and there will no longer be a need to have both plugins.

Does that mean we will be finaly able to use Grok parser for gathering metrices from the /proc procfs filesystem as well (if not using the tail plugin, at least with exec plugin with cat ... or something alike)?

maxunt pushed a commit that referenced this pull request Jun 26, 2018
@wingsof @danielnelson
Added path tag to tail input plugin (#4292)
b895147
glinton pushed a commit that referenced this pull request Jun 26, 2018
@wingsof @danielnelson
Added path tag to tail input plugin (#4292)
ea50159
@danielnelson
Copy link
Contributor

@mjf Yes, once #3479 is done you could do this with an exec script. You will also be able to use it along with a solution for #3883 soon.

@mjf
Copy link

mjf commented Jul 2, 2018

@danielnelson That's great! Thanks!

rgitzel pushed a commit to rgitzel/telegraf that referenced this pull request Oct 17, 2018
@wingsof @rgitzel
Added path tag to tail input plugin (influxdata#4292)
135f329
@dirkdevriendt
Copy link

Ouch, just noticed this change and now our data is being "polluted" with an extra tag that has no value for us...

I understand this can be a very useful feature (so, thanks! :-)), but considering how much of a pain it can be to change tags in influx, it might be better to have it as an optional / opt-in /configurable feature?

@glinton
Copy link
Contributor

glinton commented Oct 29, 2018

@dirkdevriendt That's not a bad idea, in the meantime you can use tagexclude to remove that tag from your metrics.

@dirkdevriendt
Copy link

Thanks @glinton!
That's one of those golden nuggets I must have actually seen without connecting the dots. From now on I'll be using taginclude for these use cases to make sure I'm only getting the tags I opt-in to!

otherpirate pushed a commit to otherpirate/telegraf that referenced this pull request Mar 15, 2019
@wingsof @otherpirate
Added path tag to tail input plugin (influxdata#4292)
b955661
@joriws
Copy link

joriws commented May 17, 2019 •
edited
Loading

I'd like to see same path-added to input.file too! Because if I glob there I want to be able set measurement name by filename-prefix..

I think every input.* which are doing wildcards or globbing should have tag-option or variable which could be used like:
name_override = '${current_glob_value}'
or something..

@glinton
Copy link
Contributor

glinton commented May 17, 2019

@joriws that functionality should be present in telegraf versions newer than 1.8.0. File a new issue or feature request if the behavior is unsatisfactory here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielnelson danielnelson danielnelson left review comments

Assignees
No one assigned
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin
Projects
None yet
Milestone
1.8.0
Development

Successfully merging this pull request may close these issues.
6 participants
@wingsof @danielnelson @mjf @dirkdevriendt @glinton @joriws