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 fails if client channel is used as a context manager #321

Closed
iredelmeier opened this issue Feb 8, 2021 · 2 comments · Fixed by #363
Closed
Labels
backlog bug Something isn't working help wanted Extra attention is needed

Comments

@iredelmeier
Copy link
Member

Describe your environment

Verified on python 3.8.6 with opentelemetry 0.17b0

Steps to reproduce

Use a gRPC channel as a context manager.

For instance, this happens if you add instrumentation to the gRPC helloworld example client:

# Copyright 2015 gRPC authors.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#     http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""The Python implementation of the GRPC helloworld.Greeter client."""

from __future__ import print_function
import logging

import grpc

from opentelemetry import trace
from opentelemetry.instrumentation.grpc import GrpcInstrumentorClient
from opentelemetry.sdk.trace import TracerProvider
from opentelemetry.sdk.trace.export import (
    ConsoleSpanExporter,
    SimpleExportSpanProcessor,
)

import helloworld_pb2
import helloworld_pb2_grpc

trace.set_tracer_provider(TracerProvider())
trace.get_tracer_provider().add_span_processor(
    SimpleExportSpanProcessor(ConsoleSpanExporter())
)

GrpcInstrumentorClient().instrument()


def run():
    # NOTE(gRPC Python Team): .close() is possible on a channel and should be
    # used in circumstances in which the with statement does not fit the needs
    # of the code.
    with grpc.insecure_channel('localhost:50051') as channel:
        stub = helloworld_pb2_grpc.GreeterStub(channel)
        response = stub.SayHello(helloworld_pb2.HelloRequest(name='you'))
        print("Greeter client received: " + response.message)


if __name__ == '__main__':
    logging.basicConfig()
    run()

What is the expected behavior?

The channel should be usable as normal, except now with instrumentation

What is the actual behavior?

❯ python greeter_client.py
Traceback (most recent call last):
  File "greeter_client.py", line 52, in <module>
    run()
  File "greeter_client.py", line 44, in run
    with grpc.insecure_channel('localhost:50051') as channel:
  File "/home/isobel/workspace/tmp/otlp/grpc/lib/python3.8/site-packages/opentelemetry/instrumentation/grpc/grpcext/_interceptor.py", line 235, in __enter__
    raise NotImplementedError()
NotImplementedError

Additional context

LMK and I can make a PR

@iredelmeier iredelmeier added the bug Something isn't working label Feb 8, 2021
@alertedsnake
Copy link
Contributor

I did a lot of testing with this (see recent PRs) but apparently never once tried a channel as a context manager - all of my production code saves the channel in a client class instead. Good catch!

ocelotl added a commit to ocelotl/opentelemetry-python-contrib that referenced this issue Mar 3, 2021
ocelotl added a commit to ocelotl/opentelemetry-python-contrib that referenced this issue Mar 3, 2021
@github-actions
Copy link

github-actions bot commented Apr 6, 2021

This issue was marked stale due to lack of activity. It will be closed in 30 days.

@owais owais added the help wanted Extra attention is needed label Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants