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

Wavefront output enhancements #5161

Merged
merged 14 commits into from
Dec 21, 2018
Merged

Wavefront output enhancements #5161

merged 14 commits into from
Dec 21, 2018

Conversation

puckpuck
Copy link
Contributor

Required for all PRs:

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

Updated connection and communication to use Wavefront's go-sdk. Benefits include:

  • Improved performance for higher throughput
  • Can be used with Wavefront Direction Ingestion API (https)
  • Wavefront proxy can still be used, and http is now supported (in addition to tcp)

Also marked string_to_number feature as deprecated since this is now done using the enum plugin. Goal is to remove it completely in Telegraf 2.

@danielnelson danielnelson added this to the 1.10.0 milestone Dec 19, 2018
@danielnelson danielnelson added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Dec 19, 2018
@@ -264,6 +269,7 @@ func TestBuildValueString(t *testing.T) {

}

/*
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete or update this test as appropriate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should TestBuildValueString should remain since the functionality is deprecated but not removed yet?

Did delete commented out test below (TestFormatMetricPoint)

Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove TestBuildValueString once the option is removed, for now its best to leave it.

/*
metricLine := formatMetricPoint(point, w)
_, err := connection.Write([]byte(metricLine))
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove commented out code.

plugins/outputs/wavefront/wavefront.go Show resolved Hide resolved
@@ -246,6 +246,10 @@
name = "github.com/vishvananda/netlink"
revision = "b2de5d10e38ecce8607e6b438b6d174f389a004e"

[[constraint]]
name = "github.com/wavefronthq/wavefront-sdk-go"
version = "v0.9.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

There should be some changes to Gopkg.lock as well, which is why the tests aren't working, just need to run dep ensure once.

sender, err := wavefront.NewDirectSender(&wavefront.DirectConfiguration{
Server: w.Url,
Token: w.Token,
FlushIntervalSeconds: 5,
Copy link
Contributor

Choose a reason for hiding this comment

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

Telegraf considers returning from Write() without error to be successful delivery of metrics, but with the extra layer of buffering we don't actually know if the metric was sent. There may be some reduction in durability because of this, might be worth considering a sender implementation that sends immediately without buffering?

@danielnelson danielnelson merged commit 9cc0670 into influxdata:master Dec 21, 2018
trevorwhitney pushed a commit to trevorwhitney/telegraf that referenced this pull request Feb 14, 2019
otherpirate pushed a commit to otherpirate/telegraf that referenced this pull request Mar 15, 2019
otherpirate pushed a commit to otherpirate/telegraf that referenced this pull request Mar 15, 2019
dupondje pushed a commit to dupondje/telegraf that referenced this pull request Apr 22, 2019
bitcharmer pushed a commit to bitcharmer/telegraf that referenced this pull request Oct 18, 2019
athoune pushed a commit to bearstech/telegraf that referenced this pull request Apr 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants