Skip to content

Commit

Permalink
Add finch instance name to telemetry meta in HTTP/1 request
Browse files Browse the repository at this point in the history
  • Loading branch information
arthurnovak committed Dec 5, 2023
1 parent 109c3d2 commit d7de556
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 26 deletions.
4 changes: 2 additions & 2 deletions lib/finch.ex
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ defmodule Finch do

defp __stream__(%Request{} = req, name, acc, fun, opts) do
{pool, pool_mod} = get_pool(req, name)
pool_mod.request(pool, req, acc, fun, opts)
pool_mod.request(pool, req, acc, fun, name, opts)
end

@doc """
Expand Down Expand Up @@ -550,7 +550,7 @@ defmodule Finch do
@spec async_request(Request.t(), name(), request_opts()) :: request_ref()
def async_request(%Request{} = req, name, opts \\ []) do
{pool, pool_mod} = get_pool(req, name)
pool_mod.async_request(pool, req, opts)
pool_mod.async_request(pool, req, name, opts)
end

@doc """
Expand Down
16 changes: 9 additions & 7 deletions lib/finch/http1/conn.ex
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,24 @@ defmodule Finch.HTTP1.Conn do
}
end

def connect(%{mint: mint} = conn) when not is_nil(mint) do
def connect(%{mint: mint} = conn, name) when not is_nil(mint) do
meta = %{
scheme: conn.scheme,
host: conn.host,
port: conn.port
port: conn.port,
name: name
}

Telemetry.event(:reused_connection, %{}, meta)
{:ok, conn}
end

def connect(%{mint: nil} = conn) do
def connect(%{mint: nil} = conn, name) do
meta = %{
scheme: conn.scheme,
host: conn.host,
port: conn.port
port: conn.port,
name: name
}

start_time = Telemetry.start(:connect, meta)
Expand Down Expand Up @@ -98,12 +100,12 @@ defmodule Finch.HTTP1.Conn do
end
end

def request(%{mint: nil} = conn, _, _, _, _, _, _), do: {:error, conn, "Could not connect"}
def request(%{mint: nil} = conn, _, _, _, _, _, _, _), do: {:error, conn, "Could not connect"}

def request(conn, req, acc, fun, receive_timeout, request_timeout, idle_time) do
def request(conn, req, acc, fun, name, receive_timeout, request_timeout, idle_time) do
full_path = Finch.Request.request_path(req)

metadata = %{request: req}
metadata = %{request: req, name: name}

extra_measurements = %{idle_time: idle_time}

Expand Down
12 changes: 6 additions & 6 deletions lib/finch/http1/pool.ex
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ defmodule Finch.HTTP1.Pool do
end

@impl Finch.Pool
def request(pool, req, acc, fun, opts) do
def request(pool, req, acc, fun, name, opts) do
pool_timeout = Keyword.get(opts, :pool_timeout, 5_000)
receive_timeout = Keyword.get(opts, :receive_timeout, 15_000)
request_timeout = Keyword.get(opts, :request_timeout, :infinity)

metadata = %{request: req, pool: pool}
metadata = %{request: req, pool: pool, name: name}

start_time = Telemetry.start(:queue, metadata)

Expand All @@ -42,9 +42,9 @@ defmodule Finch.HTTP1.Pool do
fn from, {state, conn, idle_time} ->
Telemetry.stop(:queue, start_time, metadata, %{idle_time: idle_time})

with {:ok, conn} <- Conn.connect(conn),
with {:ok, conn} <- Conn.connect(conn, name),
{:ok, conn, acc} <-
Conn.request(conn, req, acc, fun, receive_timeout, request_timeout, idle_time) do
Conn.request(conn, req, acc, fun, name, receive_timeout, request_timeout, idle_time) do
{{:ok, acc}, transfer_if_open(conn, state, from)}
else
{:error, conn, error} ->
Expand Down Expand Up @@ -77,15 +77,15 @@ defmodule Finch.HTTP1.Pool do
end

@impl Finch.Pool
def async_request(pool, req, opts) do
def async_request(pool, req, name, opts) do
owner = self()

pid =
spawn_link(fn ->
monitor = Process.monitor(owner)
request_ref = {__MODULE__, self()}

case request(pool, req, {owner, monitor, request_ref}, &send_async_response/2, opts) do
case request(pool, req, {owner, monitor, request_ref}, &send_async_response/2, name, opts) do
{:ok, _} -> send(owner, {request_ref, :done})
{:error, error} -> send(owner, {request_ref, {:error, error}})
end
Expand Down
4 changes: 2 additions & 2 deletions lib/finch/http2/pool.ex
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ defmodule Finch.HTTP2.Pool do
# Call the pool with the request. The pool will multiplex multiple requests
# and stream the result set back to the calling process using `send`
@impl Finch.Pool
def request(pool, request, acc, fun, opts) do
def request(pool, request, acc, fun, _name, opts) do
opts = Keyword.put_new(opts, :receive_timeout, @default_receive_timeout)
timeout = opts[:receive_timeout]
request_ref = make_request_ref(pool)
Expand Down Expand Up @@ -58,7 +58,7 @@ defmodule Finch.HTTP2.Pool do
end

@impl Finch.Pool
def async_request(pool, req, opts) do
def async_request(pool, req, _name, opts) do
opts = Keyword.put_new(opts, :receive_timeout, @default_receive_timeout)
request_ref = make_request_ref(pool)

Expand Down
17 changes: 14 additions & 3 deletions lib/finch/pool.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,22 @@ defmodule Finch.Pool do

@type request_ref :: {pool_mod :: module(), cancel_ref :: term()}

@callback request(pid(), Finch.Request.t(), acc, Finch.stream(acc), list()) ::
{:ok, acc} | {:error, term()}
@callback request(
pid(),
Finch.Request.t(),
acc,
Finch.stream(acc),
Finch.name(),
list()
) :: {:ok, acc} | {:error, term()}
when acc: term()

@callback async_request(pid(), Finch.Request.t(), list()) :: request_ref()
@callback async_request(
pid(),
Finch.Request.t(),
Finch.name(),
list()
) :: request_ref()

@callback cancel_async_request(request_ref()) :: :ok

Expand Down
11 changes: 11 additions & 0 deletions lib/finch/telemetry.ex
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ defmodule Finch.Telemetry do
#### Metadata
* `:name` - The name of the Finch instance.
* `:pool` - The pool's PID.
* `:request` - The request (`Finch.Request`).
Expand All @@ -77,6 +78,7 @@ defmodule Finch.Telemetry do
#### Metadata
* `:name` - The name of the Finch instance.
* `:pool` - The pool's PID.
* `:request` - The request (`Finch.Request`).
Expand All @@ -90,6 +92,7 @@ defmodule Finch.Telemetry do
#### Metadata
* `:name` - The name of the Finch instance.
* `:request` - The request (`Finch.Request`).
* `:kind` - The type of exception.
* `:reason` - Error description or error data.
Expand All @@ -106,6 +109,7 @@ defmodule Finch.Telemetry do
#### Metadata
* `:name` - The name of the Finch instance (populated for HTTP/1 only).
* `:scheme` - The scheme used in the connection. either `http` or `https`.
* `:host` - The host address.
* `:port` - The port to connect on.
Expand All @@ -120,6 +124,7 @@ defmodule Finch.Telemetry do
#### Metadata
* `:name` - The name of the Finch instance (populated for HTTP/1 only).
* `:scheme` - The scheme used in the connection. either `http` or `https`.
* `:host` - The host address.
* `:port` - The port to connect on.
Expand All @@ -131,6 +136,7 @@ defmodule Finch.Telemetry do
#### Measurements
* `:name` - The name of the Finch instance (populated for HTTP/1 only).
* `:system_time` - The system time.
* `:idle_time` - Elapsed time since the connection was last checked in or initialized.
Expand All @@ -144,6 +150,7 @@ defmodule Finch.Telemetry do
#### Measurements
* `:name` - The name of the Finch instance (populated for HTTP/1 only).
* `:duration` - Time taken to make the request.
* `:idle_time` - Elapsed time since the connection was last checked in or initialized.
Expand All @@ -163,6 +170,7 @@ defmodule Finch.Telemetry do
#### Metadata
* `:name` - The name of the Finch instance (populated for HTTP/1 only).
* `:request` - The request (`Finch.Request`).
### Receive Stop
Expand All @@ -176,6 +184,7 @@ defmodule Finch.Telemetry do
#### Metadata
* `:name` - The name of the Finch instance (populated for HTTP/1 only).
* `:request` - The request (`Finch.Request`).
* `:status` - The response status (`Mint.Types.status()`).
* `:headers` - The response headers (`Mint.Types.headers()`).
Expand All @@ -192,6 +201,7 @@ defmodule Finch.Telemetry do
#### Metadata
* `:name` - The name of the Finch instance (populated for HTTP/1 only).
* `:request` - The request (`Finch.Request`).
* `:kind` - The type of exception.
* `:reason` - Error description or error data.
Expand All @@ -203,6 +213,7 @@ defmodule Finch.Telemetry do
#### Metadata
* `:name` - The name of the Finch instance.
* `:scheme` - The scheme used in the connection. either `http` or `https`.
* `:host` - The host address.
* `:port` - The port to connect on.
Expand Down
12 changes: 6 additions & 6 deletions test/finch/http2/pool_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ defmodule Finch.HTTP2.PoolTest do
start_pool(port)
end)

ref = Pool.async_request(pool, req, [])
ref = Pool.async_request(pool, req, nil, [])

assert_recv_frames([headers(stream_id: stream_id)])

Expand All @@ -388,7 +388,7 @@ defmodule Finch.HTTP2.PoolTest do
:timer.sleep(10)

# We can't send any more requests since the connection is closed for writing.
ref2 = Pool.async_request(pool, req, [])
ref2 = Pool.async_request(pool, req, nil, [])
assert_receive {^ref2, {:error, %Finch.Error{reason: :read_only}}}

server_send_frames([
Expand All @@ -406,7 +406,7 @@ defmodule Finch.HTTP2.PoolTest do
Process.sleep(50)

# If we try to make a request now that the server shut down, we get an error.
ref3 = Pool.async_request(pool, req, [])
ref3 = Pool.async_request(pool, req, nil, [])
assert_receive {^ref3, {:error, %Finch.Error{reason: :disconnected}}}
end

Expand All @@ -418,7 +418,7 @@ defmodule Finch.HTTP2.PoolTest do
start_pool(port)
end)

ref = Pool.async_request(pool, req, [])
ref = Pool.async_request(pool, req, nil, [])

assert_recv_frames([headers(stream_id: stream_id)])

Expand Down Expand Up @@ -446,7 +446,7 @@ defmodule Finch.HTTP2.PoolTest do
start_pool(port)
end)

ref = Pool.async_request(pool, %{req | headers: [{"foo", "bar"}]}, [])
ref = Pool.async_request(pool, %{req | headers: [{"foo", "bar"}]}, nil, [])

assert_receive {^ref, {:error, %{reason: {:max_header_list_size_exceeded, _, _}}}}
end
Expand Down Expand Up @@ -490,7 +490,7 @@ defmodule Finch.HTTP2.PoolTest do
{:data, value}, {status, headers, body} -> {:cont, {status, headers, body <> value}}
end

Pool.request(pool, req, acc, fun, opts)
Pool.request(pool, req, acc, fun, nil, opts)
end

defp start_server_and_connect_with(opts \\ [], fun) do
Expand Down

0 comments on commit d7de556

Please sign in to comment.