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

feat: sse-timing plugin for tracing events #1361

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

jizhuozhi
Copy link
Contributor

@jizhuozhi jizhuozhi commented Oct 2, 2024

Ⅰ. Describe what this PR did

Add sse-timing plugin for tracing timestamp that each event received in higress. An alternative implementation of the server-timing protocol (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Server-Timing). It's useful to profile AI backend latency which uses event-stream MIME type.

And implemention an EventStream for common SSE(text/event-stream) parsing.

Ⅱ. Does this pull request fix one issue?

None

Ⅲ. Why don't you add test cases (unit test/integration test)?

I'm sorry I don't know how to add WASM testcase, we could discuss in this PR.

Ⅳ. Describe how to verify it

There is a docker-compose.yml in plugin directory, with a go server backend which provides sse response, up it and then just use broswer to open http://localhost:10000, you could see events in dev-tools.

Ⅴ. Special notes for reviews

@jizhuozhi jizhuozhi requested a review from 007gzs as a code owner October 2, 2024 11:02
@codecov-commenter
Copy link

codecov-commenter commented Oct 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 44.12%. Comparing base (ef31e09) to head (527970b).
Report is 125 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1361      +/-   ##
==========================================
+ Coverage   35.91%   44.12%   +8.21%     
==========================================
  Files          69       76       +7     
  Lines       11576     9895    -1681     
==========================================
+ Hits         4157     4366     +209     
+ Misses       7104     5192    -1912     
- Partials      315      337      +22     

see 91 files with indirect coverage changes

@johnlanni
Copy link
Collaborator

@jizhuozhi Please fix the CI.

@johnlanni
Copy link
Collaborator

cc @007gzs

@007gzs
Copy link
Collaborator

007gzs commented Oct 6, 2024

@jizhuozhi
Copy link
Contributor Author

Will, I have seen the test-suits in https://github.com/alibaba/higress/tree/main/test/e2e/conformance/tests, it seems just supporting static response assertion as server-timing is streaming injecting dynamic metainfo into response body, and no sse server as upstream (I just found echo-server and echo-body in deployments). Does any solution or example here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants