fix(grpc): no package in method service (#2384)

* fix(grpc): add snapshot test for method service metadata

* handle empty method package

* revert change to host parsing

* add release note

* use async_mode

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

.circleci/config.yml

@@ -622,11 +622,12 @@ jobs:
           pattern: '^httplib_contrib'
 
   grpc:
-    <<: *contrib_job
+    <<: *machine_executor
     parallelism: 6
     steps:
       - run_test:
           pattern: "grpc"
+          snapshot: true
 
   molten:
     <<: *contrib_job

ddtrace/contrib/grpc/utils.py

@@ -19,7 +19,8 @@ def set_grpc_method_meta(span, method, method_kind):
     method_path = method
     method_package, method_service, method_name = parse_method_path(method_path)
     span._set_str_tag(constants.GRPC_METHOD_PATH_KEY, method_path)
-    span._set_str_tag(constants.GRPC_METHOD_PACKAGE_KEY, method_package)
+    if method_package is not None:
+        span._set_str_tag(constants.GRPC_METHOD_PACKAGE_KEY, method_package)
     span._set_str_tag(constants.GRPC_METHOD_SERVICE_KEY, method_service)
     span._set_str_tag(constants.GRPC_METHOD_NAME_KEY, method_name)
     span._set_str_tag(constants.GRPC_METHOD_KIND_KEY, method_kind)

releasenotes/notes/fix-grpc-method-93038a476c9e95aa.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    grpc: handle no package in fully qualified method

tests/contrib/grpc/test_grpc.py

@@ -3,6 +3,7 @@
 import grpc
 from grpc._grpcio_metadata import __version__ as _GRPC_VERSION
 from grpc.framework.foundation import logging_pool
+import pytest
 
 from ddtrace import Pin
 from ddtrace.constants import ANALYTICS_SAMPLE_RATE_KEY
@@ -12,6 +13,7 @@
 from ddtrace.contrib.grpc.patch import _unpatch_server
 from ddtrace.ext import errors
 from tests.utils import TracerTestCase
+from tests.utils import snapshot
 
 from .hello_pb2 import HelloReply
 from .hello_pb2 import HelloRequest
@@ -572,3 +574,40 @@ class NotFutureLike(object):
     assert span.duration is None
     _handle_response(span, FutureLike())
     assert span.duration is not None
+
+
+@pytest.fixture()
+def patch_grpc():
+    patch()
+    try:
+        yield
+    finally:
+        unpatch()
+
+
+class _UnaryUnaryRpcHandler(grpc.GenericRpcHandler):
+    def __init__(self, handler):
+        self._handler = handler
+
+    def service(self, handler_call_details):
+        return grpc.unary_unary_rpc_method_handler(self._handler)
+
+
+@snapshot(ignores=["meta.grpc.port"])
+def test_method_service(patch_grpc):
+    def handler(request, context):
+        return b""
+
+    server = grpc.server(
+        logging_pool.pool(1),
+        options=(("grpc.so_reuseport", 0),),
+    )
+    port = server.add_insecure_port("[::]:0")
+    channel = grpc.insecure_channel("localhost:{}".format(port))
+    server.add_generic_rpc_handlers((_UnaryUnaryRpcHandler(handler),))
+    try:
+        server.start()
+        channel.unary_unary("/Servicer/Handler")(b"request")
+        channel.unary_unary("/pkg.Servicer/Handler")(b"request")
+    finally:
+        server.stop(None)

tests/snapshots/tests.contrib.grpc.test_grpc.test_method_service.snap

@@ -0,0 +1,84 @@
+[[{"name" "grpc"
+   "service" "grpc-client"
+   "resource" "/Servicer/Handler"
+   "type" "grpc"
+   "error" 0
+   "span_id" 0
+   "trace_id" 0
+   "parent_id" nil
+   "start" 1619744633550844100
+   "duration" 1911900
+   "meta" {"runtime-id" "04d67a3bd550461295cfd98eedc69d99"
+           "grpc.port" "55945"
+           "grpc.host" "localhost"
+           "grpc.method.kind" "unary"
+           "span.kind" "client"
+           "grpc.method.path" "/Servicer/Handler"
+           "grpc.status.code" "StatusCode.OK"
+           "grpc.method.service" "Servicer"
+           "grpc.method.name" "Handler"}
+   "metrics" {"_dd.agent_psr" 1.0
+              "_dd.measured" 1
+              "_sampling_priority_v1" 1
+              "system.pid" 714
+              "_dd.tracer_kr" 1.0}}
+      {"name" "grpc"
+       "service" "grpc-server"
+       "resource" "/Servicer/Handler"
+       "type" "grpc"
+       "error" 0
+       "span_id" 1
+       "trace_id" 0
+       "parent_id" 0
+       "start" 1619744633552296900
+       "duration" 48200
+       "meta" {"grpc.method.path" "/Servicer/Handler"
+               "grpc.method.service" "Servicer"
+               "grpc.method.name" "Handler"
+               "grpc.method.kind" "unary"
+               "span.kind" "server"}
+       "metrics" {"_dd.measured" 1
+                  "_sampling_priority_v1" 1}}]
+ [{"name" "grpc"
+   "service" "grpc-client"
+   "resource" "/pkg.Servicer/Handler"
+   "type" "grpc"
+   "error" 0
+   "span_id" 0
+   "trace_id" 1
+   "parent_id" nil
+   "start" 1619744633553123900
+   "duration" 1079700
+   "meta" {"runtime-id" "04d67a3bd550461295cfd98eedc69d99"
+           "grpc.port" "55945"
+           "grpc.method.package" "pkg"
+           "grpc.host" "localhost"
+           "grpc.method.kind" "unary"
+           "span.kind" "client"
+           "grpc.method.path" "/pkg.Servicer/Handler"
+           "grpc.status.code" "StatusCode.OK"
+           "grpc.method.service" "Servicer"
+           "grpc.method.name" "Handler"}
+   "metrics" {"_dd.agent_psr" 1.0
+              "_dd.measured" 1
+              "_sampling_priority_v1" 1
+              "system.pid" 714
+              "_dd.tracer_kr" 1.0}}
+      {"name" "grpc"
+       "service" "grpc-server"
+       "resource" "/pkg.Servicer/Handler"
+       "type" "grpc"
+       "error" 0
+       "span_id" 1
+       "trace_id" 1
+       "parent_id" 0
+       "start" 1619744633553812700
+       "duration" 42900
+       "meta" {"grpc.method.path" "/pkg.Servicer/Handler"
+               "grpc.method.package" "pkg"
+               "grpc.method.service" "Servicer"
+               "grpc.method.name" "Handler"
+               "grpc.method.kind" "unary"
+               "span.kind" "server"}
+       "metrics" {"_dd.measured" 1
+                  "_sampling_priority_v1" 1}}]]