From 30f0a7b94e3bb01677df3a33fcf6e1c013515fc5 Mon Sep 17 00:00:00 2001
From: kotharironak <53209990+kotharironak@users.noreply.github.com>
Date: Mon, 8 Nov 2021 17:19:38 +0530
Subject: [PATCH] feat: add forming grpc reqeuest url if rpc attributes are
 present (#280)

* feat: add forming grpc reqeust url if rpc attributes are present

* nit: minor fix on comment
---
 .../generators/SpanEventViewGenerator.java    | 11 +++-
 .../SpanEventViewGeneratorTest.java           | 51 +++++++++++++++++++
 2 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/SpanEventViewGenerator.java b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/SpanEventViewGenerator.java
index e8d4f322c..9b4ed2bd3 100644
--- a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/SpanEventViewGenerator.java
+++ b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/SpanEventViewGenerator.java
@@ -15,6 +15,7 @@
 import org.hypertrace.core.datamodel.StructuredTrace;
 import org.hypertrace.core.datamodel.shared.SpanAttributeUtils;
 import org.hypertrace.semantic.convention.utils.http.HttpSemanticConventionUtils;
+import org.hypertrace.semantic.convention.utils.rpc.RpcSemanticConventionUtils;
 import org.hypertrace.traceenricher.enrichedspan.constants.EnrichedSpanConstants;
 import org.hypertrace.traceenricher.enrichedspan.constants.utils.EnrichedSpanUtils;
 import org.hypertrace.traceenricher.enrichedspan.constants.v1.Api;
@@ -315,7 +316,15 @@ String getRequestUrl(Event event, Protocol protocol) {
             .orElse(HttpSemanticConventionUtils.getHttpPath(event).orElse(null));
 
       case PROTOCOL_GRPC:
-        return event.getEventName();
+        /**
+         * For RPC methods, we show URL/URI as a combination of rpc.service and rpc.method. The same
+         * information is also available as Span Name -
+         * https://github.com/open-telemetry/opentelemetry-specification/blob/3e380e249f60c3a5f68746f5e84d10195ba41a79/specification/trace/semantic_conventions/rpc.md#span-name
+         * So, as part of this method, we will form Url using rpc.service and rpc.method, and
+         * fallback to spanName. While setting GRPC protocol from rpc attributes, it already checks
+         * for rpc.system.
+         */
+        return RpcSemanticConventionUtils.getRpcPath(event).orElse(event.getEventName());
     }
     return null;
   }
diff --git a/hypertrace-view-generator/hypertrace-view-generator/src/test/java/org/hypertrace/viewgenerator/generators/SpanEventViewGeneratorTest.java b/hypertrace-view-generator/hypertrace-view-generator/src/test/java/org/hypertrace/viewgenerator/generators/SpanEventViewGeneratorTest.java
index 375239214..112ba2aec 100644
--- a/hypertrace-view-generator/hypertrace-view-generator/src/test/java/org/hypertrace/viewgenerator/generators/SpanEventViewGeneratorTest.java
+++ b/hypertrace-view-generator/hypertrace-view-generator/src/test/java/org/hypertrace/viewgenerator/generators/SpanEventViewGeneratorTest.java
@@ -79,6 +79,57 @@ public void test_getRequestUrl_grpcProctol_shouldReturnEventName() {
         spanEventViewGenerator.getRequestUrl(event, Protocol.PROTOCOL_GRPC));
   }
 
+  @Test
+  public void test_getRequestUrl_grpcProctol_shouldReturnRpcServiceAndMethod() {
+    Event event = mock(Event.class);
+    when(event.getAttributes())
+        .thenReturn(
+            Attributes.newBuilder()
+                .setAttributeMap(
+                    Map.of(
+                        "rpc.service",
+                        AttributeValue.newBuilder().setValue("hipstershop.AdService").build(),
+                        "rpc.method",
+                        AttributeValue.newBuilder().setValue("GetEcho").build()))
+                .build());
+    when(event.getEventName()).thenReturn("Sent.hipstershop.AdService.GetAds");
+    assertEquals(
+        "hipstershop.AdService.GetEcho",
+        spanEventViewGenerator.getRequestUrl(event, Protocol.PROTOCOL_GRPC));
+  }
+
+  @Test
+  public void test_getRequestUrl_grpcProctol_shouldReturnEventNameIfOnlyRpcService() {
+    Event event = mock(Event.class);
+    when(event.getAttributes())
+        .thenReturn(
+            Attributes.newBuilder()
+                .setAttributeMap(
+                    Map.of(
+                        "rpc.service",
+                        AttributeValue.newBuilder().setValue("hipstershop.AdService").build()))
+                .build());
+    when(event.getEventName()).thenReturn("Sent.hipstershop.AdService.GetAds");
+    assertEquals(
+        "Sent.hipstershop.AdService.GetAds",
+        spanEventViewGenerator.getRequestUrl(event, Protocol.PROTOCOL_GRPC));
+  }
+
+  @Test
+  public void test_getRequestUrl_grpcProctol_shouldReturnEventNameIfOnlyRpcMethod() {
+    Event event = mock(Event.class);
+    when(event.getAttributes())
+        .thenReturn(
+            Attributes.newBuilder()
+                .setAttributeMap(
+                    Map.of("rpc.method", AttributeValue.newBuilder().setValue("GetEcho").build()))
+                .build());
+    when(event.getEventName()).thenReturn("Sent.hipstershop.AdService.GetAds");
+    assertEquals(
+        "Sent.hipstershop.AdService.GetAds",
+        spanEventViewGenerator.getRequestUrl(event, Protocol.PROTOCOL_GRPC));
+  }
+
   @Test
   public void testGetRequestUrl_fullUrlIsAbsent() {
     Event event =