From a50059574eca38fe1000af59ad251d54fc6fb156 Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Tue, 2 Nov 2021 13:02:11 +0530 Subject: [PATCH 1/7] added gethost method --- .../enrichers/ApiBoundaryTypeAttributeEnricher.java | 9 ++++++--- .../convention/utils/rpc/RpcSemanticConventionUtils.java | 7 +++++++ .../core/span/normalizer/constants/RpcSpanTag.java | 1 + 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java index cf4fdbda0..81b7e4cb8 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java @@ -1,5 +1,7 @@ package org.hypertrace.traceenricher.enrichment.enrichers; +import static org.hypertrace.semantic.convention.utils.rpc.RpcSemanticConventionUtils.getGrpcRequestMetadataHost; + import com.google.common.base.Splitter; import com.google.common.collect.ImmutableList; import java.net.URI; @@ -128,7 +130,7 @@ public void enrichEvent(StructuredTrace trace, Event event) { private void enrichHostHeader(Event event) { Protocol protocol = EnrichedSpanUtils.getProtocol(event); if (protocol == Protocol.PROTOCOL_GRPC) { - Optional host = getGrpcAuthority(event); + Optional host = getGrpcHostHeader(event); if (host.isPresent()) { addEnrichedAttribute(event, HOST_HEADER, AttributeValueCreator.create(host.get())); return; @@ -144,12 +146,13 @@ private void enrichHostHeader(Event event) { } } - private Optional getGrpcAuthority(Event event) { + private Optional getGrpcHostHeader(Event event) { Optional grpcAuthority = RpcSemanticConventionUtils.getGrpcAuthority(event); if (grpcAuthority.isPresent()) { return getSanitizedHostValue(grpcAuthority.get()); + } else { + return getGrpcRequestMetadataHost(event); } - return Optional.empty(); } private Optional getSanitizedHostValue(String value) { diff --git a/semantic-convention-utils/src/main/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtils.java b/semantic-convention-utils/src/main/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtils.java index c913d3052..d6cebb334 100644 --- a/semantic-convention-utils/src/main/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtils.java +++ b/semantic-convention-utils/src/main/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtils.java @@ -17,6 +17,7 @@ import static org.hypertrace.core.span.normalizer.constants.RpcSpanTag.RPC_REQUEST_BODY_TRUNCATED; import static org.hypertrace.core.span.normalizer.constants.RpcSpanTag.RPC_REQUEST_METADATA_AUTHORITY; import static org.hypertrace.core.span.normalizer.constants.RpcSpanTag.RPC_REQUEST_METADATA_CONTENT_LENGTH; +import static org.hypertrace.core.span.normalizer.constants.RpcSpanTag.RPC_REQUEST_METADATA_HOST; import static org.hypertrace.core.span.normalizer.constants.RpcSpanTag.RPC_REQUEST_METADATA_PATH; import static org.hypertrace.core.span.normalizer.constants.RpcSpanTag.RPC_REQUEST_METADATA_USER_AGENT; import static org.hypertrace.core.span.normalizer.constants.RpcSpanTag.RPC_REQUEST_METADATA_X_FORWARDED_FOR; @@ -234,6 +235,12 @@ public static String getGrpcStatusMsg(Event event) { return grpcStatusMsg == null ? "" : grpcStatusMsg; } + public static Optional getGrpcRequestMetadataHost(Event event) { + return Optional.ofNullable( + SpanAttributeUtils.getFirstAvailableStringAttribute( + event, List.of(RPC_REQUEST_METADATA_HOST.getValue()))); + } + public static String getGrpcErrorMsg(Event event) { if (event.getAttributes() == null || event.getAttributes().getAttributeMap() == null) { return ""; diff --git a/span-normalizer/span-normalizer-constants/src/main/java/org/hypertrace/core/span/normalizer/constants/RpcSpanTag.java b/span-normalizer/span-normalizer-constants/src/main/java/org/hypertrace/core/span/normalizer/constants/RpcSpanTag.java index dfcd27690..01c3961c1 100644 --- a/span-normalizer/span-normalizer-constants/src/main/java/org/hypertrace/core/span/normalizer/constants/RpcSpanTag.java +++ b/span-normalizer/span-normalizer-constants/src/main/java/org/hypertrace/core/span/normalizer/constants/RpcSpanTag.java @@ -5,6 +5,7 @@ public enum RpcSpanTag { RPC_ERROR_NAME("rpc.error_name"), RPC_ERROR_MESSAGE("rpc.error_message"), RPC_REQUEST_METADATA("rpc.request.metadata"), + RPC_REQUEST_METADATA_HOST("rpc.request.metadata.host"), RPC_RESPONSE_METADATA("rpc.response.metadata"), RPC_REQUEST_BODY("rpc.request.body"), RPC_RESPONSE_BODY("rpc.response.body"), From c8b449edd97870b78ce3fbfecfe8b7b7087e3b4f Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Tue, 2 Nov 2021 13:17:07 +0530 Subject: [PATCH 2/7] modified --- .../ApiBoundaryTypeAttributeEnricher.java | 12 ++++++---- .../utils/rpc/RpcSemanticConventionUtils.java | 24 ++++++++++++++----- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java index 81b7e4cb8..c9ebf4b98 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java @@ -1,7 +1,5 @@ package org.hypertrace.traceenricher.enrichment.enrichers; -import static org.hypertrace.semantic.convention.utils.rpc.RpcSemanticConventionUtils.getGrpcRequestMetadataHost; - import com.google.common.base.Splitter; import com.google.common.collect.ImmutableList; import java.net.URI; @@ -150,9 +148,15 @@ private Optional getGrpcHostHeader(Event event) { Optional grpcAuthority = RpcSemanticConventionUtils.getGrpcAuthority(event); if (grpcAuthority.isPresent()) { return getSanitizedHostValue(grpcAuthority.get()); - } else { - return getGrpcRequestMetadataHost(event); } + + Optional grpcRequestMetadataHost = + RpcSemanticConventionUtils.getGrpcRequestMetadataHost(event); + if (grpcRequestMetadataHost.isPresent()) { + return getSanitizedHostValue(grpcRequestMetadataHost.get()); + } + + return Optional.empty(); } private Optional getSanitizedHostValue(String value) { diff --git a/semantic-convention-utils/src/main/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtils.java b/semantic-convention-utils/src/main/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtils.java index d6cebb334..f175778e5 100644 --- a/semantic-convention-utils/src/main/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtils.java +++ b/semantic-convention-utils/src/main/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtils.java @@ -235,12 +235,6 @@ public static String getGrpcStatusMsg(Event event) { return grpcStatusMsg == null ? "" : grpcStatusMsg; } - public static Optional getGrpcRequestMetadataHost(Event event) { - return Optional.ofNullable( - SpanAttributeUtils.getFirstAvailableStringAttribute( - event, List.of(RPC_REQUEST_METADATA_HOST.getValue()))); - } - public static String getGrpcErrorMsg(Event event) { if (event.getAttributes() == null || event.getAttributes().getAttributeMap() == null) { return ""; @@ -437,6 +431,24 @@ public static Optional getGrpcXForwardedFor(Event event) { return Optional.empty(); } + public static Optional getGrpcRequestMetadataHost(Event event) { + if (event.getAttributes() == null || event.getAttributes().getAttributeMap() == null) { + return Optional.empty(); + } + + Map attributeValueMap = event.getAttributes().getAttributeMap(); + + if (!isRpcSystemGrpc(attributeValueMap)) { + return Optional.empty(); + } + + if (attributeValueMap.get(RPC_REQUEST_METADATA_HOST.getValue()) != null) { + return Optional.ofNullable( + attributeValueMap.get(RPC_REQUEST_METADATA_HOST.getValue()).getValue()); + } + return Optional.empty(); + } + public static Optional getRpcPath(Event event) { String service = getRpcService(event).orElse(""); String method = getRpcMethod(event).orElse(""); From ae7c24762e054e688aa72d59e6e0dff346b570d5 Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Tue, 2 Nov 2021 13:45:20 +0530 Subject: [PATCH 3/7] added unit test --- .../rpc/RpcSemanticConventionUtilsTest.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/semantic-convention-utils/src/test/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtilsTest.java b/semantic-convention-utils/src/test/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtilsTest.java index 014574d30..606958adb 100644 --- a/semantic-convention-utils/src/test/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtilsTest.java +++ b/semantic-convention-utils/src/test/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtilsTest.java @@ -17,6 +17,7 @@ import static org.hypertrace.core.span.normalizer.constants.RpcSpanTag.RPC_REQUEST_BODY_TRUNCATED; import static org.hypertrace.core.span.normalizer.constants.RpcSpanTag.RPC_REQUEST_METADATA_AUTHORITY; import static org.hypertrace.core.span.normalizer.constants.RpcSpanTag.RPC_REQUEST_METADATA_CONTENT_LENGTH; +import static org.hypertrace.core.span.normalizer.constants.RpcSpanTag.RPC_REQUEST_METADATA_HOST; import static org.hypertrace.core.span.normalizer.constants.RpcSpanTag.RPC_REQUEST_METADATA_PATH; import static org.hypertrace.core.span.normalizer.constants.RpcSpanTag.RPC_REQUEST_METADATA_USER_AGENT; import static org.hypertrace.core.span.normalizer.constants.RpcSpanTag.RPC_REQUEST_METADATA_X_FORWARDED_FOR; @@ -165,6 +166,23 @@ public void testGetGrpcXForwardedFor() { assertEquals("198.12.34.1", RpcSemanticConventionUtils.getGrpcXForwardedFor(event).get()); } + @Test + public void testGetGrpcRequestMetadataHost() { + Event event = mock(Event.class); + when(event.getAttributes()) + .thenReturn( + Attributes.newBuilder() + .setAttributeMap( + Map.of( + OTEL_SPAN_TAG_RPC_SYSTEM.getValue(), + AttributeValue.newBuilder().setValue("grpc").build(), + RPC_REQUEST_METADATA_HOST.getValue(), + AttributeValue.newBuilder().setValue("webhost:9011").build())) + .build()); + assertEquals( + "webhost:9011", RpcSemanticConventionUtils.getGrpcRequestMetadataHost(event).get()); + } + @Test public void testGetGrpcStatusMsg() { Event event = From f1a17aa7365c59a4672205ea1020c36b952631ec Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Wed, 3 Nov 2021 10:30:04 +0530 Subject: [PATCH 4/7] resolved PR comments --- .../utils/rpc/RpcSemanticConventionUtils.java | 9 +++------ .../rpc/RpcSemanticConventionUtilsTest.java | 16 +++++++++++++++- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/semantic-convention-utils/src/main/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtils.java b/semantic-convention-utils/src/main/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtils.java index f175778e5..cc93b3985 100644 --- a/semantic-convention-utils/src/main/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtils.java +++ b/semantic-convention-utils/src/main/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtils.java @@ -69,6 +69,7 @@ public class RpcSemanticConventionUtils { private static final String OTEL_RPC_SYSTEM_GRPC = OTelRpcSemanticConventions.RPC_SYSTEM_VALUE_GRPC.getValue(); private static final String OTEL_SPAN_TAG_RPC_SYSTEM_ATTR = OTEL_SPAN_TAG_RPC_SYSTEM.getValue(); + private static final String RPC_REQUEST_METADATA_HOST_ATTR = RPC_REQUEST_METADATA_HOST.getValue(); private static final String OTHER_GRPC_HOST_PORT = RawSpanConstants.getValue(Grpc.GRPC_HOST_PORT); private static final String OTHER_GRPC_METHOD = RawSpanConstants.getValue(Grpc.GRPC_METHOD); @@ -441,12 +442,8 @@ public static Optional getGrpcRequestMetadataHost(Event event) { if (!isRpcSystemGrpc(attributeValueMap)) { return Optional.empty(); } - - if (attributeValueMap.get(RPC_REQUEST_METADATA_HOST.getValue()) != null) { - return Optional.ofNullable( - attributeValueMap.get(RPC_REQUEST_METADATA_HOST.getValue()).getValue()); - } - return Optional.empty(); + return Optional.ofNullable(attributeValueMap.get(RPC_REQUEST_METADATA_HOST_ATTR)) + .map(AttributeValue::getValue); } public static Optional getRpcPath(Event event) { diff --git a/semantic-convention-utils/src/test/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtilsTest.java b/semantic-convention-utils/src/test/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtilsTest.java index 606958adb..19b9a136c 100644 --- a/semantic-convention-utils/src/test/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtilsTest.java +++ b/semantic-convention-utils/src/test/java/org/hypertrace/semantic/convention/utils/rpc/RpcSemanticConventionUtilsTest.java @@ -167,7 +167,7 @@ public void testGetGrpcXForwardedFor() { } @Test - public void testGetGrpcRequestMetadataHost() { + public void testGetGrpcRequestMetadataHostForGrpcSystem() { Event event = mock(Event.class); when(event.getAttributes()) .thenReturn( @@ -183,6 +183,20 @@ public void testGetGrpcRequestMetadataHost() { "webhost:9011", RpcSemanticConventionUtils.getGrpcRequestMetadataHost(event).get()); } + @Test + public void testGetGrpcRequestMetadataHostForNotGrpcSystem() { + Event event = mock(Event.class); + when(event.getAttributes()) + .thenReturn( + Attributes.newBuilder() + .setAttributeMap( + Map.of( + RPC_REQUEST_METADATA_HOST.getValue(), + AttributeValue.newBuilder().setValue("webhost:9011").build())) + .build()); + assertEquals(Optional.empty(), RpcSemanticConventionUtils.getGrpcRequestMetadataHost(event)); + } + @Test public void testGetGrpcStatusMsg() { Event event = From d2f3772b76c973253a5ea31314f44181033a1517 Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Wed, 3 Nov 2021 12:01:15 +0530 Subject: [PATCH 5/7] added unit test --- .../ApiBoundaryTypeAttributeEnricherTest.java | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricherTest.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricherTest.java index 2f3be1baf..3989e9e81 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricherTest.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricherTest.java @@ -2,6 +2,7 @@ import static org.hypertrace.core.span.normalizer.constants.OTelSpanTag.OTEL_SPAN_TAG_RPC_SYSTEM; import static org.hypertrace.core.span.normalizer.constants.RpcSpanTag.RPC_REQUEST_METADATA_AUTHORITY; +import static org.hypertrace.core.span.normalizer.constants.RpcSpanTag.RPC_REQUEST_METADATA_HOST; import static org.mockito.Mockito.RETURNS_DEEP_STUBS; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -300,6 +301,34 @@ public void testEnrichEventWithGrpcNoAuthority() { Assertions.assertEquals(EnrichedSpanUtils.getHostHeader(innerEntrySpan), "testHost"); } + @Test + public void testEnrichEventWithGrpcNoAuthorityButRequestMetadataHost() { + mockProtocol(innerEntrySpan, Protocol.PROTOCOL_GRPC); + org.hypertrace.core.datamodel.eventfields.grpc.Grpc grpc = + mock(org.hypertrace.core.datamodel.eventfields.grpc.Grpc.class); + when(innerEntrySpan.getGrpc()).thenReturn(grpc); + Request request = mock(Request.class); + when(grpc.getRequest()).thenReturn(request); + RequestMetadata metadata = mock(RequestMetadata.class); + when(request.getRequestMetadata()).thenReturn(metadata); + when(metadata.getAuthority()).thenReturn("localhost:443"); + + addEnrichedAttributeToEvent( + innerEntrySpan, X_FORWARDED_HOST_METADATA, AttributeValueCreator.create("testHost")); + + addAttributeToEvent( + innerEntrySpan, + RPC_REQUEST_METADATA_HOST.getValue(), + AttributeValue.newBuilder().setValue("testHost2").build()); + addAttributeToEvent( + innerEntrySpan, + OTEL_SPAN_TAG_RPC_SYSTEM.getValue(), + AttributeValue.newBuilder().setValue("grpc").build()); + + target.enrichEvent(trace, innerEntrySpan); + Assertions.assertEquals(EnrichedSpanUtils.getHostHeader(innerEntrySpan), "testHost2"); + } + private void mockStructuredGraph() { when(graph.getParentEvent(innerExitSpan)).thenReturn(innerEntrySpan); when(graph.getChildrenEvents(innerExitSpan)).thenReturn(null); From 998a0635b204c0e325403ad0cab576e2acd45fe2 Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Wed, 3 Nov 2021 12:28:57 +0530 Subject: [PATCH 6/7] nit --- .../enrichers/ApiBoundaryTypeAttributeEnricherTest.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricherTest.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricherTest.java index 3989e9e81..d8674b1e3 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricherTest.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricherTest.java @@ -304,15 +304,6 @@ public void testEnrichEventWithGrpcNoAuthority() { @Test public void testEnrichEventWithGrpcNoAuthorityButRequestMetadataHost() { mockProtocol(innerEntrySpan, Protocol.PROTOCOL_GRPC); - org.hypertrace.core.datamodel.eventfields.grpc.Grpc grpc = - mock(org.hypertrace.core.datamodel.eventfields.grpc.Grpc.class); - when(innerEntrySpan.getGrpc()).thenReturn(grpc); - Request request = mock(Request.class); - when(grpc.getRequest()).thenReturn(request); - RequestMetadata metadata = mock(RequestMetadata.class); - when(request.getRequestMetadata()).thenReturn(metadata); - when(metadata.getAuthority()).thenReturn("localhost:443"); - addEnrichedAttributeToEvent( innerEntrySpan, X_FORWARDED_HOST_METADATA, AttributeValueCreator.create("testHost")); From ddf8d2482f99c64a9cf71aaeca41ed0936f1a56c Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Wed, 3 Nov 2021 12:40:33 +0530 Subject: [PATCH 7/7] nit --- .../enrichers/ApiBoundaryTypeAttributeEnricherTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricherTest.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricherTest.java index d8674b1e3..bf4367b5f 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricherTest.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricherTest.java @@ -274,6 +274,10 @@ public void testEnrichEventWithGrpcAuthority() { innerEntrySpan, RPC_REQUEST_METADATA_AUTHORITY.getValue(), AttributeValue.newBuilder().setValue("testHost").build()); + addAttributeToEvent( + innerEntrySpan, + RPC_REQUEST_METADATA_HOST.getValue(), + AttributeValue.newBuilder().setValue("testHost2").build()); addAttributeToEvent( innerEntrySpan, OTEL_SPAN_TAG_RPC_SYSTEM.getValue(),