[Aio] Implement server interceptor for unary unary call by ZHmao · Pull Request #22032 · grpc/grpc

ZHmao · 2020-02-15T15:45:08Z

Support for interceptors for the unary-unary call at the server-side.

Fix #21914
Ref #20482

Sync with grpc repo

sync

Sync with grpc repo

Sync with grpc

src/python/grpcio/grpc/_cython/_cygrpc/aio/server.pyx.pxi

rmariano · 2020-02-17T13:33:35Z

src/python/grpcio/grpc/_cython/_cygrpc/aio/server.pyx.pxi

+
+async def _find_method_handler(str method, tuple metadata, list generic_handlers,
+                          tuple interceptors):
+    def query_handlers(handler_call_details):


can't we replaced this by a geneator expression + next() ?

method_handlers = (generic_handler.service(handler_call_details) for generic_handler in generic_handlers) method_handler = next((mh for mh in method_handlers if mh is not None), None)

We will need to iterate them in _run_interceptors[1] if we use generator at here.
IMO, current version will make _run_interceptors simpler.

[1] https://github.com/grpc/grpc/pull/22032/files/6fef56573e9a0347c33c5aea4bc57ab625b0c6ea#diff-b53c077f7911f53dc8ce7656e16d35cdR218

rmariano · 2020-02-17T14:07:05Z

src/python/grpcio/grpc/_cython/_cygrpc/aio/server.pyx.pxi


 import inspect
 import traceback
+import functools


Where is this being used? 🤔

I inject some parameters at here: https://github.com/grpc/grpc/pull/22032/files/6fef56573e9a0347c33c5aea4bc57ab625b0c6ea#diff-b53c077f7911f53dc8ce7656e16d35cdR222

pfreixes

Thanks for the work done!

First round of comments, Ive just focused on the tests

pfreixes · 2020-02-18T15:24:12Z

src/python/grpcio_tests/tests_aio/unit/interceptor_test.py

                self.fail("Callback was not called")


+class _LoggingServerInterceptor(aio.ServerInterceptor):


I would move all of the server test interceptor logic under a new file, like sever_interceptor_test.py, and maybe I would rename the former one to client_interceptor_test.py

pfreixes · 2020-02-18T16:02:51Z

src/python/grpcio_tests/tests_aio/unit/interceptor_test.py

+
+        with self.assertRaises(aio.AioRpcError):
+            server_target, _ = await start_test_server(
+                interceptors=(InvalidInterceptor(),))


a Channel with an invalid interceptor is not allowed to be instantiated [1 ], having the feeling that we would need to do the same with the server.

[1] https://github.com/grpc/grpc/blob/master/src/python/grpcio/grpc/experimental/aio/_channel.py#L317

pfreixes · 2020-02-18T16:06:08Z

src/python/grpcio_tests/tests_aio/unit/interceptor_test.py

+            # in the right order.
+            self.assertSequenceEqual(['log1:intercept_service',
+                                      'log2:intercept_service',], self._record)
+            self.assertIsInstance(response, messages_pb2.SimpleResponse)


Maybe we can have another test - test_response_ok ? - for checking that the response is looking as we would be expecting by checking the status and the response content. With this specific test, we won't need to check at each test the response

pfreixes · 2020-02-18T16:09:21Z

src/python/grpcio_tests/tests_aio/unit/interceptor_test.py

+            _LoggingServerInterceptor('log1', self._record),
+            conditional_interceptor,
+            _LoggingServerInterceptor('log2', self._record),
+        )


Having all of the interceptors added into the setup which they would be used later for all of the tests makes the readability a bit harder for me, TBH I would prefer to have each test case with its own server with its own interceptors for testing the different functionalities.

The same as you already did for the test_invalid_interceptor test case.

Having the feeling that having this kind of sharing, the same happens with the _record class attribute, we have a lot of chances of creating down-side effects between tests.

pfreixes · 2020-02-18T16:23:02Z

src/python/grpcio_tests/tests_aio/unit/interceptor_test.py

+    return _GenericServerInterceptor(intercept_service)
+
+
+class TestServerInterceptor(AioTestBase):


I would like to have a test for checking observability of the code responses sent by the handler, in the same way as we have for the channel [1]

[1] https://github.com/grpc/grpc/blob/master/src/python/grpcio_tests/tests_aio/unit/interceptor_test.py#L93

Server interceptors are wrapped at find_method_handler , the interceptors won't get the status code of response.

lidizheng

Good work!

lidizheng · 2020-02-19T00:14:28Z

src/python/grpcio/grpc/_cython/_cygrpc/aio/server.pyx.pxi

+        return await _run_interceptor(iter(interceptors), query_handlers,
+                                      handler_call_details)
+    else:
+        return query_handlers(handler_call_details)


Both _run_interceptor and _find_method_handler functions are on the data path, is it possible to simplify them? Or run the benchmark to check the impact on performance?

what about doing something like:

async def _find_method_handler(...): for generic_handler in generic_handlers: method_handler = generic_handler.service(handler_call_details) if method_handler is not None: break if interceptors: return await _run_interceptor(....., method_handler) else: return method_handler

With this change theoretically the execution path when there is no interceptors should not be hammered.

WDYT? /cc @lidizheng

It depends on whether we allow users to change the method.

If allowed, the method handler needs to be ran as last function in the chain, otherwise users' interception might not work.

If not allowed, this is a valid optimization.

lidizheng · 2020-02-19T00:16:06Z

src/python/grpcio_tests/tests_aio/unit/interceptor_test.py

+        return await self._fn(continuation, handler_call_details)
+
+
+def _filter_server_interceptor(condition, interceptor):


To improve the maintainability of test cases, we might want to add type annotations, especially for callbacks.

lidizheng · 2020-02-19T00:16:58Z

src/python/grpcio_tests/tests_aio/unit/interceptor_test.py

+                                      'log3:intercept_service',
+                                      'log2:intercept_service',],
+                                     self._record)
+


Should we add one more test case that the interceptor returns a different method handler?

I couldn't understand this one, can you give me more details?

gnossen

LGTM pending resolution of everyone else's comments.

gnossen · 2020-02-19T19:48:37Z

src/python/grpcio_tests/tests_aio/unit/interceptor_test.py

+        return await self._fn(continuation, handler_call_details)
+
+
+def _filter_server_interceptor(condition, interceptor):


Very cool usage of interceptors. 👍

mehrdada · 2020-02-20T00:36:12Z

One of my regrets with the original design for sync interceptors is allowing server-interceptors to be flexible and change the path which massively limits the opportunity of optimization for handing over to handlers and doing so is non-idiomatic gRPC anyway. Please make sure you at least revisit whether this flexibility is worth the cost.

lidizheng · 2020-02-20T00:41:23Z

@mehrdada By

... which massively limits the opportunity of optimization for handing over to handlers...

Are you referring to the potential optimization in finding methods? Otherwise, it could be done in Cython or C++? Can you explain more about this topic?

pfreixes · 2020-02-20T08:08:16Z

@ZHmao also consider that current PR has conflicts that are preventing us to run the whole CI, I would appreciate if you could resolve the conflicts.

Thanks!

mehrdada · 2020-02-21T00:55:38Z

@lidizheng C-Core server supports registering RPC paths directly upfront as opposed to everything be "the generic handler". Python API is clearly designed to be able to leverage it (see ServiceRpcHandler and GenericRpcHandler) but never did. If we know which handler is going to be invoked upfront, we can create the interceptor handling pipeline per path upfront at server start time and have C core or Cython via a proper efficient hashtable that can run outside GIL or whatever handle it efficiently before we hand it over to Python, basically letting slow application code to resolve which method handler is going to be invoked. With the generic server interceptor API, we give user flexibility to change it which is often overkill and we won't know the handler to dispatch to upfront.

ZHmao · 2020-02-21T07:02:30Z

@ZHmao also consider that current PR has conflicts that are preventing us to run the whole CI, I would appreciate if you could resolve the conflicts.

Thanks!

Sure, I will.

pfreixes · 2020-02-21T08:10:11Z

@mehrdada @lidizheng having the chance of adding interceptors for specific handlers is something that could be really useful, not only because of performance but because the user has the chance of adding a piece of logic that is related to a specific path.

On the other hand, generic handlers are gonna be needed still for doing generic stuff like observability, tracing, etc ...

IMO the interceptors discussion with regards the design is not yet closed, we have two outstanding g questions that would need to be answered before making this feature as no longer experimental:

The granularity of the interceptors. how does the current design work with the other arities? A design based on hooks would make everything easier? and more important less prone error?
Generic handlers vs route/path handlers. How we would implement both use cases? do the route/path handlers make sense on the client-side?

Right now we are on the way of doing an alpha release and this feature is marked as experimental, so I guess that we have still full freedom on breaking the API or extending it without having the friction of being in a beta/stable release.

…ry-call

mehrdada · 2020-02-22T00:58:49Z

@pfreixes I am fine with releasing alpha or whatever as long as it can be changed. To clarify, my concern is a bit orthogonal to interceptors being able to attach to specific paths (although I am not necessarily a fan, it is a valid feature to debate). The limitation that I like to see enforced is interceptor not being able to alter the path it is invoked on, i.e. return a different path than the one it's invoked on. It is valid for it to see the path. It might require more work and a more complex API, but it is also alright if it can be configured to get invoked on certain paths and not others as long as all this info is known upfront on server start, because you can still construct the handler pipelines once ahead of the time and dispatch accordingly, even if the pipelines vary per request path.

pfreixes · 2020-02-25T08:59:59Z

src/python/grpcio/grpc/_cython/_cygrpc/aio/server.pyx.pxi

-    return None
+    # interceptor
+    if interceptors:
+        return await _run_interceptor(iter(interceptors), query_handlers,


Should we return and not execute, the _run_interceptor coroutine? This would mean that the execution will eventually happen at the very last step and the interceptor for example in the case of a unary call will be here [1]

Indeed we could name this method like _wrap_call_interceptor or something like that.

Without doing this, future interceptors that would take care of observability for measuring the time that handler took won't work since they won't be able to measure the real execution of the handler.

Also moving this to the right place, will eventually will help us for extending the interceptors for having visibility of the servicer context [2] at the interceptor level.

The only friction point of doing this that I can see, if Im not missing something is the access to the metadata. So the interceptor won't be able to have direct access to the metadata and mutate them, but could have access to the service context that will make the metadata available for the interceptor.

WDYT @lidizheng should we aim for moving the execution of the interceptor to the very last step?

Also, I'm wondering if since we are not most likely commit to having this feature for the alpha release we could start thinking on how a nex iteration of the servers interceptors could be done for knowing the response code of the method handler. Would this change help us to do so?

[1] https://github.com/grpc/grpc/blob/master/src/python/grpcio/grpc/_cython/_cygrpc/aio/server.pyx.pxi#L242
[2] https://github.com/grpc/grpc/blob/master/src/python/grpcio/grpc/_cython/_cygrpc/aio/server.pyx.pxi#L244

I agree that the server interceptors should be more powerful. In fact, it would be great, if it has similar functionality as the client-side.

Back to your proposal, it might introduce a regression if users are selecting method handlers according to the invocation metadata (client-sent initial metadata). This feature is supported in the existing stack, even without interceptors (although I wonder if any user uses it).

If we decided to optimize the design of generic handlers (basically let the Core route the requests), then users cannot use this feature anyway. In this case, yes, I think this proposal is valid.

should we aim for moving the execution of the interceptor to the very last step? ... could be done for knowing the response code of the method handler.

If we want a proper full-featured server interceptor, we can make interceptors wrapping entire method handler (similar to client-side). If we only executed it till the last step, users can only either inject logic before or after the RPC (or I might be wrong).

If we only want an interceptor design better than the existing stack, I believe the move does improve server interceptors capability.

src/python/grpcio/grpc/experimental/aio/_server.py

pfreixes · 2020-02-26T23:03:49Z

@lidizheng and @ZHmao Im happy on having an iterative process for implementing the server interceptors, so having first implemented a version that works as the legacy stack is IMO already giving value.

So I would not have any inconvenience in approving this PR and later on have other PR for making the server interceptors more powerful. Indeed I would say that it will be the best thing to do.

But there is something that I do not understand, looking at this interceptor [1] that is the one used by one of our first adopters of gRPC that are using the legacy stack, seems that the server version allows you to at least:

Catch exceptions returned by the handler
Get the response returned by the handler
Measure the time spent within the handler

Looking at the PR seems that neither of the previous capabilities are ready to be used. am I wrong? what Im missing?

[1] https://github.com/opentracing-contrib/python-grpc/blob/325edbb83f08110e7d2ea1d1c453aaf61666b6e7/grpc_opentracing/_server.py#L139

ZHmao · 2020-02-27T15:20:51Z

Hi @pfreixes , they used a new class _InterceptorServer to wrap the original server [1], and changed the behavior of add_generic_rpc_handlers, they added interceptor for every rpc handler.

[1] https://github.com/opentracing-contrib/python-grpc/blob/325edbb83f08110e7d2ea1d1c453aaf61666b6e7/grpc_opentracing/grpcext/_interceptor.py#L342

pfreixes · 2020-02-27T15:58:55Z

@ZHmao wow, I missed that! So they did what we would like to have.... great example on how the users follow their own paths for circumventing the current limitations.

So in that case, if we are already meeting the same requirements that are met by the legacy stack take me 👍 but I would like to see a plan for designing a server interceptor that supersedes the current one and is paired to what lightstep [1] has done.

PTAL to the sanity checks, most. of them are failing.

[1] https://github.com/opentracing-contrib/python-grpc/blob/325edbb83f08110e7d2ea1d1c453aaf61666b6e7/grpc_opentracing/grpcext/_interceptor.py#L342

lidizheng

Great work and great test coverage!

lidizheng · 2020-02-27T19:02:54Z

src/python/grpcio_tests/tests_aio/unit/server_interceptor_test.py

@@ -0,0 +1,144 @@
+# Copyright 2019 The gRPC Authors.


Suggested change

# Copyright 2019 The gRPC Authors.

# Copyright 2020 The gRPC Authors

lidizheng · 2020-02-27T19:04:10Z

src/python/grpcio/grpc/experimental/aio/_server.py

        """
-        self._loop.create_task(self._server.shutdown(None))
+        if hasattr(self, '_server'):
+            self._loop.create_task(self._server.shutdown(None))


ZHmao · 2020-02-28T14:09:24Z

Thanks for all your reviews.

lidizheng

The comment history got hidden by GitHub. Raise a comment that I forget to follow-up.

lidizheng · 2020-02-28T19:02:29Z

src/python/grpcio_tests/tests_aio/unit/server_interceptor_test.py

+        return await self._fn(continuation, handler_call_details)
+
+
+def _filter_server_interceptor(condition: Callable,


I missed this one in my last round of review. By adding type annotation, I mean what is the argument it accepts and returns. You can do this like:

def atoi(a: str) -> int: return int(a) atoi: Callable[[str], int]

Also, if possible, please annotate all other arguments in this file, not only this function.

ZHmao · 2020-02-29T11:27:43Z

I have added type annotation for server_interceptor_test.py, and changed the return type of continuation from grpc.RpcMethodHandler to Awaitable[grpc.RpcMethodHandler] [1].

But it failed when I ran python -m pytype .
The error message:

in _filter_server_interceptor: Function _GenericInterceptor.__init__ was called with the wrong arguments [wrong-arg-types]
  Expected: (self, fn: Callable[[Callable[[grpc.HandlerCallDetails], Awaitable[grpc.RpcMethodHandler]], grpc.HandlerCallDetails], Awaitable[grpc.RpcMethodHandler]])
  Actually passed: (self, fn: Callable[[Callable[[grpc.HandlerCallDetails], Awaitable[grpc.RpcMethodHandler]], grpc.HandlerCallDetails], grpc.RpcMethodHandler])

[1] https://github.com/grpc/grpc/pull/22032/files#diff-173e8bd5d2219ba109f86237680068a4R41-R42

cc @lidizheng

lidizheng · 2020-03-02T21:13:24Z

@ZHmao I'm not sure I understand the question here. Did you mean the pytype has a deduction error on the types? In that case, you can post it as an issue to pytype. Otherwise, the error message seems clear that there might be a confusion in the implementation.

(As I said, complex callbacks are usually confusing and needs proper typing.)

ZHmao · 2020-03-03T14:33:55Z

I have written an example code and it shows the same error:
in go: Function bar was called with the wrong arguments [wrong-arg-types] Expected: (fun: Callable[[str], Awaitable[str]]) Actually passed: (fun: Callable[[str], str])

from typing import Callable, Awaitable


async def foo(a: str) -> str:
    return a


async def bar(fun: Callable[[str], Awaitable[str]]) -> str:
    return await fun('a')


async def go() -> None:
    await bar(foo)

I don't know what's wrong, do you have any advice?

lidizheng · 2020-03-03T18:14:47Z

@ZHmao Thanks for the convincing code snippet, good find! It looks like pytype has a bug, that it unable to express async functions correctly. In the generated .pyi file, it recognized that foo is an async function that returns a coroutine. However, when you passes foo as an argument, pytype complains.

# Generated .pyi file
def bar(fun: Callable[[str], Awaitable[str]]) -> Coroutine[Any, Any, str]: ...
def foo(a: str) -> Coroutine[Any, Any, str]: ...
def go() -> Coroutine[Any, Any, None]: ...

If you have some free cycle and want to be a contributor of pytype, feel free to open an issue or post a PR directly to https://github.com/google/pytype. You can find their unit test here: https://github.com/google/pytype/blob/master/pytype/tests/py3/test_coroutine.py.

On the other hand, to move this PR forward, there are two (hacky) ways to solve the pytype complain.

Use Union for the return value: Union[grpc.RpcMethodHandler, Awaitable[grpc.RpcMethodHandler]];
Use Any for the return value.

@pfreixes Have you encountered this bug before?

ZHmao · 2020-03-04T13:18:45Z

I'll use Union to move this PR forward, and open an issue or post a PR on pytype later.

ZHmao · 2020-03-04T15:44:36Z

FYI, when I tried to fix this issue with Union, it failed again. Then I wrote another example more like ours.😅

from typing import Callable, Awaitable, Union


async def foo(a: str) -> str:
    return a


class Bar:

    def __init__(self, fun: Callable[[str], Union[Awaitable[str], str]]) -> None:
        self.fun = fun

    async def bar(self) -> str:
        return await self.fun('a')


async def go() -> None:
    Bar(foo)

Error message:

in bar: bad option in return type [bad-return-type]
           Expected: Awaitable
  Actually returned: str

But type Any is OK in this situation. So I'll use Any to move this PR forward.

cc @lidizheng @pfreixes

lidizheng · 2020-03-04T18:09:27Z

@ZHmao Thanks for the update. This definitely looks like an issue for pytype.

ZHmao · 2020-03-05T01:16:46Z

Can we merge it? Or I need to make all checks pass before we can merge it?

lidizheng · 2020-03-05T18:26:47Z

Known failures: #22208 #19834

ZHmao and others added 7 commits December 14, 2019 16:24

Merge pull request #1 from grpc/master

d6a06e7

Sync with grpc repo

Merge pull request #3 from grpc/master

2e56849

sync

Merge pull request #4 from grpc/master

cace1d7

Sync with grpc repo

Merge pull request #5 from grpc/master

4cbe126

Sync with grpc repo

Merge pull request #6 from grpc/master

9177d84

Sync with grpc

Merge pull request #7 from grpc/master

8072fcb

Sync with grpc

Implement server interceptor for unary unary call

6fef565

rmariano reviewed Feb 17, 2020

View reviewed changes

pfreixes requested changes Feb 18, 2020

View reviewed changes

pfreixes self-assigned this Feb 18, 2020

pfreixes added lang/Python release notes: yes Indicates if PR needs to be in release notes kind/enhancement labels Feb 18, 2020

lidizheng requested review from gnossen and lidizheng February 18, 2020 23:07

lidizheng reviewed Feb 19, 2020

View reviewed changes

lidizheng assigned gnossen and lidizheng Feb 19, 2020

gnossen approved these changes Feb 19, 2020

View reviewed changes

pfreixes added the kokoro:run label Feb 19, 2020

Merge branch 'master' into implement-server-interceptor-for-unary-una…

958fb08

…ry-call

grpc-kokoro removed the kokoro:run label Feb 21, 2020

upate test

128d030

pfreixes reviewed Feb 25, 2020

View reviewed changes

src/python/grpcio/grpc/experimental/aio/_server.py Outdated Show resolved Hide resolved

rmariano approved these changes Feb 27, 2020

View reviewed changes

pfreixes approved these changes Feb 27, 2020

View reviewed changes

lidizheng approved these changes Feb 27, 2020

View reviewed changes

fix sanity checks

26985fd

lidizheng added the kokoro:run label Feb 28, 2020

grpc-kokoro removed the kokoro:run label Feb 28, 2020

lidizheng reviewed Feb 28, 2020

View reviewed changes

add type annotation

701ec0b

lidizheng added the kokoro:run label Mar 2, 2020

grpc-kokoro removed the kokoro:run label Mar 2, 2020

fix type annotation

09168c5

lidizheng added the kokoro:run label Mar 4, 2020

grpc-kokoro removed the kokoro:run label Mar 4, 2020

lidizheng merged commit 87d01bf into grpc:master Mar 5, 2020

github-actions bot mentioned this pull request Feb 27, 2026

🌱 CNCF mission generation 2026-02-27 kubestellar/console-kb#6

Closed

		self.fail("Callback was not called")


		class _LoggingServerInterceptor(aio.ServerInterceptor):

		return _GenericServerInterceptor(intercept_service)


		class TestServerInterceptor(AioTestBase):

		return await self._fn(continuation, handler_call_details)


		def _filter_server_interceptor(condition, interceptor):

	# Copyright 2019 The gRPC Authors.
	# Copyright 2020 The gRPC Authors

Conversation

ZHmao commented Feb 15, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZHmao Feb 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pfreixes left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pfreixes Feb 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lidizheng left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gnossen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mehrdada commented Feb 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lidizheng commented Feb 20, 2020

Uh oh!

pfreixes commented Feb 20, 2020

Uh oh!

mehrdada commented Feb 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZHmao commented Feb 21, 2020

Uh oh!

pfreixes commented Feb 21, 2020

Uh oh!

mehrdada commented Feb 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pfreixes Feb 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lidizheng Feb 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pfreixes commented Feb 26, 2020

Uh oh!

ZHmao commented Feb 27, 2020

ZHmao Feb 18, 2020 •

edited

Loading

pfreixes Feb 18, 2020 •

edited

Loading

mehrdada commented Feb 20, 2020 •

edited

Loading

mehrdada commented Feb 21, 2020 •

edited

Loading

mehrdada commented Feb 22, 2020 •

edited

Loading

pfreixes Feb 25, 2020 •

edited

Loading

lidizheng Feb 26, 2020 •

edited

Loading

ZHmao commented Feb 29, 2020 •

edited

Loading

ZHmao commented Mar 4, 2020 •

edited

Loading