From f7a563014fb5aff59f63c4b4853eddd49c653828 Mon Sep 17 00:00:00 2001 From: Michael Stella Date: Tue, 6 Apr 2021 22:04:52 -0400 Subject: [PATCH 1/3] Update Context wrapper class There are a few cases where one needs to dig into `grpc.ServicerContext` objects, and these fields were missing from our wrapper, which can cause issues with implmementation. Yes, they're private attributes, but there's no way to get the data they hold otherwise, so we should properly map them so our wrapper Context behaves like a real one does. --- CHANGELOG.md | 2 ++ .../instrumentation/grpc/_server.py | 24 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bba0d8c440..521d6cb855 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#387](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/387)) - Update redis instrumentation to follow semantic conventions ([#403](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/403)) +- Update gRPC instrumentation to better wrap server context + ([xxx]((https://github.com/open-telemetry/opentelemetry-python-contrib/pull/xxx)) ### Added - `opentelemetry-instrumentation-urllib3` Add urllib3 instrumentation diff --git a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py index 5321964a19..46d1f1ba9c 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py @@ -71,6 +71,30 @@ def __init__(self, servicer_context, active_span): self.details = None super().__init__() + @property + def _state(self): + return self._servicer_context._state + + @_state.setter + def _state(self, value): + self._servicer_context._state = value + + @property + def _rpc_event(self): + return self._servicer_context._rpc_event + + @_rpc_event.setter + def _rpc_event(self, value): + self._servicer_context._rpc_event = value + + @property + def _request_deserializer(self): + return self._servicer_context._request_deserializer + + @_request_deserializer.setter + def _request_deserializer(self, value): + self._servicer_context._request_deserializer = value + def is_active(self, *args, **kwargs): return self._servicer_context.is_active(*args, **kwargs) From a6b03a6fd7856311f8e1b56fc7fa40e7365773dd Mon Sep 17 00:00:00 2001 From: Michael Stella Date: Tue, 6 Apr 2021 22:24:53 -0400 Subject: [PATCH 2/3] changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 521d6cb855..1ae6b1c204 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Update redis instrumentation to follow semantic conventions ([#403](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/403)) - Update gRPC instrumentation to better wrap server context - ([xxx]((https://github.com/open-telemetry/opentelemetry-python-contrib/pull/xxx)) + ([#420](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/420)) ### Added - `opentelemetry-instrumentation-urllib3` Add urllib3 instrumentation From 64f507fb2decf69ffc61a3b08cd890f72fb03b0e Mon Sep 17 00:00:00 2001 From: Michael Stella Date: Thu, 8 Apr 2021 12:27:45 -0400 Subject: [PATCH 3/3] Rework this to use `__getattr__` Based on feedback from @ocelotl. --- .../instrumentation/grpc/_server.py | 25 ++----------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py index 46d1f1ba9c..39ff47f278 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_server.py @@ -71,29 +71,8 @@ def __init__(self, servicer_context, active_span): self.details = None super().__init__() - @property - def _state(self): - return self._servicer_context._state - - @_state.setter - def _state(self, value): - self._servicer_context._state = value - - @property - def _rpc_event(self): - return self._servicer_context._rpc_event - - @_rpc_event.setter - def _rpc_event(self, value): - self._servicer_context._rpc_event = value - - @property - def _request_deserializer(self): - return self._servicer_context._request_deserializer - - @_request_deserializer.setter - def _request_deserializer(self, value): - self._servicer_context._request_deserializer = value + def __getattr__(self, attr): + return getattr(self._servicer_context, attr) def is_active(self, *args, **kwargs): return self._servicer_context.is_active(*args, **kwargs)