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.

Sign up for GitHub

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

Jump to bottom

Emqtt batch output #4094

Merged
merged 46 commits into from
May 19, 2018
Merged

Emqtt batch output #4094

merged 46 commits into from
May 19, 2018

Conversation

jvrahav
Copy link
Contributor

@jvrahav jvrahav commented May 3, 2018

Required for all PRs:

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

@jvrahav jvrahav mentioned this pull request May 4, 2018
Closed
danielnelson
danielnelson reviewed May 5, 2018
View reviewed changes
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.

In addition to my comments below, can you update this PR to use the new SerializeBatch method that I added in #4107? This method will helps with outputting batches for formats that are not line oriented (currently JSON).

We will need to use the regular Serialize for non-batching mode to preserve backwards compatibility.

Lastly, can you fix the conflict caused by some unrelated changes on master.

etc/telegraf.conf Outdated
@@ -543,6 +543,13 @@
# ## Use SSL but skip chain & host verification
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert this file, I'll generate the telegraf.conf later from the sample configurations.

plugins/outputs/mqtt/mqtt.go Outdated
TopicPrefix string
QoS int `toml:"qos"`
ClientID string `toml:"client_id"`
BatchMessage bool `toml:"batch"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Add documentation for the new option into the SampleConfig(), then run telegraf -usage mqtt to get the output for updating the README.

dmeiners88 and others added 24 commits May 9, 2018 18:24
@dmeiners88 @jvrahav
Ignore UTF8 BOM in JSON parser (influxdata#4099)
ea98635
@danielnelson @jvrahav
Update changelog
468ff6f
@danielnelson @jvrahav
Update gopsutil version
f957ca6
@danielnelson @jvrahav
Clarify max_retry option in kafka output
96146a7
@danielnelson @jvrahav
Fix grammar
7724344
@danielnelson @jvrahav
Remove -i flag from make telegraf
7b14329
@danielnelson @jvrahav
Move usage string to internal to fix go run
7e2f219
@otherpirate @jvrahav
Fix platform not supported error in build.py (influxdata#4102)
8df34fe
@nsteinmetz @jvrahav
Fix name_override example in mysql rreadme (influxdata#4100)
a529da0
@danielnelson @jvrahav
Only lowercase mysql slave metrics with metric_version = 2
7845d10
@danielnelson @jvrahav
Update kafka readme
a863b3e
@danielnelson @jvrahav
Simplify testing with TLS (influxdata#4095)
5bb797e
@danielnelson @jvrahav
Add SerializeBatch method to the Serializer interface (influxdata#4107)
403b515
@danielnelson @jvrahav
Update changelog
6875f34
@mirath @jvrahav
Add topk processor plugin (influxdata#4096)
77ae07b
@danielnelson @jvrahav
Update changelog
3f09c69
@danielnelson @jvrahav
Update issue templates (influxdata#4116)
5f1fafd
@danielnelson @jvrahav
Remove combined issue template
e2e6fe0
@grubernaut @jvrahav
Add cursor metrics to mongodb input (influxdata#4114)
eb15ece
@danielnelson @jvrahav
Update changelog
e5216d3
@danielnelson @jvrahav
Run apt-get update in release.sh
5c9c40f
@danielnelson @jvrahav
Don't report 0ms on timeout in dns_query (influxdata#4118)
9611788
@danielnelson @jvrahav
Update changelog
d43acc1
@danielnelson @jvrahav
Add instructions on how to repair windows performance counters
ad9dfdf
danielnelson and others added 15 commits May 9, 2018 18:24
@danielnelson @jvrahav
Add uint/bool support to cratedb output (influxdata#4117)
9b0e011
@danielnelson @jvrahav
Update changelog
814bf15
@danielnelson @jvrahav
Skip fields that report "not supported" in nvidia-smi (influxdata#4123)
e3cabe3
@morfien101 @jvrahav
Add tag/integer pair for result to net_response (influxdata#3455)
c3bad9d
@danielnelson @jvrahav
Use result and result_code in net_response
9a0fc78
@danielnelson @jvrahav
Update changelog
7ec960e
@danielnelson @jvrahav
Clarify max_retry option in kafka output
ad40c58
@danielnelson @jvrahav
Fix grammar
0b0d466
@danielnelson @jvrahav
Update kafka readme
10128c5
@morfien101 @jvrahav
Add tag/integer pair for result to net_response (influxdata#3455)
b1db50f
@danielnelson @jvrahav
Use result and result_code in net_response
322dfa2
@jvrahav
Merge branch 'master' into emqtt-batch-output
509d5e1
@jvrahav
Modifies emqtt to use SerializeBatch for batch mode of operation
64ad442
@jvrahav
removed serializeBatchMetric as we need to process each metric at a time
150b344
@jvrahav
removed serializeBatchMetric as we need to process each metric at a time
95ac23e
@jvrahav
Copy link
Contributor Author

jvrahav commented May 11, 2018

using serializeBatch here does not help as we need to construct topic names based on each metric before serializing. Hence i believe we can retain serialize method for this plugin

@danielnelson
Copy link
Contributor

There will still be multiple telegraf.Metric going to a single topic though, if we send them together in a batch message we will need to use SerializeBatch in order to have the JSON data encode correctly. For influx and graphite output it will be the same, but for JSON the data shouldn't be line delimited because there are unicode characters that are newlines. Also, you can imagine serialization formats that cannot be delimited with newlines such as yaml/toml that we might want to support in the future.

@danielnelson danielnelson added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label May 14, 2018
@jvrahav
Copy link
Contributor Author

jvrahav commented May 18, 2018

@danielnelson can you review the request. I wasnt able to remove the diff in telegraf conf file. I dont see any difference in my file locally.

@danielnelson danielnelson added this to the 1.7.0 milestone May 19, 2018
@danielnelson danielnelson merged commit 81f5a41 into influxdata:master May 19, 2018
@danielnelson
Copy link
Contributor

Thanks, this will be in 1.7.0.

leodido pushed a commit that referenced this pull request May 22, 2018
@jvrahav @leodido
Add batch mode to mqtt output (#4094)
eb14378
maxunt pushed a commit that referenced this pull request Jun 26, 2018
@jvrahav @danielnelson
Add batch mode to mqtt output (#4094)
cbb4dac
otherpirate pushed a commit to otherpirate/telegraf that referenced this pull request Mar 15, 2019
@jvrahav @otherpirate
Add batch mode to mqtt output (influxdata#4094)
ed3278f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielnelson danielnelson danielnelson left review comments

Assignees
No one assigned
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin
Projects
None yet
Milestone
1.7.0
Development

Successfully merging this pull request may close these issues.
8 participants
@jvrahav @danielnelson @dmeiners88 @otherpirate @nsteinmetz @mirath @grubernaut @morfien101