From e584efc99dc065354fd14e70c5aab6a4aeab7d95 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Tue, 15 Dec 2020 13:04:22 +0100 Subject: [PATCH] Fix gRPC metadata key allowed characters Signed-off-by: Pavol Loffay --- .../grpc/v1_5/GrpcSemanticAttributes.java | 31 +++++++++++-------- .../grpc/v1_5/GrpcSpanDecorator.java | 4 +-- ...ettyHttp2HeadersInstrumentationModule.java | 12 ++++--- .../grpc/v1_5/GrpcInstrumentationTest.java | 15 ++++++--- 4 files changed, 39 insertions(+), 23 deletions(-) diff --git a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSemanticAttributes.java b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSemanticAttributes.java index c6e8cc066..015439480 100644 --- a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSemanticAttributes.java +++ b/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. * - *

We cannot use prefix because e.g. ht.:path is not a valid key. + *

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 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 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 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 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; + } } diff --git a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSpanDecorator.java b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSpanDecorator.java index 1a664de6d..7b58ce6a5 100644 --- a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSpanDecorator.java +++ b/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 stringKey = Key.of(key, Metadata.ASCII_STRING_MARSHALLER); Iterable 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 metadataToMap(Metadata metadata) { Key stringKey = Key.of(key, Metadata.ASCII_STRING_MARSHALLER); Iterable stringValues = metadata.getAll(stringKey); for (String stringValue : stringValues) { - key = GrpcSemanticAttributes.removeHypertracePrefix(key); + key = GrpcSemanticAttributes.removeHypertracePrefixAndAddColon(key); mapHeaders.put(key, stringValue); } } diff --git a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/NettyHttp2HeadersInstrumentationModule.java b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/NettyHttp2HeadersInstrumentationModule.java index 962271eb7..a16f74fd6 100644 --- a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/NettyHttp2HeadersInstrumentationModule.java +++ b/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()); } } diff --git a/instrumentation/grpc-1.5/src/test/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcInstrumentationTest.java b/instrumentation/grpc-1.5/src/test/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcInstrumentationTest.java index a4fa297b9..4793254b6 100644 --- a/instrumentation/grpc-1.5/src/test/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcInstrumentationTest.java +++ b/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))); } }