From 094cc484e4871d964c03e458fde30500f83e559a Mon Sep 17 00:00:00 2001 From: Artur Novak Date: Tue, 14 Nov 2023 18:53:32 +0000 Subject: [PATCH] Add finch instance name to telemetry meta in request --- lib/finch.ex | 4 ++-- lib/finch/http1/conn.ex | 16 +++++++++------- lib/finch/http1/pool.ex | 12 ++++++------ lib/finch/http2/pool.ex | 4 ++-- lib/finch/pool.ex | 17 ++++++++++++++--- lib/finch/telemetry.ex | 11 +++++++++++ test/finch/http2/pool_test.exs | 12 ++++++------ 7 files changed, 50 insertions(+), 26 deletions(-) diff --git a/lib/finch.ex b/lib/finch.ex index 186a320f..82c979ea 100644 --- a/lib/finch.ex +++ b/lib/finch.ex @@ -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 """ @@ -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 """ diff --git a/lib/finch/http1/conn.ex b/lib/finch/http1/conn.ex index ff203b63..b331b918 100644 --- a/lib/finch/http1/conn.ex +++ b/lib/finch/http1/conn.ex @@ -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) @@ -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} diff --git a/lib/finch/http1/pool.ex b/lib/finch/http1/pool.ex index c9541938..b9124283 100644 --- a/lib/finch/http1/pool.ex +++ b/lib/finch/http1/pool.ex @@ -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) @@ -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} -> @@ -77,7 +77,7 @@ 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 = @@ -85,7 +85,7 @@ defmodule Finch.HTTP1.Pool do 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 diff --git a/lib/finch/http2/pool.ex b/lib/finch/http2/pool.ex index 15dd7f2c..d1a94544 100644 --- a/lib/finch/http2/pool.ex +++ b/lib/finch/http2/pool.ex @@ -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) @@ -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) diff --git a/lib/finch/pool.ex b/lib/finch/pool.ex index e9839ffa..315d2d78 100644 --- a/lib/finch/pool.ex +++ b/lib/finch/pool.ex @@ -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 diff --git a/lib/finch/telemetry.ex b/lib/finch/telemetry.ex index 227ec4ba..dd288cfd 100644 --- a/lib/finch/telemetry.ex +++ b/lib/finch/telemetry.ex @@ -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`). @@ -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`). @@ -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. @@ -106,6 +109,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. @@ -120,6 +124,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. @@ -131,6 +136,7 @@ defmodule Finch.Telemetry do #### Measurements + * `:name` - The name of the Finch instance. * `:system_time` - The system time. * `:idle_time` - Elapsed time since the connection was last checked in or initialized. @@ -144,6 +150,7 @@ defmodule Finch.Telemetry do #### Measurements + * `:name` - The name of the Finch instance. * `:duration` - Time taken to make the request. * `:idle_time` - Elapsed time since the connection was last checked in or initialized. @@ -163,6 +170,7 @@ defmodule Finch.Telemetry do #### Metadata + * `:name` - The name of the Finch instance. * `:request` - The request (`Finch.Request`). ### Receive Stop @@ -176,6 +184,7 @@ defmodule Finch.Telemetry do #### Metadata + * `:name` - The name of the Finch instance. * `:request` - The request (`Finch.Request`). * `:status` - The response status (`Mint.Types.status()`). * `:headers` - The response headers (`Mint.Types.headers()`). @@ -192,6 +201,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. @@ -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. diff --git a/test/finch/http2/pool_test.exs b/test/finch/http2/pool_test.exs index 39e9eae9..3196da18 100644 --- a/test/finch/http2/pool_test.exs +++ b/test/finch/http2/pool_test.exs @@ -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)]) @@ -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([ @@ -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 @@ -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)]) @@ -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 @@ -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