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

Telemetry metadata doesn't have data about response in case of error #638

Closed
fuelen opened this issue Nov 24, 2023 · 1 comment
Closed

Comments

@fuelen
Copy link

fuelen commented Nov 24, 2023

When a middleware returns an error, then metadata.env struct doesn't have status, body, headers etc, while these fields were available in the middleware. I'd expect to have this data in telemetry handler, as it helps in debugging.

Script:

Mix.install([:tesla, :telemetry])

:telemetry.attach_many(
  "tesla.request",
  [[:tesla, :request, :stop]],
  fn _event, measurements, metadata, _ ->
    dbg(metadata)
  end,
  nil
)

defmodule ExpectOKMiddleware do
  @behaviour Tesla.Middleware

  @impl Tesla.Middleware
  def call(env, next, _opts) do
    with {:ok, env} <- Tesla.run(env, next) do
      if dbg(env).status == 200 do
        {:ok, env}
      else
        {:error, {:unexpected_response, "not ok :("}}
      end
    end
  end
end

client = Tesla.client([Tesla.Middleware.Telemetry, {Tesla.Middleware.BaseUrl, "https://hex.pm/api"}, ExpectOKMiddleware])
client |> Tesla.get("/packages/tesla") |> dbg()

output:

[tesla_test.exs:18: ExpectOKMiddleware.call/3]
env #=> %Tesla.Env{
  method: :get,
  url: "https://hex.pm/api/packages/tesla",
  query: [],
  headers: [
    {"cache-control", "max-age=0, private, must-revalidate"},
    {"date", "Fri, 24 Nov 2023 14:41:30 GMT"},
    {"via", "1.1 google"},
    {"server", "Cowboy"},
    {"vary", "accept-encoding"},
    {"content-length", "56"},
    {"content-type", "application/vnd.hex+json; charset=utf-8"},
    {"strict-transport-security", "max-age=31536000"},
    {"x-request-id", "F5qWJEjGVwCUrFsJ5ONB"},
    {"alt-svc", "h3=\":443\"; ma=2592000,h3-29=\":443\"; ma=2592000"}
  ],
  body: "{\"message\":\"User-Agent header is required\",\"status\":400}",
  status: 400,
  opts: [],
  __module__: Tesla,
  __client__: %Tesla.Client{
    fun: nil,
    pre: [
      {Tesla.Middleware.Telemetry, :call, [[]]},
      {Tesla.Middleware.BaseUrl, :call, ["https://hex.pm/api"]},
      {ExpectOKMiddleware, :call, [[]]}
    ],
    post: [],
    adapter: nil
  }
}

[tesla_test.exs:7: (file)]
metadata #=> %{
  env: %Tesla.Env{
    method: :get,
    url: "/packages/tesla",
    query: [],
    headers: [],
    body: nil,
    status: nil,
    opts: [],
    __module__: Tesla,
    __client__: %Tesla.Client{
      fun: nil,
      pre: [
        {Tesla.Middleware.Telemetry, :call, [[]]},
        {Tesla.Middleware.BaseUrl, :call, ["https://hex.pm/api"]},
        {ExpectOKMiddleware, :call, [[]]}
      ],
      post: [],
      adapter: nil
    }
  },
  error: {:unexpected_response, "not ok :("}
}

[tesla_test.exs:28: (file)]
client #=> %Tesla.Client{
  fun: nil,
  pre: [
    {Tesla.Middleware.Telemetry, :call, [[]]},
    {Tesla.Middleware.BaseUrl, :call, ["https://hex.pm/api"]},
    {ExpectOKMiddleware, :call, [[]]}
  ],
  post: [],
  adapter: nil
}
|> Tesla.get("/packages/tesla") #=> {:error, {:unexpected_response, "not ok :("}}

@yordis
Copy link
Member

yordis commented Oct 25, 2024

@fuelen I am updating the documentation, it works as intended, but you must place the telemetry as close as possible to the actual request; the issue is that your ExpectOKMiddleware is transforming the response into an error tuple; so you los the information; here is working test case:

defmodule TelemetryTest do
  use ExUnit.Case

  defmodule ExpectOKMiddleware do
    @behaviour Tesla.Middleware

    @impl Tesla.Middleware
    def call(env, next, _opts) do
      with {:ok, env} <- Tesla.run(env, next) do
        if dbg(env).status == 200 do
          {:ok, env}
        else
          {:error, {:unexpected_response, "not ok :("}}
        end
      end
    end
  end

  test "assering proper telemetry metadata" do
    :telemetry.attach_many(
      "tesla.request",
      [[:tesla, :request, :stop]],
      fn _event, measurements, metadata, _ ->
        send(self(), {:telemetry, metadata})
      end,
      nil
    )

    client =
      Tesla.client([
        {Tesla.Middleware.BaseUrl, "https://hex.pm/api"},
        ExpectOKMiddleware,
        Tesla.Middleware.Telemetry,
      ])

    Tesla.get(client, "/packages/tesla")

    assert_receive {:telemetry, metadata}
    assert metadata.env.status == 400
  end
end

@yordis yordis closed this as completed Oct 25, 2024
yordis added a commit that referenced this issue Oct 25, 2024
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

No branches or pull requests

2 participants