8000 Stream input back in echo h2 + h2c curl streaming payload + minimal prometheus metrics (number of fd) by ldemailly · Pull Request #721 · fortio/fortio · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
/ fortio Public

Stream input back in echo h2 + h2c curl streaming payload + minimal prometheus metrics (number of fd) #721

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 19 commits into from
Mar 5, 2023
Merged

Stream input back in echo h2 + h2c curl streaming payload + minimal prometheus metrics (number of fd) #721

merged 19 commits into from
Mar 5, 2023

Conversation

ldemailly
Copy link
Member
@ldemailly ldemailly commented Feb 26, 2023 •
edited
Loading

This is quite a big change/refactor to essentially support Reader/Writer instead of Payload input and byte[] output

Also added h2c for client (based on getting it working on https://github.com/fortio/h2cli first)

Also includes #723 (stats about open file descriptor and number of in flight runs under a new metrics endpoint) separated out for ease of review

Also fixed #305 because it was getting tiring to keep manually changing flags output and version

Also fixed divide by 0 panic when trying to resolve "" host (discovered from adding tests).

curl --limit-rate 1000 --data-binary @README.md -H content-type:text/plain --http2-prior-knowledge "http://localhost:8080"

Server

$ go run . server -loglevel debug
14:26:30 Echo: POST / HTTP/2.0 127.0.0.1:65090 ()  "curl/7.86.0"
14:26:30 Header Host: localhost:8080
14:26:30 Header Content-Length: 51497
14:26:30 Header User-Agent: curl/7.86.0
14:26:30 Header Accept: */*
14:26:30 Header Content-Type: text/plain
14:26:30 D http_utils.go:385> Parsing delay  -> []
14:26:30 D http_server.go:177> Request # 2
14:26:30 D http_server.go:70> FlushWriter wrote 1000
14:26:31 D http_server.go:70> FlushWriter wrote 1000
14:26:32 D http_server.go:70> FlushWriter wrote 1000
14:26:33 D http_server.go:70> FlushWriter wrote 1000
14:26:34 D http_server.go:70> FlushWriter wrote 1000
14:26:35 D http_server.go:70> FlushWriter wrote 1000
14:26:36 D http_server.go:70> FlushWriter wrote 1000
14:26:37 D http_server.go:70> FlushWriter wrote 1000
14:26:38 D http_server.go:70> FlushWriter wrote 1000
[...]

and on curl window you see the readme coming back in chunks with 1s pause (from the rate limit)

in h1.1 mode results would only come back at the end

also now working:

 (echo "foo"; sleep 3; echo "bar") | go run . curl -stream -quiet localhost:8080
foo
# ^ shows right away, then 3s pause and:
bar

ie h2c streaming works from fortio curl too (not just h2cli)

github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 26, 2023
View reviewed changes
@codecov
Copy link
codecov bot commented Feb 26, 2023 •
edited
Loading

Codecov Report

Patch coverage: 88.1% and project coverage change: +0.5 🎉

Comparison is base (28a4496) 89.2% compared to head (a374d83) 89.7%.

Additional details and impacted files 
@@           Coverage Diff            @@
##           master    #721     +/-   ##
========================================
+ Coverage    89.2%   89.7%   +0.5%     
========================================
  Files          16      16             
  Lines        3917    4004     +87     
========================================
+ Hits         3495    3593     +98     
+ Misses        271     262      -9     
+ Partials      151     149      -2
Impacted Files Coverage Δ
fhttp/http_server.go 85.7% <78.3%> (+3.8%) ⬆️
fhttp/http_client.go 85.5% <89.6%> (+0.8%) ⬆️
fhttp/http_utils.go 99.1% <100.0%> (ø)
fhttp/httprunner.go 92.1% <100.0%> (+3.9%) ⬆️
fnet/network.go 90.2% <100.0%> (+0.1%) ⬆️
rapi/restHandler.go 87.4% <100.0%> (+0.2%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ldemailly ldemailly mentioned this pull request Feb 26, 2023
Merged
@ldemailly ldemailly changed the title Stream input back in echo h2 case Stream input back in echo h2 + h2c curl streaming payload Feb 28, 2023
ldemailly added 3 commits March 4, 2023 10:51
@ldemailly
fix divide by 0 bug when host name is empty, add coverage
1774f1a
@ldemailly
linter fix
bc71c9d
@ldemailly
expose scli's num_fd and stats about currently running runs in prom e…
c35451e 
…ndpoint (#723)

* expose scli's num_fd and stats about currently running runs in prom endpoint

\* split away flacky/race test

* check for error in proxy test

* Update codecov to large resource class in config.yml

* scrape more often for tests, simplify prom start
@ldemailly ldemailly changed the title Stream input back in echo h2 + h2c curl streaming payload Stream input back in echo h2 + h2c curl streaming payload + minimal prometheus metrics (number of fd) Mar 4, 2023
@ldemailly ldemailly mentioned this pull request Mar 5, 2023
Merged
@ldemailly ldemailly merged commit 49f6e72 into master Mar 5, 2023
@ldemailly ldemailly deleted the h2_echo branch March 5, 2023 02:52
@ldemailly ldemailly mentioned this pull request Mar 9, 2023
Closed
ldemailly added a commit that referenced this pull request Mar 9, 2023
@ldemailly
fix access log status code code and also use the cheaper streaming ap…
47244a0 
…i (with nil/io.discard writer) leftover from #721
ldemailly added a commit that referenced this pull request Mar 9, 2023
@ldemailly
go1.19.7 + codeOK fix + streaming misses (#727)
365da92 
* go1.19.7

* fix access log status code code and also use the cheaper streaming api (with nil/io.discard writer) leftover from #721
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wuhaoyujerry wuhaoyujerry wuhaoyujerry approved these changes

@PrabhdeepsGill PrabhdeepsGill PrabhdeepsGill approved these changes

@wiardvanrij wiardvanrij Awaiting requested review from wiardvanrij

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

auto sync version
3 participants
@ldemailly @wuhaoyujerry @PrabhdeepsGill
0