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 cgroup plugin #1350

Closed
wants to merge 2 commits into from
Closed

Add cgroup plugin #1350

wants to merge 2 commits into from

Conversation

vlasad
Copy link
Contributor

@vlasad vlasad commented Jun 8, 2016

Required for all PRs:

  • CHANGELOG.md updated
  • Sign CLA (if not already signed)
  • README.md updated (if adding a new plugin)

@sebito91
Copy link
Contributor

sebito91 commented Jun 8, 2016

👍 from us!

@Will-Beninger
Copy link

👍 from us! I've pulled in your changes and have already started testing the plugin.

No issues so far and would love to see it pulled into 1.0.1 beta version!

@sebito91
Copy link
Contributor

Any idea when we can merge this? Waiting for v.1.0.0?

@sparrc
Copy link
Contributor

sparrc commented Jun 13, 2016

Hi @sebito91, sorry for the delay but I've been very busy with work on the logparser plugin. I have been trying to concentrate on that but can hopefully get to these PRs by the middle of the week.

@Will-Beninger
Copy link

@sparrc Appreciate it! My workaround at the moment to integrate cgroups and then our own plugins is quite ugly at the moment. haha

@sparrc
Copy link
Contributor

sparrc commented Jun 14, 2016

Looks good on the whole, but are the flush_scope and writeWithBatches changes still necessary considering #1321?

@sparrc
Copy link
Contributor

sparrc commented Jun 22, 2016

@vlasad bump, is the flush_scope argument still necessary? Seems like it is a duplication of the change @maksadbek is working on.

@vlasad
Copy link
Contributor Author

vlasad commented Jun 23, 2016

It's not necessary already. I have removed it.

@sparrc sparrc closed this in d641c42 Jun 23, 2016
@sparrc
Copy link
Contributor

sparrc commented Jun 23, 2016

Thank you @vlasad !

I hope you don't mind I made one small change: I've changed the external configuration to be "files" instead of "fields". To me "files" makes more sense, and since they were already internally called "files" I went ahead and changed it externally.

@vlasad
Copy link
Contributor Author

vlasad commented Jun 23, 2016

It's OK. Also It's necessary to rename it in the README.md .

chebrolus pushed a commit to chebrolus/telegraf that referenced this pull request Jun 24, 2016
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

Successfully merging this pull request may close these issues.

4 participants