Split client/server interceptors (#289)

* Split client/server interceptors

* Add client logger test

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])
 ...
 ```
 

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

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

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
 

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

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

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
 

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)

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

lib/grpc/logger/client.ex → 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
 

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`.
   """
 

lib/grpc/interceptor.ex → 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

lib/grpc/logger/server.ex → 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/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

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

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

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}"}

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)

test/grpc/logger/server_test.exs → 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
             )