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

Adds Kafka Plugin #35

Merged
merged 1 commit into from
Jul 2, 2015
Merged

Adds Kafka Plugin #35

merged 1 commit into from
Jul 2, 2015

Conversation

es
Copy link
Contributor

@es es commented Jul 1, 2015

Kafka plugin polls a specified Kafka topic and adds messages to InfluxDB. The plugin assumes messages in the topic are JSON objects with 'measurement', 'tags', 'time', & 'values' keys. Consumer Group is used to talk to the Kafka cluster so multiple instances of telegraf can read from the same topic in parallel.

Running the test requires running Zookeeper & Kafka. The spotify/kafka docker container works pretty well for this.

@evanphx

\cc @marc-barry

@pauldix
Copy link
Member

pauldix commented Jul 1, 2015

Would it be possible to update this to use the line protocol instead? The InfluxDB client supports it and has methods for parsing things out. More on the line protocol here:
https://influxdb.com/docs/v0.9/concepts/reading_and_writing_data.html
influxdata/influxdb#2696

@es
Copy link
Contributor Author

es commented Jul 1, 2015

update this to use the line protocol instead

If I'm calling into the client, would it still make sense to have this as a plugin in telegraf?

@pauldix
Copy link
Member

pauldix commented Jul 1, 2015

Telegraf should still be responsible for posting the metrics to InfluxDB. I was just mentioning the client because it can be used to parse the line protocol. But actually I was wrong, it's in the tsdb package in InfluxDB. Specifically you can use this method: tsdb.ParsePointsWithPrecision

@es
Copy link
Contributor Author

es commented Jul 1, 2015

👍

Any recommendations on the optimal batch size?

@pauldix
Copy link
Member

pauldix commented Jul 1, 2015

@emils best to have it a configuration option in the plugin. Maybe default to a size of 2k points?

@es es force-pushed the plugins/kafka branch 3 times, most recently from 8adc828 to 9fbcdde Compare July 2, 2015 04:08
@es
Copy link
Contributor Author

es commented Jul 2, 2015

Updated to use the line protocol instead of JSON. @pauldix could you please take a second 👀?

I'll make sure to squash the commits once you're happy with the changes.

@pauldix
Copy link
Member

pauldix commented Jul 2, 2015

@emils Looks good. The only last thing I'd do is to rename the plugin from kafka to kafka_consumer. I think that makes more sense because I would expect a kafka plugin to actually pull metrics from a kafka cluster itself (for purposes of instrumenting and monitoring a kafka cluster).

@es es force-pushed the plugins/kafka branch 4 times, most recently from bf49c02 to 009dd53 Compare July 2, 2015 18:40
@es
Copy link
Contributor Author

es commented Jul 2, 2015

Squashed the commits & renamed the plugin to kafka_consumer

zookeeperPeers = ["localhost:2181"]

# Batch size of points sent to InfluxDB
batchSize = 10`
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, one last comment. I think the default batch size should be 1000. InfluxDB performance will be much better that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bumped to 1000

The Kafka consumer plugin polls a specified Kafka topic and adds messages to
InfluxDB. The plugin assumes messages follow the line protocol. Consumer Group
is used to talk to the Kafka cluster so multiple instances of telegraf can read
from the same topic in parallel.
@es es force-pushed the plugins/kafka branch from 009dd53 to 0692b4b Compare July 2, 2015 19:40
@pauldix
Copy link
Member

pauldix commented Jul 2, 2015

Looks great, thanks @emils!

pauldix added a commit that referenced this pull request Jul 2, 2015
@pauldix pauldix merged commit e9ad786 into influxdata:master Jul 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants