-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Added capability to send metrics through Http API for OpenTSDB #1539
Conversation
@@ -29,21 +31,34 @@ var sampleConfig = ` | |||
prefix = "my.specific.prefix." | |||
|
|||
## Telnet Mode ## |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove "Telnet Mode"
rather than having a use_http option, I would prefer if you integrated it with the name of the host, and then parse out the "scheme" from the url. ie,
|
I will implement the selection of API based on the host and update the PR. Should I go ahead and update Changelog ? |
yes, please do, also don't worry about squashing, I can do that within github |
All done I think. Let me know if anythings is missing. Also, do this have a chance of getting in 1.0 ? |
@sparrc Anything else you need from me ? |
10defc6
to
e5fb1b2
Compare
sorry for the noise, I will kick a rebuild. I don't think there is anything else, I just need to find time to review again and merge, there is a lot on my plate at the moment so please be patient. |
@sparrc No problem and thanks ! |
e5fb1b2
to
173ff3f
Compare
Timestamp int64 | ||
Value string | ||
Tags string | ||
type TagSet map[string]string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did you make this type? just using a map[string]string is more explicit and readable throughout
173ff3f
to
f76acdb
Compare
@sparrc I did what you suggested. |
Thanks @EricFortin, but it looks like your CHANGELOG rebase might have gotten mixed up with other commits. Can you please revert your CHANGELOG change and then rebase, putting your PR into the 1.1 release? |
… batching support.
…t would reuse connection.
…t API to use for sending metrics. Also renamed BatchSize to HttpBatchSize to better convey that it is only used when using Http API.
This reverts commit 24dba55.
3520a34
to
c56454b
Compare
@sparrc Should be good now. Thanks. |
Modified the opentsdb output plugin so it can also use Http API to send metrics. The Http API has better error management and is easier to debug. Should I squash my changes ? First time contributing.
Required for all PRs: