[Serve] Expose serve request id from http request/resp #35789

sihanwang41 · 2023-05-25T19:14:55Z

Why are these changes needed?

User can inject request id:

@serve.deployment
class Model:
    def __call__(self) -> int:
        return 1

serve.run(Model.bind())
resp = requests.get("http://localhost:8000", headers={"RAY_SERVE_REQUEST_ID": "123-234"})

Related issue number

Closes: #35785

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://docs.ray.io/en/master/.
- I've added any new APIs to the API Reference. For example, if I added a
  method in Tune, I've added it in doc/source/tune/api/ under the
  corresponding .rst file.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested :(

edoakes · 2023-05-25T21:07:40Z

cc @shayanhoshyari

edoakes · 2023-05-25T21:08:05Z

python/ray/serve/_private/constants.py

@@ -191,3 +191,6 @@ class ServeHandleType(str, Enum):

 # Serve HTTP request header key for routing requests.
 SERVE_MULTIPLEXED_MODEL_ID = "serve_multiplexed_model_id"


oh, should this one be all-caps as well?

I think headers are typically case-insensitive, but might as well be consistent...

hmm, good point, I can push a fix in the master (since headers are case insensitive, I will not do cherry-pick in this case)

python/ray/serve/_private/constants.py

edoakes · 2023-05-25T21:09:29Z

python/ray/serve/_private/http_proxy.py

@@ -164,6 +165,12 @@ async def _send_request_to_handle(handle, scope, receive, send) -> str:
        return "500"

    if isinstance(result, (starlette.responses.Response, RawASGIResponse)):
+        if isinstance(result, starlette.responses.Response):


hmm this makes me realize that my PR will complicate this a bit...

I will let your pr checked in and then revisit here.

edoakes · 2023-05-25T21:10:45Z

python/ray/serve/_private/http_util.py

+        # Set request id.
+        self.raw_headers.append(
+            [
+                RAY_SERVE_REQUEST_ID,
+                ray.serve.context._serve_request_context.get().request_id,
+            ]
+        )


Is it possible to do this only in the HTTP proxy instead of modifying the three separate codepaths?

I will rebase after your pr landed. and restructure the code. :)

Thank you 🙏

Signed-off-by: sihanwang41 <sihanwang41@gmail.com>

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

sihanwang41 · 2023-06-09T16:38:42Z

@edoakes i put the request id into the constructor of the Response & RawASGIResponse.

I feel it is more intuitive when constructing the response, we inject the request_id (a little bit weird we change response headers in some other places).
Minimal change, so that I don't need inject request_id manually for other code path (e.g. handling different exceptions, we createResponse object to send back to sender).
WDYT?

edoakes · 2023-06-09T16:45:59Z

@sihanwang41 this approach won't work for wrapped ASGI apps (like fastapi integration) because we just stream the ASGI response messages directly there

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

edoakes

looks reasonable; as discussed offline let's just make it a middleware, should be cleaner

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

edoakes · 2023-06-27T21:01:38Z

python/ray/serve/_private/constants.py

@@ -207,6 +207,10 @@ class ServeHandleType(str, Enum):
    os.environ.get("RAY_SERVE_ENABLE_EXPERIMENTAL_STREAMING", "0") == "1"
 )

+# Request ID used for logging. Can be provided as a request
+# header and will always be returned as a response header.
+RAY_SERVE_REQUEST_ID = "RAY_SERVE_REQUEST_ID"


Suggested change

RAY_SERVE_REQUEST_ID = "RAY_SERVE_REQUEST_ID"

RAY_SERVE_REQUEST_ID_HEADER = "RAY_SERVE_REQUEST_ID"

python/ray/serve/_private/http_proxy.py

edoakes · 2023-06-27T21:03:18Z

python/ray/serve/_private/http_proxy.py

+        async def send_with_request_id(message):
+            if message["type"] == "http.response.start":
+                request_id = ray.serve.context._serve_request_context.get().request_id
+                headers = MutableHeaders(scope=message)


does this update the underlying scope passed in?

Not sure i understand the question, I assume all headers information for http.response.start is under the message directly.
The MutableHeaders will only extract headers from the message https://github.com/encode/starlette/blob/2168e47052239da5df35d5353bb986f760c51cef/starlette/datastructures.py#L534

oh I'm just confused because you're never modifying the message here, does headers.append(RAY_SERVE_REQUEST_ID, request_id) do it?

ah, yes. the headers object hold the message header Dict ref. and whenever you update the headers, it will update the message. It is same as you update the message header directly, but this is more standard instead of updating raw dict by yourself.

perfect thanks for explaining

python/ray/serve/_private/http_proxy.py

python/ray/serve/tests/test_http_headers.py

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

edoakes

Thanks for updating the tests! :)

sihanwang41 · 2023-06-29T19:59:49Z

ping for merge @edoakes , test failure is unrelated.

…5789) User can inject request id: ``` @serve.deployment class Model: def __call__(self) -> int: return 1 serve.run(Model.bind()) resp = requests.get("http://localhost:8000", headers={"RAY_SERVE_REQUEST_ID": "123-234"}) ``` Signed-off-by: e428265 <arvind.chandramouli@lmco.com>

sihanwang41 assigned sihanwang41, GeneDer, edoakes, zcin and shrekris-anyscale May 25, 2023

edoakes reviewed May 25, 2023

View reviewed changes

sihanwang41 added 5 commits June 8, 2023 13:38

[Serve] Expose serve request id from http request/resp

2d8bc37

Signed-off-by: sihanwang41 <sihanwang41@gmail.com>

Update

1591c45

Signed-off-by: sihanwang41 <sihanwang41@gmail.com>

Update

7a26549

Signed-off-by: sihanwang41 <sihanwang41@gmail.com>

Update

43593fe

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Update

6f3de23

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

sihanwang41 force-pushed the request_id_header branch from b1d4037 to 6f3de23 Compare June 8, 2023 22:03

sihanwang41 added 3 commits June 8, 2023 15:04

Add missing file

5b4dad7

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Update

be22390

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Fix

e6ee3ce

sihanwang41 added 8 commits June 27, 2023 08:35

Merge branch 'master' into request_id_header

3243ef9

Update

1e3fb8f

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Update

a2ab1b8

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Update

83874b1

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Merge branch 'master' into request_id_header

d01ddfb

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Update

3d091cd

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Fix

8e4310f

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Update

38f8494

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

edoakes reviewed Jun 27, 2023

View reviewed changes

sihanwang41 added 3 commits June 27, 2023 13:45

Add middleware

10c663c

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

update

10f2848

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Update

2330e28

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

edoakes reviewed Jun 27, 2023

View reviewed changes

sihanwang41 added 7 commits June 27, 2023 14:37

Update

2a22636

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Update

f43b10d

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Update

7377923

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Update

f26fee7

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Merge branch 'master' into request_id_header

ccc7c55

Update

dc6ba44

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Update

205b498

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

edoakes approved these changes Jun 28, 2023

View reviewed changes

edoakes merged commit 8321989 into ray-project:master Jun 29, 2023

akshay-anyscale mentioned this pull request Jul 21, 2023

Add service deployment instructions to stable diffusion template #37645

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Serve] Expose serve request id from http request/resp #35789

[Serve] Expose serve request id from http request/resp #35789

sihanwang41 commented May 25, 2023 •

edited

Loading

edoakes commented May 25, 2023

edoakes May 25, 2023

edoakes May 25, 2023

sihanwang41 May 25, 2023

edoakes May 25, 2023

sihanwang41 May 25, 2023

edoakes May 25, 2023

sihanwang41 May 25, 2023

edoakes May 25, 2023

sihanwang41 commented Jun 9, 2023

edoakes commented Jun 9, 2023

edoakes left a comment

edoakes Jun 27, 2023

edoakes Jun 27, 2023

sihanwang41 Jun 27, 2023

edoakes Jun 27, 2023 •

edited

Loading

sihanwang41 Jun 27, 2023

edoakes Jun 28, 2023

edoakes left a comment

sihanwang41 commented Jun 29, 2023

		@@ -191,3 +191,6 @@ class ServeHandleType(str, Enum):

		# Serve HTTP request header key for routing requests.
		SERVE_MULTIPLEXED_MODEL_ID = "serve_multiplexed_model_id"

	RAY_SERVE_REQUEST_ID = "RAY_SERVE_REQUEST_ID"
	RAY_SERVE_REQUEST_ID_HEADER = "RAY_SERVE_REQUEST_ID"

[Serve] Expose serve request id from http request/resp #35789

[Serve] Expose serve request id from http request/resp #35789

Conversation

sihanwang41 commented May 25, 2023 • edited Loading

Why are these changes needed?

Related issue number

Checks

edoakes commented May 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sihanwang41 commented Jun 9, 2023

edoakes commented Jun 9, 2023

edoakes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edoakes Jun 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edoakes left a comment

Choose a reason for hiding this comment

sihanwang41 commented Jun 29, 2023

sihanwang41 commented May 25, 2023 •

edited

Loading

edoakes Jun 27, 2023 •

edited

Loading