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 Burrow input plugin. #2485

Closed
wants to merge 1 commit into from

Conversation

codeb2cc
Copy link
Contributor

@codeb2cc codeb2cc commented Mar 3, 2017

Burrow(https://github.com/linkedin/Burrow/) is a monitoring
companion for Apache Kafka that provides consumer lag and topic
offset checking.

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)

Burrow(https://github.com/linkedin/Burrow/) is a monitoring
companion for Apache Kafka that provides consumer lag and topic
offset checking.
@zeeto-slytherin
Copy link

just wanted to check in and see if you have any updates on this guy? we've been watching it for a bit, would be helpful for our team to have this instead of using yet another tool to pull these in.

@danielnelson
Copy link
Contributor

@zeeto-slytherin I haven't had a chance to review yet. If possible it would be helpful if you, or anyone else in the community reviewed the plugin design, code, or tested out the code. There is an amd64 linux build linked in the circleci build.

@zeeto-slytherin
Copy link

can you drop a link to where the linked artifact is? cannot seem to find it anywhere

@danielnelson
Copy link
Contributor

@danielnelson danielnelson added feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin and removed feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin plugin request labels Aug 12, 2017
@burdandrei
Copy link
Contributor

this could be useful, we're parsing burrow with http now

@arkady-emelyanov arkady-emelyanov mentioned this pull request Nov 20, 2017
3 tasks
@arkady-emelyanov
Copy link
Contributor

@burdandrei @zeeto-slytherin Hey! Since no progress on this PR, I made a rewrite of this plugin #3489. About 90% test coverage, more configuration options, stable (in production for a while). If you are still interested, maybe you will have a look?

@burdandrei
Copy link
Contributor

@arkady-emelyanov wow! i'll take a look! great job!

Copy link
Contributor

@danielnelson danielnelson left a comment

Choose a reason for hiding this comment

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

@codeb2cc Here is a high level review, I think there may be some issues with the output schema that we would need to discuss further but nothing major.

@arkady-emelyanov @codeb2cc We should first decide which burrow plugin to move forward with. Can you join forces on one of the pull requests to address the review comments?

}

var wg sync.WaitGroup
errChan := errchan.New(len(b.Urls))
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using an error channel you can just AddError to the accumulator (this was probably added after you wrote this pull request).

return errChan.Error()
}

var tr = &http.Transport{
Copy link
Contributor

Choose a reason for hiding this comment

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

Create the transport at the same time you instantiate the client, this allows differing settings across multiple plugin instances.

}

var tr = &http.Transport{
ResponseHeaderTimeout: time.Duration(3 * time.Second),
Copy link
Contributor

Choose a reason for hiding this comment

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

http.Client.Timeout is sufficient, so you can remove this timeout

}
acc.AddFields("burrow_consumer", startFields, tags, time.Unix(0, partition.Start.Timestamp*int64(time.Millisecond)))

endFields := map[string]interface{}{
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems problematic to me, since we are using the same tagset and fields as the start offset/lag, and there is potential for overwrite since we are storing the values at a timestamp based on the partition start/end.

Error bool `json:"error"`
Message string `json:"message"`
Status ConsumerGroupStatus `json:"status"`
Request BurrowResponseRequestInfo `json:"request"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably would make sense to have a type:

type BurrowResponse struct {
	Error     bool                      `json:"error"`
	Message   string                    `json:"message"`
	Request   BurrowResponseRequestInfo `json:"request"`
}

Then you should be able to embed it like:

type BurrowResponseConsumerStatus struct {
	Status  ConsumerGroupStatus       `json:"status"`
	BurrowResponse
}

- topic
- partition
- status

Copy link
Contributor

Choose a reason for hiding this comment

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

Add sample output.


- Measurement
- burrow_topic
- burrow_consumer
Copy link
Contributor

Choose a reason for hiding this comment

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

What fields?

## Topics to monitor. Default to monitor all.
#topics = []
## Groups to monitor. Default to monitor all.
#groups = []
Copy link
Contributor

Choose a reason for hiding this comment

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

Need SSL/TLS support options

@codeb2cc
Copy link
Contributor Author

It's been more than 1 year...
Telegraf and burrow must have changed a lot in this time. I think it's better to merge the newer PR, or at least discuss base on it.

@codeb2cc codeb2cc closed this Apr 25, 2018
@codeb2cc codeb2cc deleted the burrow-input-plugins branch April 25, 2018 09:41
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 new plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants