From ca94cd1c4f4221673c7fe436f314987267d840a6 Mon Sep 17 00:00:00 2001 From: Sakshi Goyal Date: Tue, 20 Dec 2022 12:27:18 +0530 Subject: [PATCH 1/7] add deployment attribute in resource enricher --- .../v1/enriched_span_attribute.proto | 7 ++++ .../enrichers/ResourceAttributeEnricher.java | 33 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/hypertrace-trace-enricher/enriched-span-constants/src/main/proto/org/hypertrace/traceenricher/enrichedspan/constants/v1/enriched_span_attribute.proto b/hypertrace-trace-enricher/enriched-span-constants/src/main/proto/org/hypertrace/traceenricher/enrichedspan/constants/v1/enriched_span_attribute.proto index 64174f451..5ab1aaac2 100644 --- a/hypertrace-trace-enricher/enriched-span-constants/src/main/proto/org/hypertrace/traceenricher/enrichedspan/constants/v1/enriched_span_attribute.proto +++ b/hypertrace-trace-enricher/enriched-span-constants/src/main/proto/org/hypertrace/traceenricher/enrichedspan/constants/v1/enriched_span_attribute.proto @@ -95,3 +95,10 @@ enum ErrorMetrics { ERROR_METRICS_TOTAL_SPANS_WITH_ERRORS = 3 [(string_value) = "span_count_with_errors"]; ERROR_METRICS_TOTAL_SPANS_WITH_EXCEPTIONS = 4 [(string_value) = "span_count_with_exceptions"]; } + +enum Deployment { + DEPLOYMENT_CANARY = 0 [(string_value) = "canary"]; + DEPLOYMENT_BASELINE = 1 [(string_value) = "baseline"]; + DEPLOYMENT_WEB = 2 [(string_value) = "web"]; + DEPLOYMENT_WORKER = 3 [(string_value) = "worker"]; +} diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java index c2757275f..e6a9a77d9 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java @@ -9,6 +9,9 @@ import org.hypertrace.core.datamodel.AttributeValue; import org.hypertrace.core.datamodel.Event; import org.hypertrace.core.datamodel.StructuredTrace; +import org.hypertrace.traceenricher.enrichedspan.constants.EnrichedSpanConstants; +import org.hypertrace.traceenricher.enrichedspan.constants.v1.Backend; +import org.hypertrace.traceenricher.enrichedspan.constants.v1.Deployment; import org.hypertrace.traceenricher.enrichment.AbstractTraceEnricher; import org.hypertrace.traceenricher.enrichment.clients.ClientRegistry; import org.slf4j.Logger; @@ -23,6 +26,11 @@ public class ResourceAttributeEnricher extends AbstractTraceEnricher { private static final Logger LOGGER = LoggerFactory.getLogger(ResourceAttributeEnricher.class); private static final String RESOURCE_ATTRIBUTES_CONFIG_KEY = "attributes"; private static final String NODE_SELECTOR_KEY = "node.selector"; + + private static final String DEPLOYMENT_KEY = "deployment"; + + private static final String POD_NAME_KEY = "pod.name"; + private static final String ATTRIBUTES_TO_MATCH_CONFIG_KEY = "attributesToMatch"; private List resourceAttributesToAdd = new ArrayList<>(); private Map resourceAttributeKeysToMatch = new HashMap<>(); @@ -63,6 +71,19 @@ public void enrichEvent(StructuredTrace trace, Event event) { } return attributeValue; })); + + // Add deployment attribute if pod name is available + if (resourceAttributeKey.equals(POD_NAME_KEY)) { + resourceAttributeMaybe.ifPresent( + attributeValue -> + attributeMap.computeIfAbsent( + DEPLOYMENT_KEY, + key -> { + attributeValue.setValue( + getDeploymentType(attributeValue.getValue())); + return attributeValue; + })); + } } } catch (Exception e) { LOGGER.error( @@ -77,4 +98,16 @@ private boolean isValidEvent(Event event) { && (event.getAttributes() != null) && (event.getAttributes().getAttributeMap() != null); } + + private String getDeploymentType(String hostName) { + hostName = EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_CANARY); + if (hostName.contains(EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_CANARY))) { + return EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_CANARY); + } else if (hostName.contains(EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_BASELINE))) { + return EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_BASELINE); + } else if (hostName.contains(EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_WORKER))) { + return EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_WORKER); + } + return EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_WEB); + } } From 32ad17d305218257377188174cff5eb8b0edff15 Mon Sep 17 00:00:00 2001 From: Sakshi Goyal Date: Tue, 20 Dec 2022 14:17:02 +0530 Subject: [PATCH 2/7] added unit test --- .../enrichers/ResourceAttributeEnricher.java | 23 +++++++++---------- .../ResourceAttributeEnricherTest.java | 10 ++++---- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java index e6a9a77d9..c033a4720 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java @@ -9,6 +9,7 @@ import org.hypertrace.core.datamodel.AttributeValue; import org.hypertrace.core.datamodel.Event; import org.hypertrace.core.datamodel.StructuredTrace; +import org.hypertrace.core.datamodel.shared.trace.AttributeValueCreator; import org.hypertrace.traceenricher.enrichedspan.constants.EnrichedSpanConstants; import org.hypertrace.traceenricher.enrichedspan.constants.v1.Backend; import org.hypertrace.traceenricher.enrichedspan.constants.v1.Deployment; @@ -73,17 +74,16 @@ public void enrichEvent(StructuredTrace trace, Event event) { })); // Add deployment attribute if pod name is available - if (resourceAttributeKey.equals(POD_NAME_KEY)) { - resourceAttributeMaybe.ifPresent( - attributeValue -> - attributeMap.computeIfAbsent( - DEPLOYMENT_KEY, - key -> { - attributeValue.setValue( - getDeploymentType(attributeValue.getValue())); - return attributeValue; - })); - } + if (resourceAttributeKey.equals(POD_NAME_KEY)) { + resourceAttributeMaybe.ifPresent( + attributeValue -> + attributeMap.computeIfAbsent( + DEPLOYMENT_KEY, + key -> { + return AttributeValueCreator + .create(getDeploymentType(attributeValue.getValue())); + })); + } } } catch (Exception e) { LOGGER.error( @@ -100,7 +100,6 @@ private boolean isValidEvent(Event event) { } private String getDeploymentType(String hostName) { - hostName = EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_CANARY); if (hostName.contains(EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_CANARY))) { return EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_CANARY); } else if (hostName.contains(EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_BASELINE))) { diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java index 899c7e5b1..43905547e 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java @@ -81,9 +81,10 @@ public void traceWithResource() { event.setResourceIndex(0); resourceAttributeEnricher.enrichEvent(structuredTrace, event); assertEquals( - resourceAttributesToAddList.size() - 2, event.getAttributes().getAttributeMap().size()); + resourceAttributesToAddList.size() - 1, event.getAttributes().getAttributeMap().size()); assertEquals( - "test-56f5d554c-5swkj", event.getAttributes().getAttributeMap().get("pod.name").getValue()); + "test-canary-56f5d554c-5swkj", event.getAttributes().getAttributeMap().get("pod.name").getValue()); + assertEquals("canary", event.getAttributes().getAttributeMap().get("deployment").getValue()); assertEquals( "01188498a468b5fef1eb4accd63533297c195a73", event.getAttributes().getAttributeMap().get("service.version").getValue()); @@ -103,7 +104,7 @@ public void traceWithResource() { addAttribute(event2, "cluster.name", "default"); resourceAttributeEnricher.enrichEvent(structuredTrace, event2); assertEquals( - resourceAttributesToAddList.size(), event2.getAttributes().getAttributeMap().size()); + resourceAttributesToAddList.size() + 1, event2.getAttributes().getAttributeMap().size()); assertEquals("123", event2.getAttributes().getAttributeMap().get("service.version").getValue()); assertEquals( "default", event2.getAttributes().getAttributeMap().get("cluster.name").getValue()); @@ -133,6 +134,7 @@ public void traceWithResource() { assertEquals( "worker-generic", event4.getAttributes().getAttributeMap().get("node.selector").getValue()); assertEquals("pod1", event4.getAttributes().getAttributeMap().get("pod.name").getValue()); + assertEquals("web", event4.getAttributes().getAttributeMap().get("deployment").getValue()); } private Resource getResource4() { @@ -210,7 +212,7 @@ private Resource getResource1() { put( "opencensus.exporterversion", AttributeValue.newBuilder().setValue("Jaeger-Go-2.23.1").build()); - put("host.name", AttributeValue.newBuilder().setValue("test-56f5d554c-5swkj").build()); + put("host.name", AttributeValue.newBuilder().setValue("test-canary-56f5d554c-5swkj").build()); put("ip", AttributeValue.newBuilder().setValue("10.21.18.171").build()); put("client-uuid", AttributeValue.newBuilder().setValue("53a112a715bda986").build()); put( From bafc012167113a6b2c1d8511105491747e207f0e Mon Sep 17 00:00:00 2001 From: Sakshi Goyal Date: Tue, 20 Dec 2022 14:27:08 +0530 Subject: [PATCH 3/7] added branch coverage tests --- .../enrichers/ResourceAttributeEnricherTest.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java index 43905547e..754f46deb 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java @@ -112,6 +112,8 @@ public void traceWithResource() { "worker-generic", event2.getAttributes().getAttributeMap().get("node.name").getValue()); assertEquals( "worker-generic", event2.getAttributes().getAttributeMap().get("node.selector").getValue()); + assertEquals("test1-baseline-56f5d554c-5swkj", event2.getAttributes().getAttributeMap().get("pod.name").getValue()); + assertEquals("baseline", event2.getAttributes().getAttributeMap().get("deployment").getValue()); Event event3 = Event.newBuilder() @@ -122,6 +124,8 @@ public void traceWithResource() { event3.setResourceIndex(2); resourceAttributeEnricher.enrichEvent(structuredTrace, event3); assertEquals("", event3.getAttributes().getAttributeMap().get("node.selector").getValue()); + assertEquals("test1-worker-56f5d554c-5swkj", event3.getAttributes().getAttributeMap().get("pod.name").getValue()); + assertEquals("worker", event3.getAttributes().getAttributeMap().get("deployment").getValue()); Event event4 = Event.newBuilder() @@ -159,6 +163,7 @@ private Resource getResource3() { AttributeValue.newBuilder() .setValue("node-role.kubernetes.io/worker-generic/") .build()); + put("host.name", AttributeValue.newBuilder().setValue("test1-worker-56f5d554c-5swkj").build()); } }; return Resource.newBuilder() @@ -179,7 +184,7 @@ private Resource getResource2() { put( "opencensus.exporterversion", AttributeValue.newBuilder().setValue("Jaeger-Go-2.23.1").build()); - put("host.name", AttributeValue.newBuilder().setValue("test1-56f5d554c-5swkj").build()); + put("host.name", AttributeValue.newBuilder().setValue("test1-baseline-56f5d554c-5swkj").build()); put("ip", AttributeValue.newBuilder().setValue("10.21.18.1712").build()); put("client-uuid", AttributeValue.newBuilder().setValue("53a112a715bdf86").build()); put("node.name", AttributeValue.newBuilder().setValue("worker-generic").build()); From 26b40e2487a3d4fd8ad69bf3889eba3dc02f4977 Mon Sep 17 00:00:00 2001 From: Sakshi Goyal Date: Wed, 21 Dec 2022 10:25:39 +0530 Subject: [PATCH 4/7] spotless java check fix --- .../enrichers/ResourceAttributeEnricher.java | 37 +++++++++---------- .../ResourceAttributeEnricherTest.java | 7 +++- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java index c033a4720..c707e40da 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java @@ -11,7 +11,6 @@ import org.hypertrace.core.datamodel.StructuredTrace; import org.hypertrace.core.datamodel.shared.trace.AttributeValueCreator; import org.hypertrace.traceenricher.enrichedspan.constants.EnrichedSpanConstants; -import org.hypertrace.traceenricher.enrichedspan.constants.v1.Backend; import org.hypertrace.traceenricher.enrichedspan.constants.v1.Deployment; import org.hypertrace.traceenricher.enrichment.AbstractTraceEnricher; import org.hypertrace.traceenricher.enrichment.clients.ClientRegistry; @@ -74,16 +73,16 @@ public void enrichEvent(StructuredTrace trace, Event event) { })); // Add deployment attribute if pod name is available - if (resourceAttributeKey.equals(POD_NAME_KEY)) { - resourceAttributeMaybe.ifPresent( - attributeValue -> - attributeMap.computeIfAbsent( - DEPLOYMENT_KEY, - key -> { - return AttributeValueCreator - .create(getDeploymentType(attributeValue.getValue())); - })); - } + if (resourceAttributeKey.equals(POD_NAME_KEY)) { + resourceAttributeMaybe.ifPresent( + attributeValue -> + attributeMap.computeIfAbsent( + DEPLOYMENT_KEY, + key -> { + return AttributeValueCreator.create( + getDeploymentType(attributeValue.getValue())); + })); + } } } catch (Exception e) { LOGGER.error( @@ -100,13 +99,13 @@ private boolean isValidEvent(Event event) { } private String getDeploymentType(String hostName) { - if (hostName.contains(EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_CANARY))) { - return EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_CANARY); - } else if (hostName.contains(EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_BASELINE))) { - return EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_BASELINE); - } else if (hostName.contains(EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_WORKER))) { - return EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_WORKER); - } - return EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_WEB); + if (hostName.contains(EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_CANARY))) { + return EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_CANARY); + } else if (hostName.contains(EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_BASELINE))) { + return EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_BASELINE); + } else if (hostName.contains(EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_WORKER))) { + return EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_WORKER); + } + return EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_WEB); } } diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java index 754f46deb..a7c6f4384 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java @@ -83,7 +83,8 @@ public void traceWithResource() { assertEquals( resourceAttributesToAddList.size() - 1, event.getAttributes().getAttributeMap().size()); assertEquals( - "test-canary-56f5d554c-5swkj", event.getAttributes().getAttributeMap().get("pod.name").getValue()); + "test-canary-56f5d554c-5swkj", + event.getAttributes().getAttributeMap().get("pod.name").getValue()); assertEquals("canary", event.getAttributes().getAttributeMap().get("deployment").getValue()); assertEquals( "01188498a468b5fef1eb4accd63533297c195a73", @@ -217,7 +218,9 @@ private Resource getResource1() { put( "opencensus.exporterversion", AttributeValue.newBuilder().setValue("Jaeger-Go-2.23.1").build()); - put("host.name", AttributeValue.newBuilder().setValue("test-canary-56f5d554c-5swkj").build()); + put( + "host.name", + AttributeValue.newBuilder().setValue("test-canary-56f5d554c-5swkj").build()); put("ip", AttributeValue.newBuilder().setValue("10.21.18.171").build()); put("client-uuid", AttributeValue.newBuilder().setValue("53a112a715bda986").build()); put( From 92421e67ad37db9692718e834b51d6d69c318543 Mon Sep 17 00:00:00 2001 From: Sakshi Goyal Date: Wed, 21 Dec 2022 10:34:22 +0530 Subject: [PATCH 5/7] spotless check fixes --- .../enrichers/ResourceAttributeEnricherTest.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java index a7c6f4384..f7aba8205 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java @@ -113,7 +113,9 @@ public void traceWithResource() { "worker-generic", event2.getAttributes().getAttributeMap().get("node.name").getValue()); assertEquals( "worker-generic", event2.getAttributes().getAttributeMap().get("node.selector").getValue()); - assertEquals("test1-baseline-56f5d554c-5swkj", event2.getAttributes().getAttributeMap().get("pod.name").getValue()); + assertEquals( + "test1-baseline-56f5d554c-5swkj", + event2.getAttributes().getAttributeMap().get("pod.name").getValue()); assertEquals("baseline", event2.getAttributes().getAttributeMap().get("deployment").getValue()); Event event3 = @@ -125,7 +127,9 @@ public void traceWithResource() { event3.setResourceIndex(2); resourceAttributeEnricher.enrichEvent(structuredTrace, event3); assertEquals("", event3.getAttributes().getAttributeMap().get("node.selector").getValue()); - assertEquals("test1-worker-56f5d554c-5swkj", event3.getAttributes().getAttributeMap().get("pod.name").getValue()); + assertEquals( + "test1-worker-56f5d554c-5swkj", + event3.getAttributes().getAttributeMap().get("pod.name").getValue()); assertEquals("worker", event3.getAttributes().getAttributeMap().get("deployment").getValue()); Event event4 = @@ -164,7 +168,9 @@ private Resource getResource3() { AttributeValue.newBuilder() .setValue("node-role.kubernetes.io/worker-generic/") .build()); - put("host.name", AttributeValue.newBuilder().setValue("test1-worker-56f5d554c-5swkj").build()); + put( + "host.name", + AttributeValue.newBuilder().setValue("test1-worker-56f5d554c-5swkj").build()); } }; return Resource.newBuilder() @@ -185,7 +191,9 @@ private Resource getResource2() { put( "opencensus.exporterversion", AttributeValue.newBuilder().setValue("Jaeger-Go-2.23.1").build()); - put("host.name", AttributeValue.newBuilder().setValue("test1-baseline-56f5d554c-5swkj").build()); + put( + "host.name", + AttributeValue.newBuilder().setValue("test1-baseline-56f5d554c-5swkj").build()); put("ip", AttributeValue.newBuilder().setValue("10.21.18.1712").build()); put("client-uuid", AttributeValue.newBuilder().setValue("53a112a715bdf86").build()); put("node.name", AttributeValue.newBuilder().setValue("worker-generic").build()); From b44dd3993f016e40217df6050d232e7d82e51b06 Mon Sep 17 00:00:00 2001 From: Sakshi Goyal Date: Wed, 21 Dec 2022 16:06:50 +0530 Subject: [PATCH 6/7] review comments --- .../v1/enriched_span_attribute.proto | 4 +-- .../enrichers/ResourceAttributeEnricher.java | 31 ++++++------------- .../ResourceAttributeEnricherTest.java | 15 +++++---- .../src/test/resources/enricher.conf | 3 +- .../resources/configs/common/application.conf | 3 +- 5 files changed, 25 insertions(+), 31 deletions(-) diff --git a/hypertrace-trace-enricher/enriched-span-constants/src/main/proto/org/hypertrace/traceenricher/enrichedspan/constants/v1/enriched_span_attribute.proto b/hypertrace-trace-enricher/enriched-span-constants/src/main/proto/org/hypertrace/traceenricher/enrichedspan/constants/v1/enriched_span_attribute.proto index 5ab1aaac2..119efa391 100644 --- a/hypertrace-trace-enricher/enriched-span-constants/src/main/proto/org/hypertrace/traceenricher/enrichedspan/constants/v1/enriched_span_attribute.proto +++ b/hypertrace-trace-enricher/enriched-span-constants/src/main/proto/org/hypertrace/traceenricher/enrichedspan/constants/v1/enriched_span_attribute.proto @@ -99,6 +99,6 @@ enum ErrorMetrics { enum Deployment { DEPLOYMENT_CANARY = 0 [(string_value) = "canary"]; DEPLOYMENT_BASELINE = 1 [(string_value) = "baseline"]; - DEPLOYMENT_WEB = 2 [(string_value) = "web"]; - DEPLOYMENT_WORKER = 3 [(string_value) = "worker"]; + DEPLOYMENT_WORKER = 2 [(string_value) = "worker"]; + DEPLOYMENT_WEB = 3 [(string_value) = "web"]; } diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java index c707e40da..70ba6af1c 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java @@ -27,9 +27,7 @@ public class ResourceAttributeEnricher extends AbstractTraceEnricher { private static final String RESOURCE_ATTRIBUTES_CONFIG_KEY = "attributes"; private static final String NODE_SELECTOR_KEY = "node.selector"; - private static final String DEPLOYMENT_KEY = "deployment"; - - private static final String POD_NAME_KEY = "pod.name"; + private static final String DEPLOYMENT_TYPE_KEY = "deployment.type"; private static final String ATTRIBUTES_TO_MATCH_CONFIG_KEY = "attributesToMatch"; private List resourceAttributesToAdd = new ArrayList<>(); @@ -68,21 +66,12 @@ public void enrichEvent(StructuredTrace trace, Event event) { attributeValue .getValue() .substring(attributeValue.getValue().lastIndexOf('/') + 1)); + } else if (resourceAttributeKey.equals(DEPLOYMENT_TYPE_KEY)) { + return AttributeValueCreator.create( + getDeploymentType(attributeValue.getValue())); } return attributeValue; })); - - // Add deployment attribute if pod name is available - if (resourceAttributeKey.equals(POD_NAME_KEY)) { - resourceAttributeMaybe.ifPresent( - attributeValue -> - attributeMap.computeIfAbsent( - DEPLOYMENT_KEY, - key -> { - return AttributeValueCreator.create( - getDeploymentType(attributeValue.getValue())); - })); - } } } catch (Exception e) { LOGGER.error( @@ -99,12 +88,12 @@ private boolean isValidEvent(Event event) { } private String getDeploymentType(String hostName) { - if (hostName.contains(EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_CANARY))) { - return EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_CANARY); - } else if (hostName.contains(EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_BASELINE))) { - return EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_BASELINE); - } else if (hostName.contains(EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_WORKER))) { - return EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_WORKER); + for (Deployment d : Deployment.values()) { + if (d == Deployment.UNRECOGNIZED) { + break; + } else if (hostName.contains(EnrichedSpanConstants.getValue(d))) { + return EnrichedSpanConstants.getValue(d); + } } return EnrichedSpanConstants.getValue(Deployment.DEPLOYMENT_WEB); } diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java index f7aba8205..5241ffde1 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricherTest.java @@ -81,11 +81,12 @@ public void traceWithResource() { event.setResourceIndex(0); resourceAttributeEnricher.enrichEvent(structuredTrace, event); assertEquals( - resourceAttributesToAddList.size() - 1, event.getAttributes().getAttributeMap().size()); + resourceAttributesToAddList.size() - 2, event.getAttributes().getAttributeMap().size()); assertEquals( "test-canary-56f5d554c-5swkj", event.getAttributes().getAttributeMap().get("pod.name").getValue()); - assertEquals("canary", event.getAttributes().getAttributeMap().get("deployment").getValue()); + assertEquals( + "canary", event.getAttributes().getAttributeMap().get("deployment.type").getValue()); assertEquals( "01188498a468b5fef1eb4accd63533297c195a73", event.getAttributes().getAttributeMap().get("service.version").getValue()); @@ -105,7 +106,7 @@ public void traceWithResource() { addAttribute(event2, "cluster.name", "default"); resourceAttributeEnricher.enrichEvent(structuredTrace, event2); assertEquals( - resourceAttributesToAddList.size() + 1, event2.getAttributes().getAttributeMap().size()); + resourceAttributesToAddList.size(), event2.getAttributes().getAttributeMap().size()); assertEquals("123", event2.getAttributes().getAttributeMap().get("service.version").getValue()); assertEquals( "default", event2.getAttributes().getAttributeMap().get("cluster.name").getValue()); @@ -116,7 +117,8 @@ public void traceWithResource() { assertEquals( "test1-baseline-56f5d554c-5swkj", event2.getAttributes().getAttributeMap().get("pod.name").getValue()); - assertEquals("baseline", event2.getAttributes().getAttributeMap().get("deployment").getValue()); + assertEquals( + "baseline", event2.getAttributes().getAttributeMap().get("deployment.type").getValue()); Event event3 = Event.newBuilder() @@ -130,7 +132,8 @@ public void traceWithResource() { assertEquals( "test1-worker-56f5d554c-5swkj", event3.getAttributes().getAttributeMap().get("pod.name").getValue()); - assertEquals("worker", event3.getAttributes().getAttributeMap().get("deployment").getValue()); + assertEquals( + "worker", event3.getAttributes().getAttributeMap().get("deployment.type").getValue()); Event event4 = Event.newBuilder() @@ -143,7 +146,7 @@ public void traceWithResource() { assertEquals( "worker-generic", event4.getAttributes().getAttributeMap().get("node.selector").getValue()); assertEquals("pod1", event4.getAttributes().getAttributeMap().get("pod.name").getValue()); - assertEquals("web", event4.getAttributes().getAttributeMap().get("deployment").getValue()); + assertEquals("web", event4.getAttributes().getAttributeMap().get("deployment.type").getValue()); } private Resource getResource4() { diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/resources/enricher.conf b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/resources/enricher.conf index 2b92ddd01..dff480ab4 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/resources/enricher.conf +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/test/resources/enricher.conf @@ -69,9 +69,10 @@ enricher { ResourceAttributeEnricher { class = "org.hypertrace.traceenricher.enrichment.enrichers.ResourceAttributeEnricher" - attributes = ["pod.name","node.name","cluster.name","ip","service.version","node.selector"] + attributes = ["pod.name","node.name","cluster.name","ip","service.version","node.selector","deployment.type"] attributesToMatch { pod.name = "host.name" + deployment.type = "host.name" } } diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher/src/main/resources/configs/common/application.conf b/hypertrace-trace-enricher/hypertrace-trace-enricher/src/main/resources/configs/common/application.conf index cd42572c7..3c39c0a56 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher/src/main/resources/configs/common/application.conf +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher/src/main/resources/configs/common/application.conf @@ -123,9 +123,10 @@ enricher { ResourceAttributeEnricher { class = "org.hypertrace.traceenricher.enrichment.enrichers.ResourceAttributeEnricher" - attributes = ["pod.name","node.name","cluster.name","ip","service.version","node.selector"] + attributes = ["pod.name","node.name","cluster.name","ip","service.version","node.selector","deployment.type"] attributesToMatch { pod.name = "host.name" + deployment.type = "host.name" } } From 791ef83ce6a8be275ef40dd845b59cff601f8f40 Mon Sep 17 00:00:00 2001 From: Sakshi Goyal Date: Thu, 22 Dec 2022 13:30:05 +0530 Subject: [PATCH 7/7] review comments --- .../enrichers/ResourceAttributeEnricher.java | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java index 70ba6af1c..e979c7634 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ResourceAttributeEnricher.java @@ -61,16 +61,18 @@ public void enrichEvent(StructuredTrace trace, Event event) { attributeMap.computeIfAbsent( resourceAttributeKey, key -> { - if (resourceAttributeKey.equals(NODE_SELECTOR_KEY)) { - attributeValue.setValue( - attributeValue - .getValue() - .substring(attributeValue.getValue().lastIndexOf('/') + 1)); - } else if (resourceAttributeKey.equals(DEPLOYMENT_TYPE_KEY)) { - return AttributeValueCreator.create( - getDeploymentType(attributeValue.getValue())); + switch (resourceAttributeKey) { + case DEPLOYMENT_TYPE_KEY: + return AttributeValueCreator.create( + getDeploymentType(attributeValue.getValue())); + case NODE_SELECTOR_KEY: + attributeValue.setValue( + attributeValue + .getValue() + .substring(attributeValue.getValue().lastIndexOf('/') + 1)); + default: + return attributeValue; } - return attributeValue; })); } } catch (Exception e) { @@ -88,6 +90,10 @@ private boolean isValidEvent(Event event) { } private String getDeploymentType(String hostName) { + /* + There can be applications which have canary/baseline workers. (eg: worker-canary, worker-baseline) + These rare cases are not handled for now. + */ for (Deployment d : Deployment.values()) { if (d == Deployment.UNRECOGNIZED) { break;