diff --git a/lib/logflare/backends/adaptor/bigquery_adaptor.ex b/lib/logflare/backends/adaptor/bigquery_adaptor.ex index ae2cf6f64f..5e1df3b7ff 100644 --- a/lib/logflare/backends/adaptor/bigquery_adaptor.ex +++ b/lib/logflare/backends/adaptor/bigquery_adaptor.ex @@ -42,7 +42,6 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptor do @managed_service_account_partition_count 5 @service_account_prefix "logflare-managed" - @reservation_error_regex ~r/reservation/i @search_query_timeout_ms 60_000 @impl Logflare.Backends.Adaptor @@ -716,8 +715,6 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptor do bigquery_project_id: project_id ) - maybe_warn_reservation_error(query_error, user, project_id, query_opts) - {:error, query_error} end end @@ -799,42 +796,4 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptor do parameterValue: %Value{value: param} } end - - @spec maybe_warn_reservation_error( - error :: QueryError.t(), - user :: User.t(), - project_id :: String.t(), - query_opts :: Keyword.t() - ) :: :ok - defp maybe_warn_reservation_error( - %QueryError{raw_error: error}, - %User{} = user, - project_id, - query_opts - ) do - with true <- reservation_error?(error), - false <- caller_logs_own_errors?(query_opts) do - Logger.warning("Possible BigQuery reservation error", - user_id: user.id, - project_id: project_id, - reservation: Keyword.get(query_opts, :reservation), - query_type: Keyword.get(query_opts, :query_type), - bq_error_message: error["message"] - ) - end - - :ok - end - - @spec caller_logs_own_errors?(query_opts :: Keyword.t()) :: boolean() - defp caller_logs_own_errors?(query_opts) do - Keyword.get(query_opts, :query_type) == :alerts - end - - @spec reservation_error?(error :: any()) :: boolean() - def reservation_error?(%{"message" => msg}) when is_non_empty_binary(msg) do - Regex.match?(@reservation_error_regex, msg) - end - - def reservation_error?(_), do: false end diff --git a/test/logflare/backends/adaptor/bigquery_adaptor_test.exs b/test/logflare/backends/adaptor/bigquery_adaptor_test.exs index 7f5332a780..5ab2524a1b 100644 --- a/test/logflare/backends/adaptor/bigquery_adaptor_test.exs +++ b/test/logflare/backends/adaptor/bigquery_adaptor_test.exs @@ -3,7 +3,6 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptorTest do use ExUnitProperties import Ecto.Query - import ExUnit.CaptureLog alias GoogleApi.BigQuery.V2.Api.Jobs, as: BqJobs alias Logflare.Backends.Backend @@ -441,112 +440,4 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptorTest do assert_received {:timeouts, 60_000, 60_000} end end - - describe "reservation error logging" do - setup do - insert(:plan, name: "Free", type: "standard") - user = insert(:user, bigquery_dataset_id: "test_dataset") - [user: user] - end - - test "logs a warning for a reservation-not-found error", %{user: user} do - body = - ~s|{"error":{"message":"User specified reservation projects/p/locations/l/reservations/missing is not found","status":"NOT_FOUND"}}| - - stub(BqJobs, :bigquery_jobs_query, fn _conn, _proj, _opts -> - {:error, %Tesla.Env{status: 404, body: body}} - end) - - log = - capture_log([level: :warning], fn -> - BigQueryAdaptor.execute_query( - {"test-project", user.bigquery_dataset_id, user.id}, - {"select 1", []}, - reservation: "projects/p/locations/l/reservations/missing" - ) - end) - - assert log =~ "Possible BigQuery reservation error" - end - - test "logs a warning for a permission-denied reservation error", %{user: user} do - body = - ~s|{"error":{"message":"Access Denied: Reservation projects/p/locations/l/reservations/r: Permission bigquery.reservations.use denied on reservation projects/p/locations/l/reservations/r (or it may not exist)","status":"PERMISSION_DENIED"}}| - - stub(BqJobs, :bigquery_jobs_query, fn _conn, _proj, _opts -> - {:error, %Tesla.Env{status: 403, body: body}} - end) - - log = - capture_log([level: :warning], fn -> - BigQueryAdaptor.execute_query( - {"test-project", user.bigquery_dataset_id, user.id}, - {"select 1", []}, - [] - ) - end) - - assert log =~ "Possible BigQuery reservation error" - end - - test "logs a warning for a slot/region reservation error", %{user: user} do - body = - ~s|{"error":{"message":"Cannot run query: project does not have the reservation in the data region or no slots are configured"}}| - - stub(BqJobs, :bigquery_jobs_query, fn _conn, _proj, _opts -> - {:error, %Tesla.Env{status: 400, body: body}} - end) - - log = - capture_log([level: :warning], fn -> - BigQueryAdaptor.execute_query( - {"test-project", user.bigquery_dataset_id, user.id}, - {"select 1", []}, - [] - ) - end) - - assert log =~ "Possible BigQuery reservation error" - end - - test "does not log centrally for alerts queries, which log their own errors", %{user: user} do - body = - ~s|{"error":{"message":"User specified reservation projects/p/locations/l/reservations/missing is not found","status":"NOT_FOUND"}}| - - stub(BqJobs, :bigquery_jobs_query, fn _conn, _proj, _opts -> - {:error, %Tesla.Env{status: 404, body: body}} - end) - - log = - capture_log([level: :warning], fn -> - BigQueryAdaptor.execute_query( - {"test-project", user.bigquery_dataset_id, user.id}, - {"select 1", []}, - query_type: :alerts - ) - end) - - refute log =~ "Possible BigQuery reservation error" - end - - test "does not log a warning for unrelated BigQuery errors", %{user: user} do - body = - ~s|{"error":{"message":"Table test-project:test_dataset.foo not found","status":"NOT_FOUND"}}| - - stub(BqJobs, :bigquery_jobs_query, fn _conn, _proj, _opts -> - {:error, %Tesla.Env{status: 404, body: body}} - end) - - log = - capture_log([level: :warning], fn -> - BigQueryAdaptor.execute_query( - {"test-project", user.bigquery_dataset_id, user.id}, - {"select 1", []}, - [] - ) - end) - - refute log =~ "Possible BigQuery reservation error" - end - end end diff --git a/test/logflare/backends/query_error_test.exs b/test/logflare/backends/query_error_test.exs index 71adb1800c..b7ee279e3e 100644 --- a/test/logflare/backends/query_error_test.exs +++ b/test/logflare/backends/query_error_test.exs @@ -78,6 +78,72 @@ defmodule Logflare.Backends.QueryErrorTest do assert log =~ "timeout" refute log =~ "user_id=" end + + test "logs BigQuery reservation backend errors with raw backend detail" do + for {message, expected_detail} <- [ + { + "User specified reservation projects/p/locations/l/reservations/missing is not found", + "projects/p/locations/l/reservations/missing" + }, + { + "Access Denied: Reservation projects/p/locations/l/reservations/r: Permission bigquery.reservations.use denied on reservation projects/p/locations/l/reservations/r (or it may not exist)", + "bigquery.reservations.use denied" + }, + { + "Cannot run query: project does not have the reservation in the data region or no slots are configured", + "no slots are configured" + } + ] do + error = + query_error( + kind: :backend_error, + raw_error: %{ + "message" => message, + "status" => "FAILED_PRECONDITION" + } + ) + + log = + capture_log( + [ + level: :error, + metadata: [:user_id, :bigquery_project_id, :backend, :error_kind, :error_string] + ], + fn -> + assert ^error = + QueryError.log(error, + user_id: 123, + bigquery_project_id: "test-project" + ) + end + ) + + assert log =~ "Backend query error" + assert log =~ "user_id=123" + assert log =~ "bigquery_project_id=test-project" + assert log =~ "backend=Logflare.Backends.Adaptor.BigQueryAdaptor" + assert log =~ "error_kind=backend_error" + assert log =~ expected_detail + refute log =~ "Possible BigQuery reservation error" + end + end + + test "does not log invalid query errors when raw detail just mentions the word reservation" do + error = + query_error( + raw_error: %{ + "message" => + "User specified reservation projects/p/locations/l/reservations/missing is not found" + } + ) + + log = + capture_log([level: :error, metadata: [:error_kind, :error_string]], fn -> + assert ^error = QueryError.log(error) + end) + + assert log == "" + end end defp query_error(attrs) do