Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

opentelemetry-instrumentation-grpc - code()/details()-bug in ServicerContext #855

Closed
CoLa5 opened this issue Jan 11, 2022 · 2 comments
Closed
Labels
bug Something isn't working

Comments

@CoLa5
Copy link
Contributor

CoLa5 commented Jan 11, 2022

Describe your environment

python version: 3.7.12
grpcio: 1.43.0

Steps to reproduce

Call inside of a RPC-method-implementation of a grpc service the methods:

  • context.code()
  • context.details()

which are supported since grpcio v1.38.0 (s. here).

Example:

Add the functions into e.g. helloworld/greeter_server.py:

class Greeter(helloworld_pb2_grpc.GreeterServicer):

    def SayHello(self, request, context):
        print("code before: %r" % context.code())
        context.set_code(grpc.StatusCode.OK)
        print("code after: %r" % context.code())

        print("details before: %r" % context.details())
        context.set_details("test")
        print("details after: %r" % context.details())

        return helloworld_pb2.HelloReply(message='Hello, %s!' % request.name)

What is the expected behavior?

This should print:

code before: None
code after: <StatusCode.OK: (0, 'ok')> 
details before: None
details after: 'test'

What is the actual behavior?

There are TypeError raised:

TypeError: 'StatusCode' object is not callable
TypeError: 'NoneType' object is not callable

Additional context

The _OpenTelemetryServicerContext inside of _server.py implements code and details as attributes while the official grpcio API implements them as methods.

Proposed Bugfix

Stick with the attributes to support older grpcio versions, but make them private. Furthermore, add code() and details() with RuntimeError:

class _OpenTelemetryServicerContext(grpc.ServicerContext):

    def __init__(self, servicer_context, active_span):
        self._servicer_context = servicer_context
        self._active_span = active_span
        self._code = grpc.StatusCode.OK
        self._details = None
        super().__init__()

    # ...

    def abort(self, code, details):
        self._code = code
        self._details = details
        self._active_span.set_attribute(
            SpanAttributes.RPC_GRPC_STATUS_CODE, code.value[0]
        )
        self._active_span.set_status(
            Status(
                status_code=StatusCode.ERROR,
                description=f"{code}:{details}",
            )
        )
        return self._servicer_context.abort(code, details)

    def code(self):
        if not hasattr(self._servicer_context, "code"):
            raise RuntimeError(
                "code() is not supported with the installed version of grpcio"
            )
        return self._servicer_context.code()

    def set_code(self, code):
        self._code = code
        # use details if we already have it, otherwise the status description
        details = self._details or code.value[1]
        self._active_span.set_attribute(
            SpanAttributes.RPC_GRPC_STATUS_CODE, code.value[0]
        )
        if code != grpc.StatusCode.OK:
            self._active_span.set_status(
                Status(
                    status_code=StatusCode.ERROR,
                    description=f"{code}:{details}",
                )
            )
        return self._servicer_context.set_code(code)

    def details(self):
        if not hasattr(self._servicer_context, "details"):
            raise RuntimeError(
                "details() is not supported with the installed version of "
                "grpcio"
            )
        return self._servicer_context.details()

    def set_details(self, details):
        self._details = details
        if self._code != grpc.StatusCode.OK:
            self._active_span.set_status(
                Status(
                    status_code=StatusCode.ERROR,
                    description=f"{self._code}:{details}",
                )
            )
        return self._servicer_context.set_details(details)
@CoLa5 CoLa5 added the bug Something isn't working label Jan 11, 2022
@CoLa5 CoLa5 changed the title opentelemetry-instrumentation-grpc _OpenTelemetryServicerContex opentelemetry-instrumentation-grpc - code()/details()-bug in ServicerContext Jan 11, 2022
@huwcbjones
Copy link

huwcbjones commented Sep 13, 2022

I've also noticed that grpc.aio.ServicerContext::abort has details as an optional argument, whereas the otel servicer context mandates it.

https://github.com/grpc/grpc/blob/v1.38.0/src/python/grpcio/grpc/aio/_base_server.py#L172-L176

vs

CoLa5 added a commit to CoLa5/opentelemetry-python-contrib that referenced this issue Jan 13, 2023
@srikanthccv
Copy link
Member

Fixed in #1578

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants