Add Support for OpenTSDB HTTP interface #2254

tcolgate · 2015-04-12T12:14:56Z

OpenTSDB support a http and telnet interface on the same port. This
patch adds support for the /api/put endpoint, both single, for both
single and multiple datapoint submissions.

The unit tests currently fail, this appears to be due to a change in the test framework that I'm not understanding.

otoolep · 2015-04-14T01:14:02Z

cmd/influxd/server_integration_test.go

@@ -1905,6 +1905,94 @@ func Test_ServerOpenTSDBIntegration_BadData(t *testing.T) {
 	}
 }

+func Test_ServerOpenTSDBIntegrationHTTPSingle(t *testing.T) {


There is a lot of boilerplate here. Do you think you could combine the tests? If you use a different series for each test, you could craft queries that wouldn't overlap.

Can do. The other test cases I looked at originally stuck to a single test case per-test

tcolgate · 2015-04-16T06:40:27Z

@otoolep the build failure is due to a race somewhere else.
Would you be happy to merge as is? I can tidy up the verbosity of the tests in a seperate pull (and tidy up the Telnet tests at the same time.

toddboom · 2015-04-17T17:51:57Z

@tcolgate we're fixing up a few tests on our end. once that's done, we'll get this merged in (which might require a few small tweaks).

otoolep · 2015-04-18T17:05:03Z

@tcolgate -- we have made some changes to our test suite, such that you'll need to rebase. If you notice, we're now pushing port 0 into test servers, so that they always use a free port. Your latest test will need to do the same.

I am OK with merging this once it passes, but it would be much appreciated if you could then refactor the tests so there is much less boilerplate. I think the openTSDB tests could run entirely within a single test (so could Graphite).

tcolgate · 2015-04-19T08:15:26Z

@otoolep rebased.

tcolgate · 2015-04-19T08:29:09Z

@otoolep I found this trick for a project at work if you are interested... I used it to set test names based on the name of the function being called...

func funcName() string {
  pc, _, _, _ := runtime.Caller(1)
  f := runtime.FuncForPC(pc)
  return f.Name()
}

then

func Test_ServerOpenTSDBIntegrationHTTPMulti(t *testing.T) {
...
testName := funcName() 
...
}

It's a bit easier to use than manually setting a unique name each time.

otoolep · 2015-04-20T19:32:10Z

cmd/influxd/server_integration_test.go

+	now := time.Now().UTC().Round(time.Second)
+	c, _ := main.NewTestConfig()
+	o := main.OpenTSDB{
+		Port:            4245,


You now need to pass in port 0 to your tests. Please see other tests on why we do this.

otoolep · 2015-04-20T19:32:30Z

Can you please re-base again?

tcolgate · 2015-04-23T18:07:08Z

@otoolep Updated and rebased

tcolgate · 2015-04-29T14:53:04Z

@otoolep rebased again

otoolep · 2015-04-30T18:25:05Z

This branch is continually failing CI. One failure I see is the following:

2015/04/30 18:17:02 openTSDB TCP listener closed
--- FAIL: Test_ServerOpenTSDBIntegrationHTTPSingle (1.54s)
    server_integration_test.go:2106: OpenTSDB Connection String: :0
    server_integration_test.go:88: Creating cluster of 1 nodes for test ServerOpenTSDBIntegrationHTTPSingle
    server_integration_test.go:97: Test ServerOpenTSDBIntegrationHTTPSingle: using tmp directory "/tmp/influxdb-761222188/broker-integration-test" for brokers
    server_integration_test.go:98: Test ServerOpenTSDBIntegrationHTTPSingle: using tmp directory "/tmp/influxdb-761222188/data-integration-test" for data nodes
    server_integration_test.go:136: http://:56253
    server_integration_test.go:175: Test: ServerOpenTSDBIntegrationHTTPSingle: creating database opentsdb
    server_integration_test.go:182: Creating retention policy raw for database opentsdb
    server_integration_test.go:2120: Post http://[::]:50865/api/put: dial tcp [::]:50865: network is unreachable
=== RUN Test_ServerOpenTSDBIntegrationHTTPMulti

tcolgate · 2015-05-01T06:43:13Z

Yes, it works locally. I suspect it's something to do with something using ipv6. Not quite sure how best to fix it, I can try forcing it to v4, but it would be nice if v6 worked.

tcolgate · 2015-05-01T07:04:39Z

@otoolep I wasn't specifying the address in the HTTP tests (TCP already specified 127.0.0.1), I suspect a plain "localhost" would do too. Some other test failed after that but doesn't seem related to the TSDB support

OpenTSDB support a http and telnet interface on the same port. This patch adds support for the /api/put endpoint, both single, for both single and multiple datapoint submissions.

tcolgate · 2015-05-01T14:37:21Z

@otoolep Large number of failrues in the tests, but I can't see how they can be related to my changes.

otoolep · 2015-05-01T17:09:25Z

Thanks @tcolgate. I kicked it again, and it passed. We are actively investigating the build failures, so I'd prefer to hold off merging this. The failure rate of this branch is quite high, so I want to be sure we didn't introduce another race.

otoolep · 2015-05-01T17:09:52Z

https://circleci.com/gh/influxdb/influxdb/tree/pull%2F2254

tcolgate · 2015-05-01T17:22:06Z

Unfortunately I've pretty much run out of time on this. Im not in a
position to use any of this at the moment. I'll leave the branch there. If
anyone wants to pick it up, go for it. I'll revisit after 0.9 is out and I
can start using it.

On Fri, 1 May 2015 18:09 otoolep notifications@github.com wrote:

https://circleci.com/gh/influxdb/influxdb/tree/pull%2F2254

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

otoolep · 2015-05-08T23:51:09Z

Thanks @tcolgate -- we are now merging this.

OK'd by @toddboom

Add Support for OpenTSDB HTTP interface

otoolep reviewed Apr 14, 2015
View reviewed changes

otoolep reviewed Apr 20, 2015
View reviewed changes

Add Support for OpenTSDB HTTP interface

2c68fd2

OpenTSDB support a http and telnet interface on the same port. This patch adds support for the /api/put endpoint, both single, for both single and multiple datapoint submissions.

otoolep added a commit that referenced this pull request May 8, 2015

Merge pull request #2254 from tcolgate/feature/opentsdb-http-input

01c7774

Add Support for OpenTSDB HTTP interface

otoolep merged commit 01c7774 into influxdata:master May 8, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Support for OpenTSDB HTTP interface #2254

Add Support for OpenTSDB HTTP interface #2254

tcolgate commented Apr 12, 2015

otoolep Apr 14, 2015

tcolgate Apr 14, 2015

tcolgate commented Apr 16, 2015

toddboom commented Apr 17, 2015

otoolep commented Apr 18, 2015

tcolgate commented Apr 19, 2015

tcolgate commented Apr 19, 2015

otoolep Apr 20, 2015

otoolep commented Apr 20, 2015

tcolgate commented Apr 23, 2015

tcolgate commented Apr 29, 2015

otoolep commented Apr 30, 2015

tcolgate commented May 1, 2015

tcolgate commented May 1, 2015

tcolgate commented May 1, 2015

otoolep commented May 1, 2015

otoolep commented May 1, 2015

tcolgate commented May 1, 2015

otoolep commented May 8, 2015

Add Support for OpenTSDB HTTP interface #2254

Add Support for OpenTSDB HTTP interface #2254

Conversation

tcolgate commented Apr 12, 2015

otoolep Apr 14, 2015

Choose a reason for hiding this comment

tcolgate Apr 14, 2015

Choose a reason for hiding this comment

tcolgate commented Apr 16, 2015

toddboom commented Apr 17, 2015

otoolep commented Apr 18, 2015

tcolgate commented Apr 19, 2015

tcolgate commented Apr 19, 2015

otoolep Apr 20, 2015

Choose a reason for hiding this comment

otoolep commented Apr 20, 2015

tcolgate commented Apr 23, 2015

tcolgate commented Apr 29, 2015

otoolep commented Apr 30, 2015

tcolgate commented May 1, 2015

tcolgate commented May 1, 2015

tcolgate commented May 1, 2015

otoolep commented May 1, 2015

otoolep commented May 1, 2015

tcolgate commented May 1, 2015

otoolep commented May 8, 2015