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.

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

Queued request timeout #1217

Merged
merged 10 commits into from
Mar 13, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions aspects/request_timeout_handler.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package aspects
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved

import (
"github.com/julienschmidt/httprouter"
"github.com/prebid/prebid-server/config"
"net/http"
"strconv"
)

func QueuedRequestTimeout(f httprouter.Handle, custHeaders config.CustomHeaders) httprouter.Handle {

return func(w http.ResponseWriter, r *http.Request, params httprouter.Params) {

reqTimeInQueue := r.Header.Get(custHeaders.RequestTimeInQueue)
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
reqTimeout := r.Header.Get(custHeaders.RequestTimeoutInQueue)

//If request timeout headers are not specified - process request as usual
if reqTimeInQueue == "" || reqTimeout == "" {
f(w, r, params)
return
}

reqTimeFloat, reqTimeFloatErr := strconv.ParseFloat(reqTimeInQueue, 64)
reqTimeoutFloat, reqTimeoutFloatErr := strconv.ParseFloat(reqTimeout, 64)

//Return HTTP 400 if request timeout headers are incorrect (wrong format)
if reqTimeFloatErr != nil || reqTimeoutFloatErr != nil {
w.WriteHeader(http.StatusBadRequest)
return
}

//Return HTTP 408 if requests stays too long in queue
if reqTimeFloat >= reqTimeoutFloat {
w.WriteHeader(http.StatusRequestTimeout)
return
}

f(w, r, params)
}

}
90 changes: 90 additions & 0 deletions aspects/request_timeout_handler_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package aspects

import (
"github.com/julienschmidt/httprouter"
"github.com/prebid/prebid-server/config"
"net/http"
"net/http/httptest"
"testing"

"github.com/stretchr/testify/assert"
)

const reqTimeInQueueHeaderName = "X-Ngx-Request-Time"
const reqTimeoutHeaderName = "X-Ngx-Request-Timeout"

func TestQueuedRequestTimeoutWithTimeout(t *testing.T) {
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved

rw := ExecuteAspectRequest(t, "6", "5", true)

assert.Equal(t, http.StatusRequestTimeout, rw.Code, "Http response code is incorrect, should be 408")
assert.Equal(t, "", string(rw.Body.Bytes()), "Body should not be present in response")

}

func TestQueuedRequestTimeoutNoTimeout(t *testing.T) {

rw := ExecuteAspectRequest(t, "0.9", "5", true)

assert.Equal(t, http.StatusOK, rw.Code, "Http response code is incorrect, should be 200")
assert.Equal(t, "Executed", string(rw.Body.Bytes()), "Body should be present in response")

}

func TestQueuedRequestNoHeaders(t *testing.T) {

rw := ExecuteAspectRequest(t, "", "", false)
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved

assert.Equal(t, http.StatusOK, rw.Code, "Http response code is incorrect, should be 200")
assert.Equal(t, "Executed", string(rw.Body.Bytes()), "Body should be present in response")

}

func TestQueuedRequestAllHeadersIncorrect(t *testing.T) {

rw := ExecuteAspectRequest(t, "test1", "test2", true)

assert.Equal(t, http.StatusBadRequest, rw.Code, "Http response code is incorrect, should be 400")
assert.Equal(t, "", string(rw.Body.Bytes()), "Body should not be present in response")

}

func TestQueuedRequestSomeHeadersIncorrect(t *testing.T) {

rw := ExecuteAspectRequest(t, "test1", "123", true)

assert.Equal(t, http.StatusBadRequest, rw.Code, "Http response code is incorrect, should be 400")
assert.Equal(t, "", string(rw.Body.Bytes()), "Body should not be present in response")

}

func MockEndpoint() httprouter.Handle {
return httprouter.Handle(MockHandler)
}

func MockHandler(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
w.Write([]byte("Executed"))
}

func ExecuteAspectRequest(t *testing.T, timeInQueue string, reqTimeout string, setHeaders bool) *httptest.ResponseRecorder {
rw := httptest.NewRecorder()
req, err := http.NewRequest("POST", "/test", nil)
if err != nil {
assert.Fail(t, "Unable create mock http request")
}
if setHeaders {
req.Header.Set(reqTimeInQueueHeaderName, timeInQueue)
req.Header.Set(reqTimeoutHeaderName, reqTimeout)
}

customHeaders := config.CustomHeaders{reqTimeInQueueHeaderName, reqTimeoutHeaderName}

handler := QueuedRequestTimeout(MockEndpoint(), customHeaders)

r := httprouter.New()
r.POST("/test", handler)

r.ServeHTTP(rw, req)

return rw
}
7 changes: 7 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ type Configuration struct {
AccountRequired bool `mapstructure:"account_required"`
// Local private file containing SSL certificates
PemCertsFile string `mapstructure:"certificates_file"`
// Custom headers set by Nginx to handle request timeout in queue or any other custom headers
Copy link
Contributor

Choose a reason for hiding this comment

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

The 'nginx' part may not be common across hosts. Consider omitting that assumption, something like:

// Custom headers to handle request timeouts from queueing infrastructure

.. but since this is the only feedback I have and it's only a comment, feel free to update this in a future PR. Logic and tests look good to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh sure! Will do, sorry missed this part.

CustomHeaders CustomHeaders `mapstructure:"custom_headers"`
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
}

const MIN_COOKIE_SIZE_BYTES = 500
Expand Down Expand Up @@ -199,6 +201,11 @@ type HostCookie struct {
TTL int64 `mapstructure:"ttl_days"`
}

type CustomHeaders struct {
RequestTimeInQueue string `mapstructure:"request_time_in_queue"`
RequestTimeoutInQueue string `mapstructure:"request_timeout_in_queue"`
}

func (cfg *HostCookie) TTLDuration() time.Duration {
return time.Duration(cfg.TTL) * time.Hour * 24
}
Expand Down
6 changes: 6 additions & 0 deletions router/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/prebid/prebid-server/adapters/rubicon"
"github.com/prebid/prebid-server/adapters/sovrn"
analyticsConf "github.com/prebid/prebid-server/analytics/config"
"github.com/prebid/prebid-server/aspects"
"github.com/prebid/prebid-server/cache"
"github.com/prebid/prebid-server/cache/dummycache"
"github.com/prebid/prebid-server/cache/filecache"
Expand Down Expand Up @@ -255,6 +256,11 @@ func New(cfg *config.Configuration, rateConvertor *currencies.RateConverter) (r
glog.Fatalf("Failed to create the video endpoint handler. %v", err)
}

customHeaders := config.CustomHeaders{}
if cfg.CustomHeaders != customHeaders {
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
videoEndpoint = aspects.QueuedRequestTimeout(videoEndpoint, cfg.CustomHeaders)
}

r.POST("/auction", endpoints.Auction(cfg, syncers, gdprPerms, r.MetricsEngine, dataCache, exchanges))
r.POST("/openrtb2/auction", openrtbEndpoint)
r.POST("/openrtb2/video", videoEndpoint)
Expand Down