Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit ec4e66e

Browse files
committedJan 29, 2024
Add req_header_bytes metric to h2
1 parent 8a5ab3f commit ec4e66e

File tree

6 files changed

+33
-20
lines changed

6 files changed

+33
-20
lines changed
 

‎lib/bandit/http2/adapter.ex

+2-2
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ defmodule Bandit.HTTP2.Adapter do
2727
opts: keyword()
2828
}
2929

30-
def init(stream, method, headers, owner, opts) do
30+
def init(stream, method, headers, owner, metrics, opts) do
3131
content_encoding =
3232
Bandit.Compression.negotiate_content_encoding(
3333
Bandit.Headers.get_header(headers, "accept-encoding"),
@@ -40,7 +40,7 @@ defmodule Bandit.HTTP2.Adapter do
4040
owner_pid: owner,
4141
opts: opts,
4242
content_encoding: content_encoding,
43-
metrics: %{req_header_end_time: Bandit.Telemetry.monotonic_time()}
43+
metrics: Map.put(metrics, :req_header_end_time, Bandit.Telemetry.monotonic_time())
4444
}
4545
end
4646

‎lib/bandit/http2/connection.ex

+3-2
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ defmodule Bandit.HTTP2.Connection do
8181

8282
streams =
8383
with_stream(connection, 1, fn stream ->
84-
Bandit.HTTP2.Stream.deliver_headers(stream, headers, false)
84+
Bandit.HTTP2.Stream.deliver_headers(stream, headers, 0, false)
8585
Bandit.HTTP2.Stream.deliver_data(stream, data, true)
8686
end)
8787

@@ -180,7 +180,8 @@ defmodule Bandit.HTTP2.Connection do
180180
{:ok, headers, recv_hpack_state} ->
181181
streams =
182182
with_stream(connection, frame.stream_id, fn stream ->
183-
Bandit.HTTP2.Stream.deliver_headers(stream, headers, frame.end_stream)
183+
frame_size = byte_size(frame.fragment)
184+
Bandit.HTTP2.Stream.deliver_headers(stream, headers, frame_size, frame.end_stream)
184185
end)
185186

186187
%{connection | recv_hpack_state: recv_hpack_state, streams: streams}

‎lib/bandit/http2/stream.ex

+18-13
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,12 @@ defmodule Bandit.HTTP2.Stream do
8787
# These functions are intended to be called by the connection process which contains this
8888
# stream. All of these start with `deliver_`
8989

90-
@spec deliver_headers(stream_handle(), Plug.Conn.headers(), boolean()) :: term()
91-
def deliver_headers(:closed, _headers, _end_stream), do: :ok
92-
def deliver_headers(pid, headers, end_stream), do: send(pid, {:headers, headers, end_stream})
90+
@spec deliver_headers(stream_handle(), Plug.Conn.headers(), non_neg_integer(), boolean()) ::
91+
term()
92+
def deliver_headers(:closed, _headers, _frame_size, _end_stream), do: :ok
93+
94+
def deliver_headers(pid, headers, frame_size, end_stream),
95+
do: send(pid, {:headers, headers, frame_size, end_stream})
9396

9497
@spec deliver_data(stream_handle(), iodata(), boolean()) :: term()
9598
def deliver_data(:closed, _data, _end_stream), do: :ok
@@ -107,7 +110,7 @@ defmodule Bandit.HTTP2.Stream do
107110

108111
def read_headers(%__MODULE__{state: :idle} = stream) do
109112
case do_recv(stream, stream.read_timeout) do
110-
{:headers, headers, stream} ->
113+
{:headers, headers, frame_size, stream} ->
111114
method = Bandit.Headers.get_header(headers, ":method")
112115
request_target = build_request_target!(headers)
113116

@@ -123,7 +126,7 @@ defmodule Bandit.HTTP2.Stream do
123126
content_length = get_content_length!(headers)
124127
headers = combine_cookie_crumbs(headers)
125128
stream = %{stream | bytes_remaining: content_length}
126-
{:ok, method, request_target, headers, stream}
129+
{:ok, method, request_target, headers, %{req_header_bytes: frame_size}, stream}
127130
rescue
128131
exception ->
129132
reraise %{exception | method: method, request_target: request_target}, __STACKTRACE__
@@ -244,7 +247,7 @@ defmodule Bandit.HTTP2.Stream do
244247
defp do_read_data(%__MODULE__{state: state} = stream, max_bytes, timeout, acc)
245248
when state in [:open, :local_closed] do
246249
case do_recv(stream, timeout) do
247-
{:headers, trailers, stream} ->
250+
{:headers, trailers, _frame_size, stream} ->
248251
no_pseudo_headers!(trailers)
249252
Logger.warning("Ignoring trailers #{inspect(trailers)}")
250253
do_read_data(stream, max_bytes, timeout, acc)
@@ -278,8 +281,9 @@ defmodule Bandit.HTTP2.Stream do
278281

279282
defp do_recv(%__MODULE__{state: :idle} = stream, timeout) do
280283
receive do
281-
{:headers, headers, end_stream} ->
282-
{:headers, headers, stream |> do_recv_headers() |> do_recv_end_stream(end_stream)}
284+
{:headers, headers, frame_size, end_stream} ->
285+
{:headers, headers, frame_size,
286+
stream |> do_recv_headers() |> do_recv_end_stream(end_stream)}
283287

284288
{:data, _data, _end_stream} ->
285289
connection_error!("Received DATA in idle state")
@@ -297,8 +301,9 @@ defmodule Bandit.HTTP2.Stream do
297301
defp do_recv(%__MODULE__{state: state} = stream, timeout)
298302
when state in [:open, :local_closed] do
299303
receive do
300-
{:headers, headers, end_stream} ->
301-
{:headers, headers, stream |> do_recv_headers() |> do_recv_end_stream(end_stream)}
304+
{:headers, headers, frame_size, end_stream} ->
305+
{:headers, headers, frame_size,
306+
stream |> do_recv_headers() |> do_recv_end_stream(end_stream)}
302307

303308
{:data, data, end_stream} ->
304309
{:data, data, stream |> do_recv_data(data) |> do_recv_end_stream(end_stream)}
@@ -315,7 +320,7 @@ defmodule Bandit.HTTP2.Stream do
315320

316321
defp do_recv(%__MODULE__{state: :remote_closed} = stream, timeout) do
317322
receive do
318-
{:headers, _headers, _end_stream} ->
323+
{:headers, _headers, _frame_size, _end_stream} ->
319324
do_stream_closed_error!("Received HEADERS in remote_closed state")
320325

321326
{:data, _data, _end_stream} ->
@@ -333,7 +338,7 @@ defmodule Bandit.HTTP2.Stream do
333338

334339
defp do_recv(%__MODULE__{state: :closed} = stream, timeout) do
335340
receive do
336-
{:headers, _headers, _end_stream} -> stream
341+
{:headers, _headers, _frame_size, _end_stream} -> stream
337342
{:data, _data, _end_stream} -> stream
338343
{:send_window_update, _delta} -> stream
339344
{:rst_stream, _error_code} -> stream
@@ -470,7 +475,7 @@ defmodule Bandit.HTTP2.Stream do
470475

471476
def ensure_completed(%__MODULE__{state: :local_closed} = stream) do
472477
receive do
473-
{:headers, _headers, true} -> do_recv_end_stream(stream, true)
478+
{:headers, _headers, _frame_size, true} -> do_recv_end_stream(stream, true)
474479
{:data, _data, true} -> do_recv_end_stream(stream, true)
475480
after
476481
# RFC9113§8.1 - hint the client to stop sending data

‎lib/bandit/http2/stream_process.ex

+2-2
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,12 @@ defmodule Bandit.HTTP2.StreamProcess do
3434

3535
@impl GenServer
3636
def handle_continue(:start_stream, state) do
37-
{:ok, method, request_target, headers, stream} =
37+
{:ok, method, request_target, headers, header_metrics, stream} =
3838
Bandit.HTTP2.Stream.read_headers(state.stream)
3939

4040
adapter =
4141
{Bandit.HTTP2.Adapter,
42-
Bandit.HTTP2.Adapter.init(stream, method, headers, self(), state.opts)}
42+
Bandit.HTTP2.Adapter.init(stream, method, headers, self(), header_metrics, state.opts)}
4343

4444
transport_info = state.stream.transport_info
4545

‎lib/bandit/telemetry.ex

+2-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ defmodule Bandit.Telemetry do
3838
* `req_line_bytes`: The length of the request line, in octets. Includes all line breaks.
3939
Not included for HTTP/2 requests
4040
* `req_header_bytes`: The length of the request headers, in octets. Includes all line
41-
breaks. Not included for HTTP/2 requests
41+
breaks. For HTTP/2 requests, this is the number of octets in the compressed version of the
42+
headers (ie: as sent on the wire)
4243
* `req_body_bytes`: The length of the request body, in octets
4344
* `resp_start_time`: The time that the response started, in `:native` units
4445
* `resp_end_time`: The time that the response completed, in `:native` units

‎test/bandit/http2/plug_test.exs

+6
Original file line numberDiff line numberDiff line change
@@ -685,6 +685,7 @@ defmodule HTTP2PlugTest do
685685
%{
686686
monotonic_time: integer(),
687687
duration: integer(),
688+
req_header_bytes: 43,
688689
req_header_end_time: integer(),
689690
resp_body_bytes: 0,
690691
resp_start_time: integer(),
@@ -715,6 +716,7 @@ defmodule HTTP2PlugTest do
715716
%{
716717
monotonic_time: integer(),
717718
duration: integer(),
719+
req_header_bytes: 47,
718720
req_header_end_time: integer(),
719721
req_body_start_time: integer(),
720722
req_body_end_time: integer(),
@@ -752,6 +754,7 @@ defmodule HTTP2PlugTest do
752754
%{
753755
monotonic_time: integer(),
754756
duration: integer(),
757+
req_header_bytes: 47,
755758
req_header_end_time: integer(),
756759
req_body_start_time: integer(),
757760
req_body_end_time: integer(),
@@ -789,6 +792,7 @@ defmodule HTTP2PlugTest do
789792
%{
790793
monotonic_time: integer(),
791794
duration: integer(),
795+
req_header_bytes: 54,
792796
req_header_end_time: integer(),
793797
req_body_start_time: integer(),
794798
req_body_end_time: integer(),
@@ -823,6 +827,7 @@ defmodule HTTP2PlugTest do
823827
%{
824828
monotonic_time: integer(),
825829
duration: integer(),
830+
req_header_bytes: 45,
826831
req_header_end_time: integer(),
827832
resp_body_bytes: 4,
828833
resp_start_time: integer(),
@@ -852,6 +857,7 @@ defmodule HTTP2PlugTest do
852857
%{
853858
monotonic_time: integer(),
854859
duration: integer(),
860+
req_header_bytes: 49,
855861
req_header_end_time: integer(),
856862
resp_body_bytes: 6,
857863
resp_start_time: integer(),

0 commit comments

Comments
 (0)
Please sign in to comment.