[outputs.signalfx] Add output plugin for SignalFX by amoghe · Pull Request #5204 · influxdata/telegraf

amoghe · 2018-12-30T03:16:50Z

This output plugin converts the telegraf.Metrics into signalfx
datapoints and then transmits them to the ingest servers using
signalfx golang client lib.

As of this commit, the client lib is allowed to pick sane defaults
and none of its fields are overridable via telegraf config. This
can be changed in the future if needed.

The unit tests only test for conversion of telegraf.Metrics to
the datapoint structs. All code that executes after that is
assumed to be tested in the signalfx client lib itself (and not
worth writing end-to-end tests for).

TODO: datapoints are flushed back to the signalfx server in the
batch size that is passed into the Write method of this plugin.
However we could allow this batch size to be configurable via
some plugin config knobs if desired.

Required for all PRs:

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

amoghe · 2019-01-03T21:56:22Z

attn: @danielnelson

danielnelson · 2019-01-03T22:21:56Z

Thanks, I'll try to check it out soon, but will need some time to look through the new dependencies.

amoghe · 2019-01-26T00:39:35Z

can I get some eyes on this please ❓

danielnelson · 2019-01-28T03:16:18Z

@amoghe Sorry, I'll take a look this week, thanks for being patient.

danielnelson

Looks very good, nice straightforward code which I love :). Here are the final bits I think are needed:

plugins/outputs/signalfx/README.md

plugins/outputs/signalfx/signalfx.go

Gopkg.lock

danielnelson · 2019-04-16T01:02:42Z

There is a fix for the syslog test failure in master, can you merge or rebase?

amoghe · 2019-04-17T20:29:03Z

There is a fix for the syslog test failure in master, can you merge or rebase?

Done. (rebased on master)

amoghe · 2019-05-29T02:57:09Z

@danielnelson , looks like this PR is has two outstanding comments

~~text of the ASL for a dependency~~
canonical import path in a depdency

Do we have to hold up this PR on item 2?

danielnelson · 2019-05-30T20:21:27Z

@amoghe I think the upstream PR is stalled, do you think you could open a new one with the change and addressing the comments?

mstumpfx · 2019-06-12T19:20:10Z

@danielnelson Hi - since the PR stalled I've created a new one to get things moving again- signalfx/golib#138

mstumpfx · 2019-06-12T21:31:50Z

@danielnelson signalfx/golib#138 has been merged into master - anything else required here?

danielnelson · 2019-06-14T18:51:33Z

Just need to update the Gopkg.toml to resolve the conflict and set the new import path.

This output plugin converts the `telegraf.Metrics` into signalfx `datapoint`s and then transmits them to the ingest servers using signalfx golang client lib. As of this commit, the client lib is allowed to pick sane defaults and none of its fields are overridable via telegraf config. This can be changed in the future if needed. The unit tests only test for conversion of `telegraf.Metric`s to the `datapoint` structs. All code that executes after that is assumed to be tested in the signalfx client lib itself (and not worth writing end-to-end tests for). TODO: datapoints are flushed back to the signalfx server in the batch size that is passed into the `Write` method of this plugin. However we could allow this batch size to be configurable via some plugin config knobs if desired.

1. try converting bools to int instead of dropping them 2. fix readme for this output plugin 3. prevent multiple calls to `m.Tags()` 4. add more detail to debug log line

So that we pick up all the changes to Gopkg files from master.

amoghe · 2019-06-14T19:06:32Z

Just need to update the Gopkg.toml to resolve the conflict and set the new import path.

Rebased and then force-pushed. PTAL

danielnelson · 2019-06-14T21:04:18Z

Gopkg.toml

+
+[[constraint]]
+  name = "github.com/signalfx/golib"
+  version = "2.0.0"


Can you set this to branch = "master" with a comment that we are waiting on 2.3.5 or later for dependency changes. I'll make a note to update this before the next release.

# Gopkg.lock is out of sync: github.com/signalfx/golib@v2.0.0: not allowed by constraint master

Hmmm, looks like the CI build failed. I'm not sure what is going on here (because I'm not very familiar with dep). It looks like even though I updated Gopkg.toml to specify branch = "master", and then ran make deps, the Gopkg.lock still has this:

[[projects]] digest = "1:3d52a2ff8fabf5cb15a88e8451a524af87f203c80ad8a33209fd52e15eaa4997" name = "github.com/signalfx/golib" packages = [ "datapoint", "errors", "event", "eventcounter", "log", "sfxclient", "timekeeper", "trace", "trace/format", ] pruneopts = "" revision = "271e7b77c46c2cb201be901a302cb181a9d1645a" version = "v2.0.0"

I suspect the version shouldn't be here any more after a successful run of dep ensure...

@danielnelson - any pointers on what to do next?

I get a couple errors running dep ensure, first this one:

master: Could not introduce github.com/signalfx/golib@master, as it has a dependency on github.com/go-logfmt/logfmt with constraint ^0.3.0, which has no overlap with existing constraint ^0.4.0 from (root)

This can be fixed by changing Telegraf's logfmt dependency to ">=0.3.0, <0.5.0", or another fix would be to do the same on signalfx. It doesn't appear to cause an issue, tests are still passing.

After changing this I get:

master: Could not introduce github.com/signalfx/golib@master, as it has a dependency on github.com/golang/protobuf with constraint 1643683e1b54a9e88ad26d98f81400c8c9d9f4f9, which has no overlap with existing constraint ^1.1.0 from (root)

This one probably needs to be fixed in signalfx: `version = "1.0.0" or such, since they are saying only this revision of protobuf will work, which is too strict and I expect any version >= 1.0.0 (the first release) would be fine. At any rate I believe we need 1.1.0 due to an existing dependency. It looks like this revision is a bit older than 1.0.0, but maybe they would be willing to move it forward so that we can pin a release.

This can be fixed by changing Telegraf's logfmt dependency to ">=0.3.0, <0.5.0"

I can create a commit for this

which is too strict and I expect any version >= 1.0.0 (the first release) would be fine.

@mstumpfx - since we have your eyes on this - what are your thoughts on making this change on the golib side?

@amoghe I will talk to to a couple others about this today and figure out why we pin this revision, and if it can be updated. Will get back to you.

Our protobuf version is tightly coupled to our client libraries so this may take a bit of work on our end, I'm still talking to a couple of the engineers who typically maintain the repo about it.

Sorry for the delayed response - we're in the process of getting this prioritized on our end, and there's a few different teams involved. I don't have much finite information to share yet, but things are in motion.

@mstumpfx - thanks for the update!

… available

mstumpfx · 2019-11-04T14:21:15Z

@amoghe & @danielnelson
We have updated golib protobuf to use version >= 1.0 now
signalfx/golib#158

This was released in v2.5.0 of golib.

The log.fmt issue still may exist, we restrict to v0.3.0
https://github.com/signalfx/golib/blob/master/Gopkg.toml#L62

danielnelson · 2019-11-08T00:11:04Z

I believe we can move our dependency back to 0.3.0, though it would be nice if signalfx accepted both versions. I don't see any breaking interface changes.

@amoghe Would you be able update your pull request?

amoghe · 2019-11-10T03:19:48Z

I suppose my inexperience with dep may be causing this:

v2.0.0: Could not introduce github.com/signalfx/golib@v2.0.0, as it is not allowed by constraint ^2.5.0 from project github.com/influxdata/telegraf.
v2.5.1: Could not introduce github.com/signalfx/golib@v2.5.1, as it has a dependency on github.com/go-logfmt/logfmt with constraint ^0.3.0, which has no overlap with existing constraint ^0.4.0 from (root)
v2.5.0: Could not introduce github.com/signalfx/golib@v2.5.0, as it has a dependency on github.com/go-logfmt/logfmt with constraint ^0.3.0, which has no overlap with existing constraint ^0.4.0 from (root)

amoghe · 2019-11-10T03:37:00Z

I tried to update my branch using dep ensure -update github.com/signalfx/golib , which also results in errors

Solving failure: No versions of github.com/signalfx/golib met constraints:
	v2.5.1: Could not introduce github.com/signalfx/golib@v2.5.1, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v2.5.0: Could not introduce github.com/signalfx/golib@v2.5.0, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	2.4.3: Could not introduce github.com/signalfx/golib@2.4.3, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v2.4.3: Could not introduce github.com/signalfx/golib@v2.4.3, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v2.4.2: Could not introduce github.com/signalfx/golib@v2.4.2, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v2.4.1: Could not introduce github.com/signalfx/golib@v2.4.1, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v2.4.0: Could not introduce github.com/signalfx/golib@v2.4.0, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v2.3.4: Could not introduce github.com/signalfx/golib@v2.3.4, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v2.3.3: Could not introduce github.com/signalfx/golib@v2.3.3, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v2.3.2: Could not introduce github.com/signalfx/golib@v2.3.2, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v2.3.1: Could not introduce github.com/signalfx/golib@v2.3.1, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v2.3.0: Could not introduce github.com/signalfx/golib@v2.3.0, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v2.2.0: Could not introduce github.com/signalfx/golib@v2.2.0, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v2.1.0: Could not introduce github.com/signalfx/golib@v2.1.0, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v2.0.0: Could not introduce github.com/signalfx/golib@v2.0.0, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v1.1.7: Could not introduce github.com/signalfx/golib@v1.1.7, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v1.1.6: Could not introduce github.com/signalfx/golib@v1.1.6, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v1.1.5: Could not introduce github.com/signalfx/golib@v1.1.5, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v1.1.4: Could not introduce github.com/signalfx/golib@v1.1.4, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v1.1.3: Could not introduce github.com/signalfx/golib@v1.1.3, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v1.1.2: Could not introduce github.com/signalfx/golib@v1.1.2, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v1.1.1: Could not introduce github.com/signalfx/golib@v1.1.1, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v1.1.0: Could not introduce github.com/signalfx/golib@v1.1.0, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	v1.0.0: Could not introduce github.com/signalfx/golib@v1.0.0, as it is not allowed by constraint master from project github.com/influxdata/telegraf.
	master: Could not introduce github.com/signalfx/golib@master, as it has a dependency on github.com/go-logfmt/logfmt with constraint ^0.3.0, which has no overlap with existing constraint ^0.4.0 from (root)

keitwb · 2019-11-20T19:19:06Z

@amoghe I rebased your branch on top of the upstream master to fix the merge conflicts and changed the versions of golib and logfmt to work. I was able to get a clean build that way. The code is in the signalfx fork at https://github.com/signalfx/telegraf/tree/amoghe_signalfx_output.

If you want, you can force push your branch to what is at the equivalent signalfx fork branch and it should fix this PR. Or you can close this one and I'll just open one with my version -- whatever is easiest for you.

amoghe · 2019-11-21T18:01:33Z

@keitwb - I can close this PR and you can create a new one with all these changes in it.

amoghe · 2019-11-22T07:14:07Z

@keitwb - please paste a link to the new PR in a comment , so that we can follow this one into the next

keitwb · 2019-11-25T12:39:09Z

Ok thanks @amoghe. I realized some enhancements that need to be made to your work such as being able to specify the datapoint url, so I'm going to add that and then make the PR. Will link it here when done.

amoghe mentioned this pull request Dec 30, 2018

signalfx output plugin #923

Closed

amoghe force-pushed the amoghe_signalfx_output branch 2 times, most recently from 8eabea7 to 04b460e Compare December 31, 2018 23:02

danielnelson added the new plugin label Jan 2, 2019

danielnelson self-requested a review January 3, 2019 22:19

danielnelson suggested changes Feb 3, 2019

View reviewed changes

amoghe force-pushed the amoghe_signalfx_output branch from 58a928a to ae63aa2 Compare April 17, 2019 20:28

amoghe force-pushed the amoghe_signalfx_output branch from ae63aa2 to effba2b Compare May 29, 2019 01:09

amoghe added 3 commits June 14, 2019 12:03

[outputs.signalfx] Address review comments

b4fbc17

1. try converting bools to int instead of dropping them 2. fix readme for this output plugin 3. prevent multiple calls to `m.Tags()` 4. add more detail to debug log line

[outputs.signalfx] Rebase on top of master

2380008

So that we pick up all the changes to Gopkg files from master.

amoghe force-pushed the amoghe_signalfx_output branch from effba2b to 2380008 Compare June 14, 2019 19:05

danielnelson reviewed Jun 14, 2019

View reviewed changes

[outputs.signalfx] Use master branch of signalfx/golib until 2.3.5 is…

452571b

… available

keitwb mentioned this pull request Oct 1, 2019

collectd vs golang-based default monitors? signalfx/signalfx-agent#1000

Closed

amoghe closed this Nov 22, 2019

keitwb mentioned this pull request Nov 25, 2019

SignalFx Output #6714

Merged

Conversation

amoghe commented Dec 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Required for all PRs:

Uh oh!

amoghe commented Jan 3, 2019

Uh oh!

danielnelson commented Jan 3, 2019

Uh oh!

amoghe commented Jan 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danielnelson commented Jan 28, 2019

Uh oh!

danielnelson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

danielnelson commented Apr 16, 2019

Uh oh!

amoghe commented Apr 17, 2019

Uh oh!

amoghe commented May 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danielnelson commented May 30, 2019

Uh oh!

mstumpfx commented Jun 12, 2019

Uh oh!

mstumpfx commented Jun 12, 2019

Uh oh!

danielnelson commented Jun 14, 2019

Uh oh!

amoghe commented Jun 14, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amoghe Jun 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mstumpfx commented Nov 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danielnelson commented Nov 8, 2019

Uh oh!

amoghe commented Nov 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amoghe commented Nov 10, 2019

Uh oh!

keitwb commented Nov 20, 2019

Uh oh!

amoghe commented Nov 21, 2019

Uh oh!

amoghe commented Nov 22, 2019

Uh oh!

amoghe commented Dec 30, 2018 •

edited

Loading

amoghe commented Jan 26, 2019 •

edited

Loading

amoghe commented May 29, 2019 •

edited

Loading

amoghe Jun 14, 2019 •

edited

Loading

mstumpfx commented Nov 4, 2019 •

edited

Loading

amoghe commented Nov 10, 2019 •

edited

Loading