Repository: elixir-grpc/grpc
Pull request #289 (2023-01-02 21:02:12)

Split client/server interceptors
Resolves https://github.com/elixir-grpc/grpc/issues/284.




Patch:
diff --git a/README.md b/README.md
index 2defec5..6164ab1 100644
--- a/README.md
+++ b/README.md
@@ -65,7 +65,7 @@ You can start the gRPC server as a supervised process. First, add `GRPC.Server.S
 defmodule Helloworld.Endpoint do
   use GRPC.Endpoint
 
-  intercept GRPC.Logger.Server
+  intercept GRPC.Server.Interceptors.Logger
   run Helloworld.Greeter.Server
 end
 
@@ -92,7 +92,7 @@ iex> request = Helloworld.HelloRequest.new(name: "grpc-elixir")
 iex> {:ok, reply} = channel |> Helloworld.Greeter.Stub.say_hello(request)
 
 # With interceptors
-iex> {:ok, channel} = GRPC.Stub.connect("localhost:50051", interceptors: [GRPC.Logger.Client])
+iex> {:ok, channel} = GRPC.Stub.connect("localhost:50051", interceptors: [GRPC.Client.Interceptors.Logger])
 ...
 ```
 
diff --git a/examples/helloworld/lib/endpoint.ex b/examples/helloworld/lib/endpoint.ex
index 70533a4..c8bc64f 100644
--- a/examples/helloworld/lib/endpoint.ex
+++ b/examples/helloworld/lib/endpoint.ex
@@ -1,6 +1,6 @@
 defmodule Helloworld.Endpoint do
   use GRPC.Endpoint
 
-  intercept GRPC.Logger.Server
+  intercept GRPC.Server.Interceptors.Logger
   run Helloworld.Greeter.Server
 end
diff --git a/examples/helloworld/priv/client.exs b/examples/helloworld/priv/client.exs
index dc6bea5..0209556 100644
--- a/examples/helloworld/priv/client.exs
+++ b/examples/helloworld/priv/client.exs
@@ -1,4 +1,4 @@
-{:ok, channel} = GRPC.Stub.connect("localhost:50051", interceptors: [GRPC.Logger.Client])
+{:ok, channel} = GRPC.Stub.connect("localhost:50051", interceptors: [GRPC.Client.Interceptors.Logger])
 
 {:ok, reply} =
   channel
diff --git a/examples/route_guide/lib/endpoint.ex b/examples/route_guide/lib/endpoint.ex
index 08ca7fc..5a25c58 100644
--- a/examples/route_guide/lib/endpoint.ex
+++ b/examples/route_guide/lib/endpoint.ex
@@ -1,6 +1,6 @@
 defmodule Routeguide.Endpoint do
   use GRPC.Endpoint
 
-  intercept GRPC.Logger.Server
+  intercept GRPC.Server.Interceptors.Logger
   run Routeguide.RouteGuide.Server
 end
diff --git a/examples/route_guide/priv/client.exs b/examples/route_guide/priv/client.exs
index 79a33f0..5545ce9 100644
--- a/examples/route_guide/priv/client.exs
+++ b/examples/route_guide/priv/client.exs
@@ -1,4 +1,4 @@
-opts = [interceptors: [GRPC.Logger.Client]]
+opts = [interceptors: [GRPC.Client.Interceptors.Logger]]
 
 opts =
   if System.get_env("TLS") do
diff --git a/interop/lib/interop/endpoint.ex b/interop/lib/interop/endpoint.ex
index c979ac1..43f29cd 100644
--- a/interop/lib/interop/endpoint.ex
+++ b/interop/lib/interop/endpoint.ex
@@ -1,7 +1,7 @@
 defmodule Interop.Endpoint do
   use GRPC.Endpoint
 
-  intercept GRPC.Logger.Server
+  intercept GRPC.Server.Interceptors.Logger
   intercept GRPCPrometheus.ServerInterceptor
   intercept Interop.ServerInterceptor
 
diff --git a/interop/script/run.exs b/interop/script/run.exs
index 81cb5d5..99d334e 100644
--- a/interop/script/run.exs
+++ b/interop/script/run.exs
@@ -18,7 +18,7 @@ alias Interop.Client
 
 1..concurrency
 |> Task.async_stream(fn _cli ->
-  ch = Client.connect("127.0.0.1", port, interceptors: [GRPCPrometheus.ClientInterceptor, GRPC.Logger.Client])
+  ch = Client.connect("127.0.0.1", port, interceptors: [GRPCPrometheus.ClientInterceptor, GRPC.Client.Interceptors.Logger])
 
   for _ <- 1..rounds do
     Client.empty_unary!(ch)
diff --git a/lib/grpc/client/interceptor.ex b/lib/grpc/client/interceptor.ex
new file mode 100644
index 0000000..2746bf8e
--- /dev/null
+++ b/lib/grpc/client/interceptor.ex
@@ -0,0 +1,13 @@
+defmodule GRPC.Client.Interceptor do
+  @moduledoc """
+  Interceptor on client side. See `GRPC.Stub.connect/2`.
+  """
+  alias GRPC.Client.Stream
+
+  @type options :: any()
+  @type req :: struct() | nil
+  @type next :: (Stream.t(), req -> GRPC.Stub.rpc_return())
+
+  @callback init(options) :: options
+  @callback call(stream :: Stream.t(), req, next, options) :: GRPC.Stub.rpc_return()
+end
diff --git a/lib/grpc/logger/client.ex b/lib/grpc/client/interceptors/logger.ex
similarity index 79%
rename from lib/grpc/logger/client.ex
rename to lib/grpc/client/interceptors/logger.ex
index a841e34..35f7b6a 100644
--- a/lib/grpc/logger/client.ex
+++ b/lib/grpc/client/interceptors/logger.ex
@@ -1,4 +1,4 @@
-defmodule GRPC.Logger.Client do
+defmodule GRPC.Client.Interceptors.Logger do
   @moduledoc """
   Print log around client rpc calls, like
 
@@ -13,18 +13,18 @@ defmodule GRPC.Logger.Client do
 
   ## Usage
 
-      {:ok, channel} = GRPC.Stub.connect("localhost:50051", interceptors: [GRPC.Logger.Client])
+      {:ok, channel} = GRPC.Stub.connect("localhost:50051", interceptors: [GRPC.Client.Interceptors.Logger])
       # This will log on `:info` and greater priority
-      {:ok, channel} = GRPC.Stub.connect("localhost:50051", interceptors: [{GRPC.Logger.Client, level: :info}])
+      {:ok, channel} = GRPC.Stub.connect("localhost:50051", interceptors: [{GRPC.Client.Interceptors.Logger, level: :info}])
       # This will log only on `:info`
-      {:ok, channel} = GRPC.Stub.connect("localhost:50051", interceptors: [{GRPC.Logger.Client, level: :info, accepted_comparators: [:eq]}])
+      {:ok, channel} = GRPC.Stub.connect("localhost:50051", interceptors: [{GRPC.Client.Interceptors.Logger, level: :info, accepted_comparators: [:eq]}])
       # This will log on `:info` and lower priority
-      {:ok, channel} = GRPC.Stub.connect("localhost:50051", interceptors: [{GRPC.Logger.Client, level: :info, accepted_comparators: [:eq, :gt]}])
+      {:ok, channel} = GRPC.Stub.connect("localhost:50051", interceptors: [{GRPC.Client.Interceptors.Logger, level: :info, accepted_comparators: [:eq, :gt]}])
   """
 
   require Logger
 
-  @behaviour GRPC.ClientInterceptor
+  @behaviour GRPC.Client.Interceptor
 
   @impl true
   def init(opts) do
@@ -53,7 +53,7 @@ defmodule GRPC.Logger.Client do
         Logger.log(level, fn ->
           diff = System.convert_time_unit(stop - start, :native, :microsecond)
 
-          ["Got ", inspect(status), " in ", GRPC.Logger.Server.formatted_diff(diff)]
+          ["Got ", inspect(status), " in ", GRPC.Server.Interceptors.Logger.formatted_diff(diff)]
         end)
       end
 
diff --git a/lib/grpc/endpoint.ex b/lib/grpc/endpoint.ex
index a51cfb2..dadf09b 100644
--- a/lib/grpc/endpoint.ex
+++ b/lib/grpc/endpoint.ex
@@ -7,15 +7,15 @@ defmodule GRPC.Endpoint do
       defmodule Your.Endpoint do
         use GRPC.Endpoint
 
-        intercept GRPC.Logger.Server, level: :info
+        intercept GRPC.Server.Interceptors.Logger, level: :info
         intercept Other.Interceptor
         run HelloServer, interceptors: [HelloHaltInterceptor]
         run FeatureServer
       end
 
   Interceptors will be run around your rpc calls from top to bottom. And you can even set
-  interceptors for some of servers. In the above example, `[GRPC.Logger.Server, Other.Interceptor,
-  HelloHaltInterceptor]` will be run for `HelloServer`, and `[GRPC.Logger.Server, Other.Interceptor]`
+  interceptors for some of servers. In the above example, `[GRPC.Server.Interceptors.Logger, Other.Interceptor,
+  HelloHaltInterceptor]` will be run for `HelloServer`, and `[GRPC.Server.Interceptors.Logger, Other.Interceptor]`
   will be run for `FeatureServer`.
   """
 
diff --git a/lib/grpc/interceptor.ex b/lib/grpc/server/interceptor.ex
similarity index 50%
rename from lib/grpc/interceptor.ex
rename to lib/grpc/server/interceptor.ex
index ac64dc4..5c9ff6f 100644
--- a/lib/grpc/interceptor.ex
+++ b/lib/grpc/server/interceptor.ex
@@ -1,4 +1,4 @@
-defmodule GRPC.ServerInterceptor do
+defmodule GRPC.Server.Interceptor do
   @moduledoc """
   Interceptor on server side. See `GRPC.Endpoint`.
   """
@@ -12,17 +12,3 @@ defmodule GRPC.ServerInterceptor do
   @callback init(options) :: options
   @callback call(GRPC.Server.rpc_req(), stream :: Stream.t(), next, options) :: rpc_return
 end
-
-defmodule GRPC.ClientInterceptor do
-  @moduledoc """
-  Interceptor on client side. See `GRPC.Stub.connect/2`.
-  """
-  alias GRPC.Client.Stream
-
-  @type options :: any()
-  @type req :: struct() | nil
-  @type next :: (Stream.t(), req -> GRPC.Stub.rpc_return())
-
-  @callback init(options) :: options
-  @callback call(stream :: Stream.t(), req, next, options) :: GRPC.Stub.rpc_return()
-end
diff --git a/lib/grpc/logger/server.ex b/lib/grpc/server/interceptors/logger.ex
similarity index 84%
rename from lib/grpc/logger/server.ex
rename to lib/grpc/server/interceptors/logger.ex
index b9070ea..5f6e5d1 100644
--- a/lib/grpc/logger/server.ex
+++ b/lib/grpc/server/interceptors/logger.ex
@@ -1,4 +1,4 @@
-defmodule GRPC.Logger.Server do
+defmodule GRPC.Server.Interceptors.Logger do
   @moduledoc """
   Print log around server rpc calls, like:
 
@@ -16,27 +16,27 @@ defmodule GRPC.Logger.Server do
       defmodule Your.Endpoint do
         use GRPC.Endpoint
 
-        intercept GRPC.Logger.Server, level: :info
+        intercept GRPC.Server.Interceptors.Logger, level: :info
       end
 
       defmodule Your.Endpoint do
         use GRPC.Endpoint
 
         # logs on :info and higher priority (warn, error...)
-        intercept GRPC.Logger.Server, level: :info, accepted_comparators: [:lt, :eq]
+        intercept GRPC.Server.Interceptors.Logger, level: :info, accepted_comparators: [:lt, :eq]
       end
 
       defmodule Your.Endpoint do
         use GRPC.Endpoint
 
         # logs only on :error
-        intercept GRPC.Logger.Server, level: :error, accepted_comparators: [:eq]
+        intercept GRPC.Server.Interceptors.Logger, level: :error, accepted_comparators: [:eq]
       end
   """
 
   require Logger
 
-  @behaviour GRPC.ServerInterceptor
+  @behaviour GRPC.Server.Interceptor
 
   @impl true
   def init(opts) do


Test patch:
diff --git a/examples/helloworld/test/hello_world_test.exs b/examples/helloworld/test/hello_world_test.exs
index 962d07a..55c51ed 100644
--- a/examples/helloworld/test/hello_world_test.exs
+++ b/examples/helloworld/test/hello_world_test.exs
@@ -4,7 +4,7 @@ defmodule HelloworldTest do
   use ExUnit.Case
 
   setup_all do
-    {:ok, channel} = GRPC.Stub.connect("localhost:50051", interceptors: [GRPC.Logger.Client])
+    {:ok, channel} = GRPC.Stub.connect("localhost:50051", interceptors: [GRPC.Client.Interceptors.Logger])
     [channel: channel]
   end
 
diff --git a/test/grpc/client/interceptors/logger_test.exs b/test/grpc/client/interceptors/logger_test.exs
new file mode 100644
index 0000000..7fc4513
--- /dev/null
+++ b/test/grpc/client/interceptors/logger_test.exs
@@ -0,0 +1,50 @@
+defmodule GRPC.Client.Interceptors.LoggerTest do
+  use ExUnit.Case, async: false
+
+  import ExUnit.CaptureLog
+
+  alias GRPC.Client.Interceptors.Logger, as: LoggerInterceptor
+  alias GRPC.Client.Stream
+
+  test "accepted_comparators filter logs correctly" do
+    for {configured_level, accepted_comparators, should_log} <-
+          [
+            {:error, [:lt], false},
+            {:error, [:eq], false},
+            {:error, [:gt], true},
+            {:debug, [:eq], false},
+            {:debug, [:eq, :gt], false},
+            {:info, [:lt, :eq], true}
+          ] do
+      logger_level = Logger.level()
+      assert logger_level == :info
+
+      service_name = "service_name"
+      rpc = {1, 2, 3}
+
+      logs =
+        capture_log(fn ->
+          stream = %Stream{grpc_type: :unary, rpc: rpc, service_name: service_name}
+
+          LoggerInterceptor.call(
+            stream,
+            :request,
+            fn ^stream, :request -> {:ok, :ok} end,
+            LoggerInterceptor.init(
+              level: configured_level,
+              accepted_comparators: accepted_comparators
+            )
+          )
+        end)
+
+      if should_log do
+        assert Regex.match?(
+                 ~r/\[#{configured_level}\]\s+Call #{to_string(elem(rpc, 0))} of #{service_name}/,
+                 logs
+               )
+      else
+        assert logs == ""
+      end
+    end
+  end
+end
diff --git a/test/grpc/integration/client_interceptor_test.exs b/test/grpc/integration/client_interceptor_test.exs
index b6897a2..b24ef7c 100644
--- a/test/grpc/integration/client_interceptor_test.exs
+++ b/test/grpc/integration/client_interceptor_test.exs
@@ -12,6 +12,8 @@ defmodule GRPC.Integration.ClientInterceptorTest do
   end
 
   defmodule AddHeadersClientInterceptor do
+    @behaviour GRPC.Client.Interceptor
+
     def init(label), do: label
 
     def call(%{headers: headers} = stream, req, next, label) do
diff --git a/test/grpc/integration/endpoint_test.exs b/test/grpc/integration/endpoint_test.exs
index a0015cf..af3d338 100644
--- a/test/grpc/integration/endpoint_test.exs
+++ b/test/grpc/integration/endpoint_test.exs
@@ -13,7 +13,7 @@ defmodule GRPC.Integration.EndpointTest do
   defmodule HelloEndpoint do
     use GRPC.Endpoint
 
-    intercept GRPC.Logger.Server, level: :info, accepted_comparators: [:lt, :eq, :gt]
+    intercept GRPC.Server.Interceptors.Logger, level: :info, accepted_comparators: [:lt, :eq, :gt]
     run HelloServer
   end
 
@@ -51,14 +51,14 @@ defmodule GRPC.Integration.EndpointTest do
   defmodule FeatureEndpoint do
     use GRPC.Endpoint
 
-    intercept GRPC.Logger.Server, accepted_comparators: [:lt, :eq, :gt]
+    intercept GRPC.Server.Interceptors.Logger, accepted_comparators: [:lt, :eq, :gt]
     run FeatureServer
   end
 
   defmodule FeatureAndHelloHaltEndpoint do
     use GRPC.Endpoint
 
-    intercept GRPC.Logger.Server, accepted_comparators: [:lt, :eq, :gt]
+    intercept GRPC.Server.Interceptors.Logger, accepted_comparators: [:lt, :eq, :gt]
     run HelloServer, interceptors: [HelloHaltInterceptor]
     run FeatureServer
   end
diff --git a/test/grpc/integration/erlpack_notypes.ex b/test/grpc/integration/erlpack_notypes.ex
index cf230f2..2037652 100644
--- a/test/grpc/integration/erlpack_notypes.ex
+++ b/test/grpc/integration/erlpack_notypes.ex
@@ -24,7 +24,7 @@ defmodule GRPC.Integration.ErplackNotypesTest do
       {:ok, channel} =
         GRPC.Stub.connect(
           "localhost:#{port}",
-          interceptors: [GRPC.Logger.Client],
+          interceptors: [GRPC.Client.Interceptors.Logger],
           codec: GRPC.Codec.Erlpack
         )
 
@@ -36,7 +36,9 @@ defmodule GRPC.Integration.ErplackNotypesTest do
 
   test "Says hello over erlpack call level" do
     run_server(HelloServer, fn port ->
-      {:ok, channel} = GRPC.Stub.connect("localhost:#{port}", interceptors: [GRPC.Logger.Client])
+      {:ok, channel} =
+        GRPC.Stub.connect("localhost:#{port}", interceptors: [GRPC.Client.Interceptors.Logger])
+
       name = "World"
       {:ok, reply} = channel |> HelloErlpackStub.reply_hello(name, codec: GRPC.Codec.Erlpack)
       assert reply == {:ok, "Hello, #{name}"}
diff --git a/test/grpc/integration/stub_test.exs b/test/grpc/integration/stub_test.exs
index 7a25642..fd37e95 100644
--- a/test/grpc/integration/stub_test.exs
+++ b/test/grpc/integration/stub_test.exs
@@ -58,7 +58,9 @@ defmodule GRPC.Integration.StubTest do
 
   test "body larger than 2^14 works" do
     run_server(HelloServer, fn port ->
-      {:ok, channel} = GRPC.Stub.connect("localhost:#{port}", interceptors: [GRPC.Logger.Client])
+      {:ok, channel} =
+        GRPC.Stub.connect("localhost:#{port}", interceptors: [GRPC.Client.Interceptors.Logger])
+
       name = String.duplicate("a", round(:math.pow(2, 15)))
       req = Helloworld.HelloRequest.new(name: name)
       {:ok, reply} = channel |> Helloworld.Greeter.Stub.say_hello(req)
diff --git a/test/grpc/logger/server_test.exs b/test/grpc/server/interceptors/logger_test.exs
similarity index 84%
rename from test/grpc/logger/server_test.exs
rename to test/grpc/server/interceptors/logger_test.exs
index b666762..3370063 100644
--- a/test/grpc/logger/server_test.exs
+++ b/test/grpc/server/interceptors/logger_test.exs
@@ -1,10 +1,9 @@
-defmodule GRPC.Logger.ServerTest do
+defmodule GRPC.Server.Interceptors.LoggerTest do
   use ExUnit.Case, async: false
 
   import ExUnit.CaptureLog
 
-  alias GRPC.Logger.Server
-
+  alias GRPC.Server.Interceptors.Logger, as: LoggerInterceptor
   alias GRPC.Server.Stream
 
   test "request id is only set if not previously set" do
@@ -13,22 +12,22 @@ defmodule GRPC.Logger.ServerTest do
     request_id = to_string(System.monotonic_time())
     stream = %Stream{server: :server, rpc: {1, 2, 3}, request_id: request_id}
 
-    Server.call(
+    LoggerInterceptor.call(
       :request,
       stream,
       fn :request, ^stream -> {:ok, :ok} end,
-      Server.init(level: :info)
+      LoggerInterceptor.init(level: :info)
     )
 
     assert [request_id: request_id] == Logger.metadata()
 
     stream = %{stream | request_id: nil}
 
-    Server.call(
+    LoggerInterceptor.call(
       :request,
       stream,
       fn :request, ^stream -> {:ok, :ok} end,
-      Server.init(level: :info)
+      LoggerInterceptor.init(level: :info)
     )
 
     assert request_id == Logger.metadata()[:request_id]
@@ -53,11 +52,11 @@ defmodule GRPC.Logger.ServerTest do
         capture_log(fn ->
           stream = %Stream{server: server_name, rpc: {1, 2, 3}, request_id: "1234"}
 
-          Server.call(
+          LoggerInterceptor.call(
             :request,
             stream,
             fn :request, ^stream -> {:ok, :ok} end,
-            Server.init(
+            LoggerInterceptor.init(
               level: configured_level,
               accepted_comparators: accepted_comparators
             )