Fix gRPC metadata key allowed characters (#178)

Signed-off-by: Pavol Loffay <[email protected]>

Author: pavolloffay

instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSemanticAttributes.java

@@ -21,32 +21,37 @@
 public class GrpcSemanticAttributes {
   private GrpcSemanticAttributes() {}
 
-  public static final String SCHEME = ":scheme";
-  public static final String PATH = ":path";
-  public static final String AUTHORITY = ":authority";
-  public static final String METHOD = ":method";
+  public static final String SCHEME = "scheme";
+  public static final String PATH = "path";
+  public static final String AUTHORITY = "authority";
+  public static final String METHOD = "method";
 
   /**
    * These metadata headers are added in Http2Headers instrumentation. We use different names than
    * original HTTP2 header names to avoid any collisions with app code.
    *
-   * <p>We cannot use prefix because e.g. ht.:path is not a valid key.
+   * <p>For valid characters in keys read
+   * https://grpc.github.io/grpc-java/javadoc/io/grpc/Metadata.Key.html
    */
-  private static final String SUFFIX = ".ht";
+  private static final String PREFIX = "ht.";
 
   public static final Metadata.Key<String> SCHEME_METADATA_KEY =
-      Metadata.Key.of(SCHEME + SUFFIX, Metadata.ASCII_STRING_MARSHALLER);
+      Metadata.Key.of(PREFIX + SCHEME, Metadata.ASCII_STRING_MARSHALLER);
   public static final Metadata.Key<String> PATH_METADATA_KEY =
-      Metadata.Key.of(PATH + SUFFIX, Metadata.ASCII_STRING_MARSHALLER);
+      Metadata.Key.of(PREFIX + PATH, Metadata.ASCII_STRING_MARSHALLER);
   public static final Metadata.Key<String> AUTHORITY_METADATA_KEY =
-      Metadata.Key.of(AUTHORITY + SUFFIX, Metadata.ASCII_STRING_MARSHALLER);
+      Metadata.Key.of(PREFIX + AUTHORITY, Metadata.ASCII_STRING_MARSHALLER);
   public static final Metadata.Key<String> METHOD_METADATA_KEY =
-      Metadata.Key.of(METHOD + SUFFIX, Metadata.ASCII_STRING_MARSHALLER);
+      Metadata.Key.of(PREFIX + METHOD, Metadata.ASCII_STRING_MARSHALLER);
 
-  public static String removeHypertracePrefix(String key) {
-    if (key.endsWith(SUFFIX)) {
-      return key.replace(SUFFIX, "");
+  public static String removeHypertracePrefixAndAddColon(String key) {
+    if (key.startsWith(PREFIX)) {
+      return addColon(key.replaceFirst(PREFIX, ""));
     }
     return key;
   }
+
+  public static String addColon(String key) {
+    return ":" + key;
+  }
 }

instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSpanDecorator.java

@@ -58,7 +58,7 @@ public static void addMetadataAttributes(
       Key<String> stringKey = Key.of(key, Metadata.ASCII_STRING_MARSHALLER);
       Iterable<String> stringValues = metadata.getAll(stringKey);
       for (String stringValue : stringValues) {
-        key = GrpcSemanticAttributes.removeHypertracePrefix(key);
+        key = GrpcSemanticAttributes.removeHypertracePrefixAndAddColon(key);
         span.setAttribute(keySupplier.apply(key), stringValue);
       }
     }
@@ -80,7 +80,7 @@ public static Map<String, String> metadataToMap(Metadata metadata) {
       Key<String> stringKey = Key.of(key, Metadata.ASCII_STRING_MARSHALLER);
       Iterable<String> stringValues = metadata.getAll(stringKey);
       for (String stringValue : stringValues) {
-        key = GrpcSemanticAttributes.removeHypertracePrefix(key);
+        key = GrpcSemanticAttributes.removeHypertracePrefixAndAddColon(key);
         mapHeaders.put(key, stringValue);
       }
     }

instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/NettyHttp2HeadersInstrumentationModule.java

@@ -97,22 +97,26 @@ public static void exit(
       Span currentSpan = Java8BytecodeBridge.currentSpan();
       if (scheme != null) {
         currentSpan.setAttribute(
-            HypertraceSemanticAttributes.rpcRequestMetadata(GrpcSemanticAttributes.SCHEME),
+            HypertraceSemanticAttributes.rpcRequestMetadata(
+                GrpcSemanticAttributes.addColon(GrpcSemanticAttributes.SCHEME)),
             scheme.toString());
       }
       if (defaultPath != null) {
         currentSpan.setAttribute(
-            HypertraceSemanticAttributes.rpcRequestMetadata(GrpcSemanticAttributes.PATH),
+            HypertraceSemanticAttributes.rpcRequestMetadata(
+                GrpcSemanticAttributes.addColon(GrpcSemanticAttributes.PATH)),
             defaultPath.toString());
       }
       if (authority != null) {
         currentSpan.setAttribute(
-            HypertraceSemanticAttributes.rpcRequestMetadata(GrpcSemanticAttributes.AUTHORITY),
+            HypertraceSemanticAttributes.rpcRequestMetadata(
+                GrpcSemanticAttributes.addColon(GrpcSemanticAttributes.AUTHORITY)),
             authority.toString());
       }
       if (method != null) {
         currentSpan.setAttribute(
-            HypertraceSemanticAttributes.rpcRequestMetadata(GrpcSemanticAttributes.METHOD),
+            HypertraceSemanticAttributes.rpcRequestMetadata(
+                GrpcSemanticAttributes.addColon(GrpcSemanticAttributes.METHOD)),
             method.toString());
       }
     }

instrumentation/grpc-1.5/src/test/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcInstrumentationTest.java

@@ -235,19 +235,26 @@ private void assertHttp2HeadersForSayHelloMethod(SpanData span) {
     Assertions.assertEquals(
         "http",
         span.getAttributes()
-            .get(HypertraceSemanticAttributes.rpcRequestMetadata(GrpcSemanticAttributes.SCHEME)));
+            .get(
+                HypertraceSemanticAttributes.rpcRequestMetadata(
+                    ":" + GrpcSemanticAttributes.SCHEME)));
     Assertions.assertEquals(
         "POST",
         span.getAttributes()
-            .get(HypertraceSemanticAttributes.rpcRequestMetadata(GrpcSemanticAttributes.METHOD)));
+            .get(
+                HypertraceSemanticAttributes.rpcRequestMetadata(
+                    ":" + GrpcSemanticAttributes.METHOD)));
     Assertions.assertEquals(
         String.format("localhost:%d", SERVER.getPort()),
         span.getAttributes()
             .get(
-                HypertraceSemanticAttributes.rpcRequestMetadata(GrpcSemanticAttributes.AUTHORITY)));
+                HypertraceSemanticAttributes.rpcRequestMetadata(
+                    ":" + GrpcSemanticAttributes.AUTHORITY)));
     Assertions.assertEquals(
         "/org.hypertrace.example.Greeter/SayHello",
         span.getAttributes()
-            .get(HypertraceSemanticAttributes.rpcRequestMetadata(GrpcSemanticAttributes.PATH)));
+            .get(
+                HypertraceSemanticAttributes.rpcRequestMetadata(
+                    ":" + GrpcSemanticAttributes.PATH)));
   }
 }