Add a OpenTSDB input protocol #870

tcolgate · 2014-08-24T18:19:24Z

This adds an input supporting the telnet put interface from OpenTSDB.
OpenTSDB's tag keys and values and tranformed into Inlflux DB columns.
At present all values are stored as 64 bit floats.

This is based on a mix of the UDP, and Graphite APIs. I've given it a bit of initial testing with tcollector, and so far things looks OK. Not load tested it yet.

I noticed the graphite collector uses a buffered channel, where the UDP one does not. Is the buffering taken care of elsewhere now? TSDB clients will be quite bursty, so buffering may improve things if needed

pauldix · 2014-09-08T19:14:40Z

Thanks @tcolgate! Would it be possible to update the config so that there are different ways to map tag keys?

Having tag keys mapped as columns may end up giving users very poor query performance. It's probably preferable to map the tag keys into the series names. For example:

cpu|host=foo|datacenter=uswest

Or something like that.

tcolgate · 2014-09-10T06:44:24Z

Hi,
Using columns maps to the OpenTSDB usage directly, and has proved to be a really nice way of working with data (You can build up aggregates and queries without needing to resort to regex's, it generally feels more correct).
I will look into it though, but may not be doing it any time soon. The current state of the derivative gunction in influx means that I can't use it at the moment (I'm trying to review the code to work out if I can help fix that), and I've not got much time spare right now.
I don't mean to be negative, Influx is looking fantastic, and working with the code is a real joy, so if I can find the time I will.

pauldix · 2014-09-10T18:43:20Z

Yeah, it might make it better to work with in some ways, but if people are commonly doing queries where the filter out a ton of data by doing "where tag = 'foo'" then the performance is going to be very poor.

Is this something you need for yourself right now? If not, I think I'd rather hold off on this until we can address how to map OpenTSDB onto InfluxDB in a way that provides good performance.

tcolgate · 2014-09-10T19:26:54Z

No urgency, we're back on to OpenTSDB for the moment, and I'll follow influx's progress. I'll have a crack at the alternative mapping at some point, if no one beats me to it.

Dieterbe · 2014-09-10T20:19:32Z

note: it seems we'll get this optimisation in a transparent way when we have #582

toddboom · 2015-02-27T23:53:21Z

@tcolgate The underlying codebase has changed a bunch for the v0.9.0 release. Is there any chance you'd be willing to update this code to work with the new codebase? The new plugins should give you a pretty good idea of how things would need to be restructured.

otoolep · 2015-02-28T07:08:06Z

+1, I'd really like this to see this re-implemented for 0.9. We can certainly offer advice along the way.

tcolgate · 2015-03-28T11:03:10Z

Hi all, sorry, missed the comments. Will look into it.

tcolgate · 2015-03-29T19:54:17Z

@toddboom First stab at updating for the new API, pretty much untested.

otoolep · 2015-03-30T00:57:38Z

cmd/influxd/run.go

+		// Spin up any OpenTSDB servers
+		if config.OpenTSDB.Enabled {
+			o := config.OpenTSDB
+			db := opentsdb.DefaultDatabaseName


We prefer to do this on the config object. In other words, the config object should have a method called "OpenTSDBDatabase()` (or something like that) and encapsulate the default-detection logic.

otoolep · 2015-03-30T01:01:54Z

Thanks @tcolgate -- generally making sense. I do have some feedback, and you'll want to look into adding an integration test in server_integration.go. There are some Graphite examples in there already.

tcolgate · 2015-03-30T07:48:34Z

@otoolep Cheers, will update. FWIW, the graphite and collectd plugins seemed to use different approaches for the settings so it wasn't completely clear which was the preferred option.

tcolgate · 2015-04-01T19:35:02Z

@otoolep hopefully satisfied the review comments.

The test doesn't pass yet, not quite sure why, but will try and fix that up as soon as possible.

otoolep · 2015-04-01T20:29:01Z

cmd/influxd/config.go

+	Enabled         bool   `toml:"enabled"`
+	Database        string `toml:"database"`
+	RetentionPolicy string `toml:"retention-policy"`
+}


You should add a toml directive here, just so it's clear what it is.

Sorry, not sure what you mean here. Each line already has the toml struct tag, and the entry in the Config struct does too (line 97)

Yes, this is what I meant, I see you've got the toml directive in the right place after all -- thanks.

otoolep · 2015-04-01T20:31:29Z

Looking good, I'll wait until the tests are green before I fully review it. Can you squash the commits?

tcolgate · 2015-04-02T06:09:25Z

@otoolep input unit test now passes (will add more later)

tcolgate · 2015-04-02T06:28:00Z

@otoolep config test is in. Server test passes. Commits squashed

otoolep · 2015-04-02T23:20:37Z

Thanks @tcolgate -- have you signed the CLA?

tcolgate · 2015-04-03T07:44:42Z

Done
On 3 Apr 2015 00:20, "otoolep" notifications@github.com wrote:

Thanks @tcolgate https://github.com/tcolgate -- have you signed the CLA?

—
Reply to this email directly or view it on GitHub
#870 (comment).

tcolgate · 2015-04-03T17:23:52Z

I'm going to try and add support for the http interface (TSDB is a bit "special" in this regard, with a http and telnet inteface on the same port), so it presents an interesting challenge.
I'll also add support for batching of the data point writes at some point.

otoolep · 2015-04-03T17:49:02Z

@tcolgate -- I'm not sure I follow you. What do you mean "add support for the HTTP interface"? Do you mean propose a change to our HTTP API for writing data?

tcolgate · 2015-04-03T17:55:03Z

@otoolep nope. TSDB actually provides it's telnet interface and http on the same port.The http interface supports batch upload of data so will map quite nicely to influx internal API. The telnet interface is used by tcollector (the tool I use). The http protocol is used by scollector, a newer tool, and probably other more sane tools out there.
Supporting both would be nice, and users will expect to be able to use the same port for both (it's bonkers, but true).
I've got a rough proof of concept that I should be able to make work.
This wont be on the influxdb endpoint, it'll be served on the same TCP port that the OpenTSDB input currently spins up.
See the feature/opentsdb-http-input branch for my WIP (much tidying to do)

otoolep · 2015-04-03T18:13:28Z

cmd/influxd/config.go

+	return o.Database
+}
+
+func (o OpenTSDB) ListenAddress() string {


This function needs to take the default bind address, and use it if an address is not set for openTSDB. See https://github.com/influxdb/influxdb/blob/master/cmd/influxd/config.go#L390

tcolgate · 2015-04-05T09:19:46Z

@otoolep updated and rebased as request. One point unresolved.

Add an input collector for the OpenTSDB telnet protcol

Add a OpenTSDB input protocol

otoolep · 2015-04-06T18:09:58Z

Thanks @tcolgate for this very useful functionality.

pauldix · 2015-04-06T18:28:02Z

awesome work, thanks @tcolgate!

corylanou · 2015-04-06T18:29:47Z

Super cool @tcolgate thanks for your effort!

toddboom · 2015-04-06T19:39:46Z

👏 💃 👏

tcolgate · 2015-04-07T08:03:18Z

Thanks for merging. I've got some in progress work on support the http interface too.

jvshahid force-pushed the master branch 2 times, most recently from 987efd5 to 5e9750f Compare October 20, 2014 22:37

otoolep reviewed Mar 30, 2015
View reviewed changes

otoolep reviewed Apr 1, 2015
View reviewed changes

otoolep reviewed Apr 3, 2015
View reviewed changes

OpenTSDB server interface

32d6433

Add an input collector for the OpenTSDB telnet protcol

otoolep added a commit that referenced this pull request Apr 6, 2015

Merge pull request #870 from tcolgate/feature/opentsdb-input

7e58f98

Add a OpenTSDB input protocol

otoolep merged commit 7e58f98 into influxdata:master Apr 6, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a OpenTSDB input protocol #870

Add a OpenTSDB input protocol #870

tcolgate commented Aug 24, 2014

pauldix commented Sep 8, 2014

tcolgate commented Sep 10, 2014

pauldix commented Sep 10, 2014

tcolgate commented Sep 10, 2014

Dieterbe commented Sep 10, 2014

toddboom commented Feb 27, 2015

otoolep commented Feb 28, 2015

tcolgate commented Mar 28, 2015

tcolgate commented Mar 29, 2015

otoolep Mar 30, 2015

otoolep commented Mar 30, 2015

tcolgate commented Mar 30, 2015

tcolgate commented Apr 1, 2015

otoolep Apr 1, 2015

tcolgate Apr 2, 2015

otoolep Apr 3, 2015

otoolep commented Apr 1, 2015

tcolgate commented Apr 2, 2015

tcolgate commented Apr 2, 2015

otoolep commented Apr 2, 2015

tcolgate commented Apr 3, 2015

tcolgate commented Apr 3, 2015

otoolep commented Apr 3, 2015

tcolgate commented Apr 3, 2015

otoolep Apr 3, 2015

tcolgate Apr 5, 2015

tcolgate commented Apr 5, 2015

otoolep commented Apr 6, 2015

pauldix commented Apr 6, 2015

corylanou commented Apr 6, 2015

toddboom commented Apr 6, 2015

tcolgate commented Apr 7, 2015

Add a OpenTSDB input protocol #870

Add a OpenTSDB input protocol #870

Conversation

tcolgate commented Aug 24, 2014

pauldix commented Sep 8, 2014

tcolgate commented Sep 10, 2014

pauldix commented Sep 10, 2014

tcolgate commented Sep 10, 2014

Dieterbe commented Sep 10, 2014

toddboom commented Feb 27, 2015

otoolep commented Feb 28, 2015

tcolgate commented Mar 28, 2015

tcolgate commented Mar 29, 2015

otoolep Mar 30, 2015

Choose a reason for hiding this comment

otoolep commented Mar 30, 2015

tcolgate commented Mar 30, 2015

tcolgate commented Apr 1, 2015

otoolep Apr 1, 2015

Choose a reason for hiding this comment

tcolgate Apr 2, 2015

Choose a reason for hiding this comment

otoolep Apr 3, 2015

Choose a reason for hiding this comment

otoolep commented Apr 1, 2015

tcolgate commented Apr 2, 2015

tcolgate commented Apr 2, 2015

otoolep commented Apr 2, 2015

tcolgate commented Apr 3, 2015

tcolgate commented Apr 3, 2015

otoolep commented Apr 3, 2015

tcolgate commented Apr 3, 2015

otoolep Apr 3, 2015

Choose a reason for hiding this comment

tcolgate Apr 5, 2015

Choose a reason for hiding this comment

tcolgate commented Apr 5, 2015

otoolep commented Apr 6, 2015

pauldix commented Apr 6, 2015

corylanou commented Apr 6, 2015

toddboom commented Apr 6, 2015

tcolgate commented Apr 7, 2015