Feature/ping response code #45

kinok · 2016-11-15T17:27:32Z

Hey guys,

We had a problem regarding the hardcoded 204 No Content of /ping response code.
Indeed, Google Cloud Plateform http health-checks only accept 200 OK.
With this PR, you'll be able to control what response code of /ping you want/need to have, from the config file, using default-ping-response.

Thank you guys for this great tool!

Now you can decide what type of http status code the ping will return.

joelegasse

Just a couple smaller changes I'd recommend

joelegasse · 2016-11-15T17:38:47Z

relay/http.go

+ addr string
+ name string
+ schema string
+ response int


I would rather see a name like pingResponseCode and have it separated by a newline so that it's visually separate from the above group of related fields.

joelegasse · 2016-11-15T17:39:20Z

relay/http.go

@@ -113,10 +116,15 @@ func (h *HTTP) Stop() error {
 func (h *HTTP) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 start := time.Now()

+ pingStatusResponse := DefaultHttpPingResponse


This whole block should only happen once, and not on every request.

joelegasse · 2016-11-15T17:44:39Z

relay/http.go

- w.Header().Add("X-InfluxDB-Version", "relay")
- w.WriteHeader(http.StatusNoContent)
- return
+ w.Header().Add("X-InfluxDB-Version", "relay")


If this is going to return codes other than 204, Content-Length should also be set to 0.

kinok · 2016-11-16T09:10:28Z

Hey @joelegasse, thanks for your relevant review. I agree with all your remarks, just made the changes.

kinok · 2017-03-08T17:28:12Z

Hey there,
Any chance to have a review of this PR?
Thank you guys

roman-vynar · 2017-09-07T11:15:30Z

Also Amazon ELB requires 200 and does not work with 204.

roman-vynar · 2017-09-07T11:58:43Z

Getting an error with this PR:

goroutine 1 [running]:
github.com/influxdata/influxdb-relay/relay.NewHTTP(0xc420016761, 0xb, 0xc420014e21, 0xe, 0xc4200167e5, 0x0, 0x0, 0x0, 0xc8, 0xc42009c2c0, ...)
	/go/src/github.com/influxdata/influxdb-relay/relay/http.go:61 +0xdf
github.com/influxdata/influxdb-relay/relay.New(0xc4200561e0, 0x1, 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
	/go/src/github.com/influxdata/influxdb-relay/relay/relay.go:18 +0x15b
main.main()
	/go/src/github.com/influxdata/influxdb-relay/main.go:31 +0x1a1
panic: assignment to entry in nil map

Fix:

-       h := new(HTTP)
-
-       h.addr = cfg.Addr
-       h.name = cfg.Name
+       h := &HTTP{
+               addr:                cfg.Addr,
+               name:                cfg.Name,
+               pingResponseHeaders: map[string]string{},
+       }

PatrickSplice · 2020-10-26T17:00:52Z

Soooo...this is still an issue. Google gcp still only accepts 200 response codes. This has been open since 2016 and never fixed? It has been adjusted in both influxdb and telegraf. See this issue: https://github.com/influxdata/telegraf/pull/5704/files

kinok added 3 commits November 15, 2016 18:14

Move ping response type to config

6ed47a7

Now you can decide what type of http status code the ping will return.

Fix indent

6aa5bb0

Add docs & example

402ce3e

joelegasse reviewed Nov 15, 2016

View reviewed changes

Added content-length header & reword a bit

3409c36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/ping response code #45

Feature/ping response code #45

kinok commented Nov 15, 2016

joelegasse left a comment

joelegasse Nov 15, 2016

joelegasse Nov 15, 2016

joelegasse Nov 15, 2016

kinok commented Nov 16, 2016 •

edited

Loading

kinok commented Mar 8, 2017

roman-vynar commented Sep 7, 2017 •

edited

Loading

roman-vynar commented Sep 7, 2017

PatrickSplice commented Oct 26, 2020

Feature/ping response code #45

Are you sure you want to change the base?

Feature/ping response code #45

Conversation

kinok commented Nov 15, 2016

joelegasse left a comment

Choose a reason for hiding this comment

joelegasse Nov 15, 2016

Choose a reason for hiding this comment

joelegasse Nov 15, 2016

Choose a reason for hiding this comment

joelegasse Nov 15, 2016

Choose a reason for hiding this comment

kinok commented Nov 16, 2016 • edited Loading

kinok commented Mar 8, 2017

roman-vynar commented Sep 7, 2017 • edited Loading

roman-vynar commented Sep 7, 2017

PatrickSplice commented Oct 26, 2020

kinok commented Nov 16, 2016 •

edited

Loading

roman-vynar commented Sep 7, 2017 •

edited

Loading