Burrow input plugin #3489

arkady-emelyanov · 2017-11-20T17:00:45Z

Required for all PRs:

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

This is another implementation of Burrow input plugin, completely rewritten and fully tested version of #2485

Besides, it supports Basic HTTP auth and SSL.

Burrow is a monitoring companion for Apache Kafka that provides consumer lag and topic offset checking.

arkady-emelyanov · 2017-12-02T09:47:36Z

We are using this plugin (and custom build of telegraf) for about 5 days, and now plugin is feature complete and considered stable.
Could you please accept this plugin to mainline?

danielnelson · 2017-12-04T20:20:57Z

Thanks @arkady-emelyanov, we will make sure to consider this in our planning.

fenneh · 2018-01-02T14:43:12Z

Any ETA on a potential merge?

arkady-emelyanov · 2018-01-15T14:26:53Z

Burrow stable release update. Minimal supported version - Burrow v1.0.0

Changes:

Supported API updated from v2 to v3
Measurements simplification

varun06 · 2018-01-17T19:49:05Z

plugins/inputs/burrow/README.md

+### Configuration
+
+```
+  ## Burrow endpoints in format "sheme://[user:password@]host:port"


Yeah, typo. Fixed.

arkady-emelyanov · 2018-02-06T11:01:27Z

PR has been rebased to resolve conflicts, but now, circle-ci fails with

/var/lib/gems/2.3.0/gems/ffi-1.9.21/ext/ffi_c/libffi/autogen.sh: 2: exec: autoreconf: not found

Which have no relations with plugin implementation, any thoughts?

danielnelson · 2018-02-06T21:38:25Z

@arkady-emelyanov This is fixed now but you may need to rebase.

arkady-emelyanov · 2018-02-07T08:37:27Z

@danielnelson now failure in statsd. Ok, i'll just wait for a few days and then do rebase again.

arkady-emelyanov · 2018-03-23T15:56:03Z

@danielnelson still no ETA? :(

danielnelson · 2018-03-23T18:38:13Z

Sorry about the delay, I'll schedule a review for 1.7

danielnelson

@arkady-emelyanov I'm really impressed with how detailed you were with pr. However, I don't understand why we need to use channels for processing and returning values. From what I can see there are no long running requests so it would be much simpler to remove the channels. I think the only channel we need is for the connection semaphore.

danielnelson · 2018-04-24T01:15:41Z

plugins/inputs/burrow/README.md

+  ## e.g.
+  ##   servers = ["http://localhost:8080"]
+  ##   servers = ["https://example.com:8000"]
+  ##   servers = ["http://user:[email protected]:8000"]


I suggest we remove support for the userinfo section of the URL. My experience with it in other plugins is that the escaping passwords is tricky for users and causes confusion about where the credentials end up being passed. I realize that this allows per server credentials but it may be better to require multiple plugin instantiations for this unless we expect this to be very common.

How about only using the username and password fields below?

danielnelson · 2018-04-24T01:17:43Z

plugins/inputs/burrow/README.md

+  servers = [ "http://127.0.0.1:8000" ]
+
+  ## Prefix all HTTP API requests.
+  #api_prefix = "/v3/kafka"


I'm guessing this is so you can use the plugin with v2 or future versions? Can you add a list of versions that the plugin has been tested against.

danielnelson · 2018-04-24T01:18:25Z

plugins/inputs/burrow/README.md

+  #topics = ["topicA"]
+
+  ## Concurrent connections limit (per server), default is 4.
+  #max_concurrent_connections = 10


Use default:

#max_concurrent_connections = 4

Or perhaps 10 is the actual default, in which case update the comment.

danielnelson · 2018-04-24T01:19:14Z

plugins/inputs/burrow/burrow.go

+func (b *burrow) Gather(acc telegraf.Accumulator) error {
+	var workers sync.WaitGroup
+
+	errorChan := b.getErrorChannel(acc)


Just pass in the Accumulator to avoid channels

danielnelson · 2018-04-24T01:20:23Z

plugins/inputs/burrow/burrow.go

+
+	errorChan := b.getErrorChannel(acc)
+	for _, addr := range b.Servers {
+		c, err := b.getClient(acc, addr, errorChan)


Make the clients once on first run, guarded so it only runs once.

danielnelson · 2018-04-24T01:22:05Z

plugins/inputs/burrow/burrow.go

+	}
+
+	// enable SSL configuration (if provided by configuration)
+	tlsCfg, err := internal.GetTLSConfig(b.SSLCert, b.SSLKey, b.SSLCA, b.InsecureSkipVerify)


Run once and share for all servers.

danielnelson · 2018-04-24T01:22:24Z

plugins/inputs/burrow/burrow.go

+	// construct client
+	c = apiClient{
+		client: http.Client{
+			Transport: &http.Transport{


Use the same transport across plugin.

danielnelson · 2018-04-24T01:47:25Z

plugins/inputs/burrow/README.md

+  #debug = false
+
+  ## Disable burrow_group measurement, default is false
+  #disable_groups = false


Is this option related to the groups option above? If so what we usually do is add an include/exclude option pair like in the docker input, these support globs:

groups_include = [] groups_exclude = []

danielnelson · 2018-04-24T22:06:37Z

plugins/inputs/burrow/README.md

+  #worker_queue_length = 5
+
+  ## Log requests and response codes, default is false
+  #debug = false


Remove this option, you can just log at debug level D!.

danielnelson · 2018-04-24T22:09:45Z

plugins/inputs/burrow/burrow_group.go

+	for _, cluster := range clusterList {
+		escaped := url.PathEscape(cluster)
+		producerChan <- apiCall{
+			URI:     fmt.Sprintf("%s/%s/consumer", api.apiPrefix, escaped),


I suggest using the url package for creating urls.

arkady-emelyanov · 2018-05-20T15:08:30Z

@danielnelson sorry for delay.

Well, i made full rewrite of code, hope now, it feels in harmony with the rest of the plugins. Could you please take a look again?

danielnelson · 2018-05-21T22:14:39Z

plugins/inputs/burrow/burrow.go

+	var err error
+
+	// compile glob patterns
+	b.gClusters, err = makeGlobs(b.Clusters)


You can use filter.Compile( for the globs, I believe it works in the same manner.

danielnelson · 2018-05-21T22:16:34Z

plugins/inputs/burrow/README.md

+
+  ## Filter out clusters by providing list of glob patterns.
+  ## Default is no filtering.
+  # clusters = []


Rename to clusters_exclude so that in the future you can add cluster_include, or add them both now and you can use filters.NewIncludeExcludeFilter(. Also applies to groups and topics.

Thanks, totally missed that filter package is already present and very useful. I did include/exclude separation and replaced glob with filter.

danielnelson · 2018-05-21T22:22:03Z

plugins/inputs/burrow/testdata/error.json

+    "group": "",
+    "topic": ""
+  }
+}


Can you add a newline at the end of these json files?

* newlines for json * use filter package * include/exclude options for clusters, groups and topics

danielnelson · 2018-05-22T21:57:14Z

Thanks @arkady-emelyanov, merged for 1.7!

arkady-emelyanov · 2018-05-22T21:59:41Z

@danielnelson indeed. my longest PR ever :)
Thanks!

danielnelson · 2018-05-22T22:20:48Z

Yeah, sorry about that. Hoping to add another maintainer soon.

arkady-emelyanov force-pushed the burrow-input-plugins branch from 062a456 to 1a863dc Compare November 23, 2017 15:40

arkady-emelyanov changed the title ~~Implement Burrow input plugin~~ WIP: Implement Burrow input plugin Nov 23, 2017

arkady-emelyanov force-pushed the burrow-input-plugins branch from aff0fec to c721b8d Compare November 30, 2017 13:20

arkady-emelyanov changed the title ~~WIP: Implement Burrow input plugin~~ Burrow input plugin Dec 2, 2017

arkady-emelyanov mentioned this pull request Dec 4, 2017

Add Burrow input plugin. #2485

Closed

3 tasks

danielnelson mentioned this pull request Dec 6, 2017

Lag in messages per follower replica is giving 404 #2873

Closed

arkady-emelyanov force-pushed the burrow-input-plugins branch from aa62064 to 1f85c0e Compare January 15, 2018 19:46

This was referenced Jan 17, 2018

I want to provide influxdb support linkedin/Burrow#210

Open

Feature request - Prometheus metrics support linkedin/Burrow#318

Open

Adding graphite notifier linkedin/Burrow#136

Closed

varun06 reviewed Jan 17, 2018

View reviewed changes

arkady-emelyanov force-pushed the burrow-input-plugins branch from 8740260 to 63ba09d Compare February 6, 2018 10:55

arkady-emelyanov force-pushed the burrow-input-plugins branch from 63ba09d to aa683ed Compare February 7, 2018 08:32

arkady-emelyanov force-pushed the burrow-input-plugins branch from aa683ed to 6ad8772 Compare February 12, 2018 08:41

danielnelson added this to the 1.7.0 milestone Mar 23, 2018

danielnelson added the new plugin label Mar 23, 2018

danielnelson reviewed Apr 24, 2018

View reviewed changes

arkady-emelyanov force-pushed the burrow-input-plugins branch from 6ad8772 to 6061716 Compare May 18, 2018 10:44

arkady-emelyanov changed the title ~~Burrow input plugin~~ WIP: Burrow input plugin May 18, 2018

arkady-emelyanov force-pushed the burrow-input-plugins branch 2 times, most recently from ca5801d to 6061716 Compare May 20, 2018 14:36

arkady-emelyanov force-pushed the burrow-input-plugins branch 2 times, most recently from b12eaf2 to de39ff9 Compare May 20, 2018 14:54

arkady-emelyanov changed the title ~~WIP: Burrow input plugin~~ Burrow input plugin May 20, 2018

danielnelson reviewed May 21, 2018

View reviewed changes

arkady-emelyanov added 4 commits May 22, 2018 11:49

Implement Burrow input plugin

9fe0533

Implement Burrow input plugin

57a8214

Fix after rebase

11ef36f

Review suggestions implemented:

091f728

* newlines for json * use filter package * include/exclude options for clusters, groups and topics

arkady-emelyanov force-pushed the burrow-input-plugins branch from 68ba41c to 091f728 Compare May 22, 2018 08:55

danielnelson merged commit fd22b1e into influxdata:master May 22, 2018

maxunt pushed a commit that referenced this pull request Jun 26, 2018

Add burrow input plugin (#3489)

2f9b180

otherpirate pushed a commit to otherpirate/telegraf that referenced this pull request Mar 15, 2019

Add burrow input plugin (influxdata#3489)

708569f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Burrow input plugin #3489

Burrow input plugin #3489

arkady-emelyanov commented Nov 20, 2017

arkady-emelyanov commented Dec 2, 2017

danielnelson commented Dec 4, 2017

fenneh commented Jan 2, 2018

arkady-emelyanov commented Jan 15, 2018

varun06 Jan 17, 2018

arkady-emelyanov Jan 18, 2018

arkady-emelyanov commented Feb 6, 2018

danielnelson commented Feb 6, 2018

arkady-emelyanov commented Feb 7, 2018

arkady-emelyanov commented Mar 23, 2018

danielnelson commented Mar 23, 2018

danielnelson left a comment

danielnelson Apr 24, 2018

danielnelson Apr 24, 2018

danielnelson Apr 24, 2018

danielnelson Apr 24, 2018

danielnelson Apr 24, 2018

danielnelson Apr 24, 2018

danielnelson Apr 24, 2018

danielnelson Apr 24, 2018

danielnelson Apr 24, 2018

danielnelson Apr 24, 2018

arkady-emelyanov commented May 20, 2018

danielnelson May 21, 2018

danielnelson May 21, 2018

arkady-emelyanov May 22, 2018

danielnelson May 21, 2018

arkady-emelyanov May 22, 2018

danielnelson commented May 22, 2018

arkady-emelyanov commented May 22, 2018 •

edited

Loading

danielnelson commented May 22, 2018

Burrow input plugin #3489

Burrow input plugin #3489

Conversation

arkady-emelyanov commented Nov 20, 2017

Required for all PRs:

arkady-emelyanov commented Dec 2, 2017

danielnelson commented Dec 4, 2017

fenneh commented Jan 2, 2018

arkady-emelyanov commented Jan 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arkady-emelyanov commented Feb 6, 2018

danielnelson commented Feb 6, 2018

arkady-emelyanov commented Feb 7, 2018

arkady-emelyanov commented Mar 23, 2018

danielnelson commented Mar 23, 2018

danielnelson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arkady-emelyanov commented May 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielnelson commented May 22, 2018

arkady-emelyanov commented May 22, 2018 • edited Loading

danielnelson commented May 22, 2018

arkady-emelyanov commented May 22, 2018 •

edited

Loading