Improve logging

Author: louiscb

lib/remote_persistent_term/fetcher/s3.ex

@@ -80,7 +80,7 @@ defmodule RemotePersistentTerm.Fetcher.S3 do
         {:error, "could not find s3://#{state.bucket}/#{state.key}"}
 
       {:error, reason} ->
-        Logger.error("#{__MODULE__} - unknown error: #{inspect(reason)}")
+        Logger.error("#{__MODULE__} - s3://#{state.bucket}/#{state.key} - unknown error: #{inspect(reason)}")
         {:error, "Unknown error"}
     end
   end
@@ -134,29 +134,29 @@ defmodule RemotePersistentTerm.Fetcher.S3 do
   defp aws_client_request(op, %{region: region, failover_regions: nil}),
     do: client().request(op, region: region)
 
-  defp aws_client_request(op, %{region: region, failover_regions: failover_regions})
+  defp aws_client_request(op, %{region: region, bucket: bucket, key: key, failover_regions: failover_regions})
        when is_list(failover_regions) do
     with {:error, reason} <- client().request(op, region: region) do
       Logger.error(
-        "Failed to fetch from primary region #{region}: #{inspect(reason)}, will try failover regions"
+        "s3://#{bucket}/#{key} - Failed to fetch from primary region #{region}: #{inspect(reason)}, will try failover regions"
       )
 
-      try_failover_regions(op, failover_regions)
+      try_failover_regions(op, failover_regions, bucket, key)
     end
   end
 
-  defp try_failover_regions(_op, []), do: {:error, "All regions failed"}
+  defp try_failover_regions(_op, [], _bucket, _key), do: {:error, "All regions failed"}
 
-  defp try_failover_regions(op, [region | remaining_regions]) do
-    Logger.info("Trying failover region: #{region}")
+  defp try_failover_regions(op, [region | remaining_regions], bucket, key) do
+    Logger.info("s3://#{bucket}/#{key} - Trying failover region: #{region}")
 
     case client().request(op, region: region) do
       {:ok, result} ->
         {:ok, result}
 
       {:error, reason} ->
-        Logger.error("Failed to fetch from failover region #{region}: #{inspect(reason)}")
-        try_failover_regions(op, remaining_regions)
+        Logger.error("s3://#{bucket}/#{key} - Failed to fetch from failover region #{region}: #{inspect(reason)}")
+        try_failover_regions(op, remaining_regions, bucket, key)
     end
   end
 

test/remote_persistent_term/fetcher/s3_test.exs

@@ -17,9 +17,9 @@ defmodule RemotePersistentTerm.Fetcher.S3Test do
     end)
 
     assert capture_log(fn ->
-             assert {:error, "Unknown error"} = S3.current_version(%S3{bucket: "bucket"})
+             assert {:error, "Unknown error"} = S3.current_version(%S3{bucket: "bucket", key: "key"})
            end) =~
-             "Elixir.RemotePersistentTerm.Fetcher.S3 - unknown error: :unknown_error"
+             "Elixir.RemotePersistentTerm.Fetcher.S3 - s3://bucket/key - unknown error: :unknown_error"
   end
 
   describe "init/1" do
@@ -67,9 +67,9 @@ defmodule RemotePersistentTerm.Fetcher.S3Test do
           assert {:ok, "current-etag"} = result
         end)
 
-      assert log =~ "Failed to fetch from primary region #{@region}"
+      assert log =~ "s3://#{@bucket}/#{@key} - Failed to fetch from primary region #{@region}"
       assert log =~ "will try failover regions"
-      assert log =~ "Trying failover region: failover-region-1"
+      assert log =~ "s3://#{@bucket}/#{@key} - Trying failover region: failover-region-1"
     end
 
     test "download/1 tries first failover region when primary region fails" do
@@ -97,9 +97,9 @@ defmodule RemotePersistentTerm.Fetcher.S3Test do
           assert {:ok, "content from failover region"} = result
         end)
 
-      assert log =~ "Failed to fetch from primary region #{@region}"
+      assert log =~ "s3://#{@bucket}/#{@key} - Failed to fetch from primary region #{@region}"
       assert log =~ "will try failover regions"
-      assert log =~ "Trying failover region: failover-region-1"
+      assert log =~ "s3://#{@bucket}/#{@key} - Trying failover region: failover-region-1"
     end
 
     test "returns error when primary and all failover regions fail" do
@@ -131,12 +131,12 @@ defmodule RemotePersistentTerm.Fetcher.S3Test do
           assert message =~ "All regions failed"
         end)
 
-      assert log =~ "Failed to fetch from primary region #{@region}"
+      assert log =~ "s3://#{@bucket}/#{@key} - Failed to fetch from primary region #{@region}"
       assert log =~ "will try failover regions"
-      assert log =~ "Trying failover region: failover-region-1"
-      assert log =~ "Failed to fetch from failover region failover-region-1"
-      assert log =~ "Trying failover region: failover-region-2"
-      assert log =~ "Failed to fetch from failover region failover-region-2"
+      assert log =~ "s3://#{@bucket}/#{@key} - Trying failover region: failover-region-1"
+      assert log =~ "s3://#{@bucket}/#{@key} - Failed to fetch from failover region failover-region-1"
+      assert log =~ "s3://#{@bucket}/#{@key} - Trying failover region: failover-region-2"
+      assert log =~ "s3://#{@bucket}/#{@key} - Failed to fetch from failover region failover-region-2"
     end
 
     test "tries second failover region when first failover region fails" do
@@ -167,10 +167,10 @@ defmodule RemotePersistentTerm.Fetcher.S3Test do
           assert {:ok, "content from second failover region"} = result
         end)
 
-      assert log =~ "Failed to fetch from primary region #{@region}"
-      assert log =~ "Trying failover region: failover-region-1"
-      assert log =~ "Failed to fetch from failover region failover-region-1"
-      assert log =~ "Trying failover region: failover-region-2"
+      assert log =~ "s3://#{@bucket}/#{@key} - Failed to fetch from primary region #{@region}"
+      assert log =~ "s3://#{@bucket}/#{@key} - Trying failover region: failover-region-1"
+      assert log =~ "s3://#{@bucket}/#{@key} - Failed to fetch from failover region failover-region-1"
+      assert log =~ "s3://#{@bucket}/#{@key} - Trying failover region: failover-region-2"
     end
   end
 end