From 3146bab5817b2148e78f526242b4086cf9ed950e Mon Sep 17 00:00:00 2001 From: alvarovillen Date: Mon, 2 Jan 2023 19:52:35 +0100 Subject: [PATCH 1/2] Split client/server interceptors --- README.md | 4 ++-- examples/helloworld/lib/endpoint.ex | 2 +- examples/helloworld/priv/client.exs | 2 +- examples/helloworld/test/hello_world_test.exs | 2 +- examples/route_guide/lib/endpoint.ex | 2 +- examples/route_guide/priv/client.exs | 2 +- interop/lib/interop/endpoint.ex | 2 +- interop/script/run.exs | 2 +- lib/grpc/client/interceptor.ex | 13 +++++++++++++ .../client.ex => client/interceptors/logger.ex} | 14 +++++++------- lib/grpc/endpoint.ex | 6 +++--- lib/grpc/{ => server}/interceptor.ex | 16 +--------------- .../server.ex => server/interceptors/logger.ex} | 10 +++++----- .../integration/client_interceptor_test.exs | 2 ++ test/grpc/integration/endpoint_test.exs | 6 +++--- test/grpc/integration/erlpack_notypes.ex | 6 ++++-- test/grpc/integration/stub_test.exs | 4 +++- .../interceptors/logger_test.exs} | 17 ++++++++--------- 18 files changed, 58 insertions(+), 54 deletions(-) create mode 100644 lib/grpc/client/interceptor.ex rename lib/grpc/{logger/client.ex => client/interceptors/logger.ex} (79%) rename lib/grpc/{ => server}/interceptor.ex (50%) rename lib/grpc/{logger/server.ex => server/interceptors/logger.ex} (84%) rename test/grpc/{logger/server_test.exs => server/interceptors/logger_test.exs} (84%) diff --git a/README.md b/README.md index 2defec51..6164ab17 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 70533a48..c8bc64f6 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 dc6bea5d..0209556f 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/helloworld/test/hello_world_test.exs b/examples/helloworld/test/hello_world_test.exs index 962d07ac..55c51ed5 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/examples/route_guide/lib/endpoint.ex b/examples/route_guide/lib/endpoint.ex index 08ca7fc2..5a25c582 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 79a33f0f..5545ce9f 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 c979ac15..43f29cd6 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 81cb5d53..99d334e7 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 00000000..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 a841e342..35f7b6a8 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 a51cfb2d..dadf09be 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 ac64dc45..5c9ff6fc 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 b9070eaa..5f6e5d1d 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 diff --git a/test/grpc/integration/client_interceptor_test.exs b/test/grpc/integration/client_interceptor_test.exs index b6897a25..b24ef7cf 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 a0015cf1..af3d338a 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 cf230f2f..20376523 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 7a256429..fd37e954 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 b6667622..3370063f 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 ) From c81bd7bbdab3b4269103c16e8aa00eb151fa82f1 Mon Sep 17 00:00:00 2001 From: alvarovillen Date: Mon, 2 Jan 2023 21:52:50 +0100 Subject: [PATCH 2/2] Add client logger test --- test/grpc/client/interceptors/logger_test.exs | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 test/grpc/client/interceptors/logger_test.exs diff --git a/test/grpc/client/interceptors/logger_test.exs b/test/grpc/client/interceptors/logger_test.exs new file mode 100644 index 00000000..7fc45130 --- /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