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

NATS output plugin #1697

Closed
wants to merge 5 commits into from
Closed

NATS output plugin #1697

wants to merge 5 commits into from

Conversation

pires
Copy link
Contributor

@pires pires commented Sep 1, 2016 •
edited
Loading

Required for all PRs:

  • CHANGELOG.md updated (we recommend not updating this until the PR has been approved by a maintainer)
  • Sign CLA (if not already signed)
  • README.md updated (if adding a new plugin)

Fixes #1487

@nhaugo nhaugo added this to the 1.1.0 milestone Sep 1, 2016
@jwilder jwilder added the feature request Requests for new plugin and for new features to existing plugins label Sep 1, 2016
@subfuzion subfuzion mentioned this pull request Sep 1, 2016
Closed
9 tasks
@sparrc
Copy link
Contributor

sparrc commented Sep 6, 2016

looks good @pires, can you change the SSL handling to use the internal.GetTLSConfig function? You can see how the influxdb output plugin does it here: https://github.com/influxdata/telegraf/blob/master/plugins/outputs/influxdb/influxdb.go#L91

@pires
Copy link
Contributor Author

pires commented Sep 6, 2016

@sparrc sure thing. On it!

@sparrc
Copy link
Contributor

sparrc commented Sep 6, 2016

also you can update the changelog with this under the 1.1 section 👍

@pires
Copy link
Contributor Author

pires commented Sep 6, 2016

@sparrc yep, was just waiting on your approval.

@pires
Copy link
Contributor Author

pires commented Sep 6, 2016

@sparrc done, sir!

@sparrc sparrc closed this in 6b1cc67 Sep 6, 2016
@subfuzion subfuzion mentioned this pull request Sep 6, 2016
Closed
jackzampolin pushed a commit that referenced this pull request Oct 7, 2016
@pires @jackzampolin
Add NATS output plugin.
a4c0557 
Added NATS server container needed for tests.

Added NATS output plug-in. Fixes #1487

NATS output plug-in use internal.GetTLSConfig to instrument TLS configuration.

Added NATS output plug-in to changelog.

closes #1487
closes #1697
@m4ce
Copy link
Contributor

m4ce commented Oct 26, 2016 •
edited
Loading

Looks like if the NATS server happens to be unreachable and later comes back, telegraf never recovers:

2016/10/26 10:35:10 I! Output [nats] buffer fullness: 10045 / 10000 metrics. Total gathered metrics: 650575. Total dropped metrics: 9125.
2016/10/26 10:35:10 E! Error writing to output [nats]: FAILED to send NATS message: nats: connection closed

@sparrc
Copy link
Contributor

sparrc commented Oct 26, 2016

@m4ce please open a proper issue, commenting on a closed PR is not productive

@pires pires deleted the 1487-nats_output_plugin branch October 26, 2016 13:28
@pires
Copy link
Contributor Author

pires commented Oct 26, 2016

@m4ce that shouldn't be the case since by default NATS client will try to reconnect.

If you happen to open an issue as correctly suggested by @sparrc, make sure you CC me.

@m4ce
Copy link
Contributor

m4ce commented Oct 26, 2016

Sorry for writing this here. Thank you guys for looking into this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
feature request Requests for new plugin and for new features to existing plugins
Projects
None yet
Milestone
1.1.0
Development

Successfully merging this pull request may close these issues.
5 participants
@pires @sparrc @m4ce @jwilder @nhaugo