search

openfaas / of-watchdog

Reverse proxy for STDIO and HTTP microservices
MIT License
259 stars 115 forks source link

Provide X-Duration-Seconds in HTTP response #38

Closed rgee0 closed 5 years ago

rgee0 commented 5 years ago

Expected Behaviour

Similar behaviour to the classic watchdog:

HTTP/1.1 200 OK
Server: nginx/1.13.12
Date: Wed, 07 Nov 2018 14:37:08 GMT
Content-Type: text/html
Content-Length: 176
Connection: keep-alive
X-Call-Id: 6afdc9b1-ac15-47fa-ba83-3a546ab9193d
X-Duration-Seconds: 0.047277
X-Start-Time: 1541601428214993189
Strict-Transport-Security: max-age=15724800; includeSubDomains

Current Behaviour

HTTP/2 200 
server: nginx/1.13.12
date: Wed, 07 Nov 2018 14:36:22 GMT
content-type: application/cloudevents+json
content-length: 267
x-call-id: 0e8128ca-d51f-4aa1-b2f2-2fdc1c04f615
x-start-time: 1541601382939613945
strict-transport-security: max-age=15724800; includeSubDomains

Context

Consistent experience across classic and of-watchdog. Greater compatibility for users once they migrate to of-watchdog

alexellis commented 5 years ago

The HTTP mode has been handled via b899713

The other modes are still pending the fix.

alexellis commented 5 years ago

@ivanayov would you like to pick up the fix for the other remaining modes when you come back from the holidays?

Alex

omrishtam commented 5 years ago

Since the afterburn mode is meant to be deprecated, there's no need for the header in there too.

As for the streaming mode (the default) it's said that HTTP headers cannot be sent after function starts executing. So there's no need to even try providing the header there since its value can only be known after function starts executing.

@alexellis already added the header for the HTTP mode, so that leaves only the serializing mode to provide the header in. @omerzamir and I would take the fix for this issue if that's fine with you.

Omri

alexellis commented 5 years ago

SGTM

alexellis commented 5 years ago

Derek close: fixed

alexellis commented 5 years ago

Thank you both.