From 5076087b7682a45e4a62f70cae8ef2b0e7fb6de2 Mon Sep 17 00:00:00 2001 From: Michael Stella Date: Thu, 10 Dec 2020 18:04:50 -0500 Subject: [PATCH 01/11] Add rpc fields in the spec to client interceptor --- .../instrumentation/grpc/_client.py | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_client.py b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_client.py index b38fd4d511..9da869c1cf 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_client.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_client.py @@ -102,8 +102,16 @@ def __init__(self, tracer, exporter, interval): ) def _start_span(self, method): + service, meth = method.lstrip("/").split("/", 1) + attributes = { + "rpc.system": "grpc", + "rpc.grpc.status_code": grpc.StatusCode.OK.value[0], + "rpc.method": meth, + "rpc.service": service, + } + return self._tracer.start_as_current_span( - name=method, kind=trace.SpanKind.CLIENT + name=method, kind=trace.SpanKind.CLIENT, attributes=attributes ) # pylint:disable=no-self-use @@ -133,6 +141,7 @@ def _trace_result(self, guarded_span, rpc_info, result, client_info): self._metrics_recorder.record_bytes_in( response.ByteSize(), client_info.full_method ) + return result def _start_guarded_span(self, *args, **kwargs): @@ -175,11 +184,12 @@ def intercept_unary(self, request, metadata, client_info, invoker): try: result = invoker(request, metadata) - except grpc.RpcError: + except grpc.RpcError as e: guarded_span.generated_span.set_status( Status(StatusCode.ERROR) ) - raise + guarded_span.set_attribute("rpc.grpc.status_code", e.code().value[0]) + raise e return self._trace_result( guarded_span, rpc_info, result, client_info @@ -230,9 +240,10 @@ def _intercept_server_stream( response.ByteSize(), client_info.full_method ) yield response - except grpc.RpcError: + except grpc.RpcError as e: span.set_status(Status(StatusCode.ERROR)) - raise + guarded_span.set_attribute("rpc.grpc.status_code", e.code().value[0]) + raise e def intercept_stream( self, request_or_iterator, metadata, client_info, invoker @@ -268,11 +279,12 @@ def intercept_stream( try: result = invoker(request_or_iterator, metadata) - except grpc.RpcError: + except grpc.RpcError as e: guarded_span.generated_span.set_status( Status(StatusCode.ERROR) ) - raise + guarded_span.set_attribute("rpc.grpc.status_code", e.code().value[0]) + raise e return self._trace_result( guarded_span, rpc_info, result, client_info From 28bacd14c720c66ffa0c00289f5191334ac7ab6f Mon Sep 17 00:00:00 2001 From: Michael Stella Date: Fri, 18 Dec 2020 16:39:45 -0500 Subject: [PATCH 02/11] Rework how the client instruments --- .../instrumentation/grpc/__init__.py | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/__init__.py b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/__init__.py index f963a2102f..8d718862dd 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/__init__.py @@ -185,29 +185,36 @@ class GrpcInstrumentorClient(BaseInstrumentor): """ + # Figures out which channel type we need to wrap + def _which_channel(self, kwargs): + # handle legacy argument + if "channel_type" in kwargs: + if kwargs.get("channel_type") == "secure": + return ("secure_channel", ) + else: + return ("insecure_channel", ) + + # handle modern arguments + types = [] + for ctype in ("secure_channel", "insecure_channel"): + if kwargs.get(ctype, True): + types.append(ctype) + + return types + def _instrument(self, **kwargs): exporter = kwargs.get("exporter", None) interval = kwargs.get("interval", 30) - if kwargs.get("channel_type") == "secure": - _wrap( - "grpc", - "secure_channel", - partial(self.wrapper_fn, exporter, interval), - ) - - else: + for ctype in self._which_channel(kwargs): _wrap( "grpc", - "insecure_channel", + ctype, partial(self.wrapper_fn, exporter, interval), ) def _uninstrument(self, **kwargs): - if kwargs.get("channel_type") == "secure": - unwrap(grpc, "secure_channel") - - else: - unwrap(grpc, "insecure_channel") + for ctype in self._which_channel(kwargs): + unwrap(grpc, ctype) def wrapper_fn( self, exporter, interval, original_func, instance, args, kwargs From 83f27d2c766b5fee25c179b6a0a4588357c742f2 Mon Sep 17 00:00:00 2001 From: Michael Stella Date: Fri, 18 Dec 2020 16:58:13 -0500 Subject: [PATCH 03/11] grpc client fix, additional tests Add tests for the new attributes --- .../instrumentation/grpc/_client.py | 12 ++++-- .../tests/test_client_interceptor.py | 40 +++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_client.py b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_client.py index 9da869c1cf..930c86d332 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_client.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_client.py @@ -188,7 +188,10 @@ def intercept_unary(self, request, metadata, client_info, invoker): guarded_span.generated_span.set_status( Status(StatusCode.ERROR) ) - guarded_span.set_attribute("rpc.grpc.status_code", e.code().value[0]) + guarded_span.generated_span.set_attribute( + "rpc.grpc.status_code", + e.code().value[0] + ) raise e return self._trace_result( @@ -242,7 +245,7 @@ def _intercept_server_stream( yield response except grpc.RpcError as e: span.set_status(Status(StatusCode.ERROR)) - guarded_span.set_attribute("rpc.grpc.status_code", e.code().value[0]) + span.set_attribute("rpc.grpc.status_code", e.code().value[0]) raise e def intercept_stream( @@ -283,7 +286,10 @@ def intercept_stream( guarded_span.generated_span.set_status( Status(StatusCode.ERROR) ) - guarded_span.set_attribute("rpc.grpc.status_code", e.code().value[0]) + guarded_span.generated_span.set_attribute( + "rpc.grpc.status_code", + e.code().value[0], + ) raise e return self._trace_result( diff --git a/instrumentation/opentelemetry-instrumentation-grpc/tests/test_client_interceptor.py b/instrumentation/opentelemetry-instrumentation-grpc/tests/test_client_interceptor.py index f272a90240..56ba843c6e 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/tests/test_client_interceptor.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/tests/test_client_interceptor.py @@ -116,6 +116,16 @@ def test_unary_unary(self): self._verify_success_records(8, 8, "/GRPCTestServer/SimpleMethod") + self.assert_span_has_attributes( + span, + { + "rpc.method": "SimpleMethod", + "rpc.service": "GRPCTestServer", + "rpc.system": "grpc", + "rpc.grpc.status_code": grpc.StatusCode.OK.value[0], + }, + ) + def test_unary_stream(self): server_streaming_method(self._stub) spans = self.memory_exporter.get_finished_spans() @@ -134,6 +144,16 @@ def test_unary_stream(self): 8, 40, "/GRPCTestServer/ServerStreamingMethod" ) + self.assert_span_has_attributes( + span, + { + "rpc.method": "ServerStreamingMethod", + "rpc.service": "GRPCTestServer", + "rpc.system": "grpc", + "rpc.grpc.status_code": grpc.StatusCode.OK.value[0], + }, + ) + def test_stream_unary(self): client_streaming_method(self._stub) spans = self.memory_exporter.get_finished_spans() @@ -152,6 +172,16 @@ def test_stream_unary(self): 40, 8, "/GRPCTestServer/ClientStreamingMethod" ) + self.assert_span_has_attributes( + span, + { + "rpc.method": "ClientStreamingMethod", + "rpc.service": "GRPCTestServer", + "rpc.system": "grpc", + "rpc.grpc.status_code": grpc.StatusCode.OK.value[0], + }, + ) + def test_stream_stream(self): bidirectional_streaming_method(self._stub) spans = self.memory_exporter.get_finished_spans() @@ -172,6 +202,16 @@ def test_stream_stream(self): 40, 40, "/GRPCTestServer/BidirectionalStreamingMethod" ) + self.assert_span_has_attributes( + span, + { + "rpc.method": "BidirectionalStreamingMethod", + "rpc.service": "GRPCTestServer", + "rpc.system": "grpc", + "rpc.grpc.status_code": grpc.StatusCode.OK.value[0], + }, + ) + def _verify_error_records(self, method): # pylint: disable=protected-access,no-member self.channel._interceptor.controller.tick() From 546d623fd03a59c936e95c087b6e98fab675aceb Mon Sep 17 00:00:00 2001 From: Michael Stella Date: Thu, 10 Dec 2020 17:20:37 -0500 Subject: [PATCH 04/11] gRPC client instrumentation docs fixes Bugfix for docs that led me astray for a good half hour :) --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38e65c98d6..3401f3d350 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,6 +74,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#246](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/246)) - Update TraceState to adhere to specs ([#276](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/276)) +- `opentelemetry-instrumentation-grpc` Updated client attributes, added tests, fixed examples, docs + ([#269](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/269)) ### Removed - Remove Configuration From 82ee18fffc6c1e2c245bcb57f0d96b186705ee05 Mon Sep 17 00:00:00 2001 From: Michael Stella Date: Fri, 18 Dec 2020 17:15:25 -0500 Subject: [PATCH 05/11] run black --- .../src/opentelemetry/instrumentation/grpc/_client.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_client.py b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_client.py index 930c86d332..5da7c55534 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_client.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_client.py @@ -189,8 +189,7 @@ def intercept_unary(self, request, metadata, client_info, invoker): Status(StatusCode.ERROR) ) guarded_span.generated_span.set_attribute( - "rpc.grpc.status_code", - e.code().value[0] + "rpc.grpc.status_code", e.code().value[0] ) raise e @@ -245,7 +244,9 @@ def _intercept_server_stream( yield response except grpc.RpcError as e: span.set_status(Status(StatusCode.ERROR)) - span.set_attribute("rpc.grpc.status_code", e.code().value[0]) + span.set_attribute( + "rpc.grpc.status_code", e.code().value[0] + ) raise e def intercept_stream( @@ -287,8 +288,7 @@ def intercept_stream( Status(StatusCode.ERROR) ) guarded_span.generated_span.set_attribute( - "rpc.grpc.status_code", - e.code().value[0], + "rpc.grpc.status_code", e.code().value[0], ) raise e From b911c529da4c27c0f1b59b807a23ccd1ff3a336a Mon Sep 17 00:00:00 2001 From: Michael Stella Date: Fri, 18 Dec 2020 21:23:24 -0500 Subject: [PATCH 06/11] Why do I always have to run black twice? --- .../src/opentelemetry/instrumentation/grpc/__init__.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/__init__.py b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/__init__.py index 8d718862dd..d31b1ffec3 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/__init__.py @@ -190,9 +190,9 @@ def _which_channel(self, kwargs): # handle legacy argument if "channel_type" in kwargs: if kwargs.get("channel_type") == "secure": - return ("secure_channel", ) + return ("secure_channel",) else: - return ("insecure_channel", ) + return ("insecure_channel",) # handle modern arguments types = [] @@ -207,9 +207,7 @@ def _instrument(self, **kwargs): interval = kwargs.get("interval", 30) for ctype in self._which_channel(kwargs): _wrap( - "grpc", - ctype, - partial(self.wrapper_fn, exporter, interval), + "grpc", ctype, partial(self.wrapper_fn, exporter, interval), ) def _uninstrument(self, **kwargs): From 6aa158e24ad697384c506f4ad98bcd78d0fdbca1 Mon Sep 17 00:00:00 2001 From: Michael Stella Date: Thu, 24 Dec 2020 00:44:37 -0500 Subject: [PATCH 07/11] Fix client metric labels The docs on metric labels suggests that they should probably be strings, and all others I can find are strings, and so these ought to be also. Otherwise, some of the exporters/processors have to handle things specifically, and not all of these come out as nice as could be when you `str()` them. I've also made sure to use the `StatusCode` name, as that's the interesting thing. Finally, there's no need to report specifically that `error=false`, so I've removed that tag. --- .../instrumentation/grpc/_utilities.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_utilities.py b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_utilities.py index 9de95c9724..da730687e9 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_utilities.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_utilities.py @@ -72,33 +72,32 @@ def __init__(self, meter, span_kind): def record_bytes_in(self, bytes_in, method): if self._meter: - labels = {"method": method} + labels = {"rpc.method": method} self._bytes_in.add(bytes_in, labels) def record_bytes_out(self, bytes_out, method): if self._meter: - labels = {"method": method} + labels = {"rpc.method": method} self._bytes_out.add(bytes_out, labels) @contextmanager def record_latency(self, method): start_time = time() labels = { - "method": method, - "status_code": grpc.StatusCode.OK, # pylint:disable=no-member + "rpc.system": "grpc", + "rpc.grpc.method": method, + "rpc.status_code": grpc.StatusCode.OK.name, } try: yield labels except grpc.RpcError as exc: # pylint:disable=no-member if self._meter: # pylint: disable=no-member - labels["status_code"] = exc.code() + labels["rpc.status_code"] = exc.code().name self._error_count.add(1, labels) - labels["error"] = True + labels["error"] = "true" raise finally: if self._meter: - if "error" not in labels: - labels["error"] = False elapsed_time = (time() - start_time) * 1000 self._duration.record(elapsed_time, labels) From 6a0dd0f19d6ac12006b95496540e3f54861ed35b Mon Sep 17 00:00:00 2001 From: Michael Stella Date: Thu, 24 Dec 2020 09:35:17 -0500 Subject: [PATCH 08/11] Update/fix tests, fix code Hey, good thing there are tests. Also, I did the `sorted()` thing in here so future us don't have to think about the ordering of these when writing new tests :) --- .../instrumentation/grpc/_utilities.py | 6 +-- .../tests/test_client_interceptor.py | 46 ++++++++++--------- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_utilities.py b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_utilities.py index da730687e9..078a98b9e4 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_utilities.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_utilities.py @@ -84,16 +84,16 @@ def record_bytes_out(self, bytes_out, method): def record_latency(self, method): start_time = time() labels = { + "rpc.method": method, "rpc.system": "grpc", - "rpc.grpc.method": method, - "rpc.status_code": grpc.StatusCode.OK.name, + "rpc.grpc.status_code": grpc.StatusCode.OK.name, } try: yield labels except grpc.RpcError as exc: # pylint:disable=no-member if self._meter: # pylint: disable=no-member - labels["rpc.status_code"] = exc.code().name + labels["rpc.grpc.status_code"] = exc.code().name self._error_count.add(1, labels) labels["error"] = "true" raise diff --git a/instrumentation/opentelemetry-instrumentation-grpc/tests/test_client_interceptor.py b/instrumentation/opentelemetry-instrumentation-grpc/tests/test_client_interceptor.py index 56ba843c6e..1d7b0ee149 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/tests/test_client_interceptor.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/tests/test_client_interceptor.py @@ -73,21 +73,21 @@ def _verify_success_records(self, num_bytes_out, num_bytes_in, method): self.assertIsNotNone(bytes_out) self.assertEqual(bytes_out.instrument.name, "grpcio/client/bytes_out") - self.assertEqual(bytes_out.labels, (("method", method),)) + self.assertEqual(bytes_out.labels, (("rpc.method", method),)) self.assertIsNotNone(bytes_in) self.assertEqual(bytes_in.instrument.name, "grpcio/client/bytes_in") - self.assertEqual(bytes_in.labels, (("method", method),)) + self.assertEqual(bytes_in.labels, (("rpc.method", method),)) self.assertIsNotNone(duration) self.assertEqual(duration.instrument.name, "grpcio/client/duration") - self.assertEqual( - duration.labels, - ( - ("error", False), - ("method", method), - ("status_code", grpc.StatusCode.OK), - ), + self.assertSequenceEqual( + sorted(duration.labels), + [ + ("rpc.grpc.status_code", grpc.StatusCode.OK.name), + ("rpc.method", method), + ("rpc.system", "grpc"), + ], ) self.assertEqual(type(bytes_out.aggregator), SumAggregator) @@ -235,22 +235,24 @@ def _verify_error_records(self, method): self.assertIsNotNone(duration) self.assertEqual(errors.instrument.name, "grpcio/client/errors") - self.assertEqual( - errors.labels, - ( - ("method", method), - ("status_code", grpc.StatusCode.INVALID_ARGUMENT), - ), + self.assertSequenceEqual( + sorted(errors.labels), + sorted(( + ("rpc.grpc.status_code", grpc.StatusCode.INVALID_ARGUMENT.name), + ("rpc.method", method), + ("rpc.system", "grpc"), + )), ) self.assertEqual(errors.aggregator.checkpoint, 1) - self.assertEqual( - duration.labels, - ( - ("error", True), - ("method", method), - ("status_code", grpc.StatusCode.INVALID_ARGUMENT), - ), + self.assertSequenceEqual( + sorted(duration.labels), + sorted(( + ("error", "true"), + ("rpc.method", method), + ("rpc.system", "grpc"), + ("rpc.grpc.status_code", grpc.StatusCode.INVALID_ARGUMENT.name), + )), ) def test_error_simple(self): From 9c3aadafbaf8bb2efa1a8bf1b9fcae1a8dc06348 Mon Sep 17 00:00:00 2001 From: Michael Stella Date: Thu, 24 Dec 2020 09:55:51 -0500 Subject: [PATCH 09/11] black --- .../tests/test_client_interceptor.py | 32 ++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-grpc/tests/test_client_interceptor.py b/instrumentation/opentelemetry-instrumentation-grpc/tests/test_client_interceptor.py index 1d7b0ee149..a13a9a99b5 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/tests/test_client_interceptor.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/tests/test_client_interceptor.py @@ -237,22 +237,32 @@ def _verify_error_records(self, method): self.assertEqual(errors.instrument.name, "grpcio/client/errors") self.assertSequenceEqual( sorted(errors.labels), - sorted(( - ("rpc.grpc.status_code", grpc.StatusCode.INVALID_ARGUMENT.name), - ("rpc.method", method), - ("rpc.system", "grpc"), - )), + sorted( + ( + ( + "rpc.grpc.status_code", + grpc.StatusCode.INVALID_ARGUMENT.name, + ), + ("rpc.method", method), + ("rpc.system", "grpc"), + ) + ), ) self.assertEqual(errors.aggregator.checkpoint, 1) self.assertSequenceEqual( sorted(duration.labels), - sorted(( - ("error", "true"), - ("rpc.method", method), - ("rpc.system", "grpc"), - ("rpc.grpc.status_code", grpc.StatusCode.INVALID_ARGUMENT.name), - )), + sorted( + ( + ("error", "true"), + ("rpc.method", method), + ("rpc.system", "grpc"), + ( + "rpc.grpc.status_code", + grpc.StatusCode.INVALID_ARGUMENT.name, + ), + ) + ), ) def test_error_simple(self): From abda221a67d44f5d2d0452e4b323c4d631430929 Mon Sep 17 00:00:00 2001 From: Michael Stella Date: Thu, 14 Jan 2021 16:21:14 -0500 Subject: [PATCH 10/11] Appease the linter This code passed before, so something clearly changed. --- .../instrumentation/grpc/__init__.py | 3 +-- .../instrumentation/grpc/_client.py | 18 +++++++++--------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/__init__.py b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/__init__.py index d31b1ffec3..af82ae2be7 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/__init__.py @@ -191,8 +191,7 @@ def _which_channel(self, kwargs): if "channel_type" in kwargs: if kwargs.get("channel_type") == "secure": return ("secure_channel",) - else: - return ("insecure_channel",) + return ("insecure_channel",) # handle modern arguments types = [] diff --git a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_client.py b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_client.py index 5da7c55534..3cf4c74df7 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_client.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/_client.py @@ -184,14 +184,14 @@ def intercept_unary(self, request, metadata, client_info, invoker): try: result = invoker(request, metadata) - except grpc.RpcError as e: + except grpc.RpcError as err: guarded_span.generated_span.set_status( Status(StatusCode.ERROR) ) guarded_span.generated_span.set_attribute( - "rpc.grpc.status_code", e.code().value[0] + "rpc.grpc.status_code", err.code().value[0] ) - raise e + raise err return self._trace_result( guarded_span, rpc_info, result, client_info @@ -242,12 +242,12 @@ def _intercept_server_stream( response.ByteSize(), client_info.full_method ) yield response - except grpc.RpcError as e: + except grpc.RpcError as err: span.set_status(Status(StatusCode.ERROR)) span.set_attribute( - "rpc.grpc.status_code", e.code().value[0] + "rpc.grpc.status_code", err.code().value[0] ) - raise e + raise err def intercept_stream( self, request_or_iterator, metadata, client_info, invoker @@ -283,14 +283,14 @@ def intercept_stream( try: result = invoker(request_or_iterator, metadata) - except grpc.RpcError as e: + except grpc.RpcError as err: guarded_span.generated_span.set_status( Status(StatusCode.ERROR) ) guarded_span.generated_span.set_attribute( - "rpc.grpc.status_code", e.code().value[0], + "rpc.grpc.status_code", err.code().value[0], ) - raise e + raise err return self._trace_result( guarded_span, rpc_info, result, client_info From 3abab947e8de89a6f446456f99fc35de069d6a04 Mon Sep 17 00:00:00 2001 From: Michael Stella Date: Thu, 4 Feb 2021 19:08:46 -0500 Subject: [PATCH 11/11] Be consistent with return type --- .../src/opentelemetry/instrumentation/grpc/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/__init__.py b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/__init__.py index af82ae2be7..eeffea1b87 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/__init__.py @@ -199,7 +199,7 @@ def _which_channel(self, kwargs): if kwargs.get(ctype, True): types.append(ctype) - return types + return tuple(types) def _instrument(self, **kwargs): exporter = kwargs.get("exporter", None)