From 74e7b90a5340813bee12f7110219c33ab68fed4c Mon Sep 17 00:00:00 2001 From: Laxman Ch Date: Wed, 27 Oct 2021 12:09:18 +0530 Subject: [PATCH 1/3] apply hypertrace codestyle --- build.gradle.kts | 3 + .../core/datamodel/shared/ApiNode.java | 11 +- .../datamodel/shared/AttributeSearch.java | 11 +- .../datamodel/shared/DataflowMetricUtils.java | 39 ++-- .../datamodel/shared/SpanAttributeUtils.java | 65 +++--- .../shared/StructuredTraceGraph.java | 2 +- .../datamodel/shared/TraceAttributeUtils.java | 15 +- .../datamodel/shared/TraceEntitiesGraph.java | 16 +- .../datamodel/shared/TraceEventsGraph.java | 28 +-- .../shared/trace/AttributeValueCreator.java | 7 +- .../shared/trace/StructuredTraceBuilder.java | 188 +++++++++--------- .../shared/DataflowMetricUtilsTest.java | 21 +- .../shared/StructuredTraceGraphTest.java | 165 +++++++++------ 13 files changed, 304 insertions(+), 267 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 0717695..18c1ffa 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -3,6 +3,7 @@ import org.hypertrace.gradle.publishing.License plugins { id("org.hypertrace.repository-plugin") version "0.4.0" + id("org.hypertrace.code-style-plugin") version "1.1.1" apply false id("org.hypertrace.ci-utils-plugin") version "0.3.0" id("org.hypertrace.publish-plugin") version "1.0.2" apply false id("org.hypertrace.jacoco-report-plugin") version "0.2.0" apply false @@ -15,4 +16,6 @@ subprojects { license.set(License.TRACEABLE_COMMUNITY) } } + + apply(plugin = "org.hypertrace.code-style-plugin") } diff --git a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/ApiNode.java b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/ApiNode.java index d3d7808..bdb8382 100644 --- a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/ApiNode.java +++ b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/ApiNode.java @@ -15,10 +15,11 @@ public class ApiNode { /** * This creates an API Node(surprise) which contains all the events under an API call within a * single service. The call could be external or internal. + * * @param headEvent: The event that defines the API call. The head of the API Trace. * @param events: All events under the API call including the head event. * @param apiEntryEvent: The Entry API event. Equals the head event if the head event is an Entry - * Event. Otherwise it's null. + * Event. Otherwise it's null. * @param exitEvents: The exit events into another service and API from this API call. */ public ApiNode(T headEvent, List events, T apiEntryEvent, List exitEvents) { @@ -61,10 +62,10 @@ public boolean equals(Object o) { return false; } ApiNode apiNode = (ApiNode) o; - return Objects.equals(entryApiBoundaryEvent, apiNode.entryApiBoundaryEvent) && - Objects.equals(exitApiBoundaryEvents, apiNode.exitApiBoundaryEvents) && - Objects.equals(headEvent, apiNode.headEvent) && - Objects.equals(events, apiNode.events); + return Objects.equals(entryApiBoundaryEvent, apiNode.entryApiBoundaryEvent) + && Objects.equals(exitApiBoundaryEvents, apiNode.exitApiBoundaryEvents) + && Objects.equals(headEvent, apiNode.headEvent) + && Objects.equals(events, apiNode.events); } @Override diff --git a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/AttributeSearch.java b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/AttributeSearch.java index 9dac816..1dd63f1 100644 --- a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/AttributeSearch.java +++ b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/AttributeSearch.java @@ -15,15 +15,14 @@ public static Optional searchForAttribute(Attributes attributes, String return Optional.empty(); } - public static Optional searchForAttributeIgnoreKeyCase(Attributes attributes, - String key) { + public static Optional searchForAttributeIgnoreKeyCase( + Attributes attributes, String key) { if (attributes != null && attributes.getAttributeMap() != null) { - Optional attributeKey = attributes.getAttributeMap().keySet().stream() - .filter(key::equalsIgnoreCase).findFirst(); + Optional attributeKey = + attributes.getAttributeMap().keySet().stream().filter(key::equalsIgnoreCase).findFirst(); if (attributeKey.isPresent()) { - return Optional.of(attributes.getAttributeMap() - .get(attributeKey.get()).getValue()); + return Optional.of(attributes.getAttributeMap().get(attributeKey.get()).getValue()); } } return Optional.empty(); diff --git a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/DataflowMetricUtils.java b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/DataflowMetricUtils.java index 7791f77..4ab830c 100644 --- a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/DataflowMetricUtils.java +++ b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/DataflowMetricUtils.java @@ -1,54 +1,59 @@ package org.hypertrace.core.datamodel.shared; import io.micrometer.core.instrument.Timer; +import java.util.concurrent.TimeUnit; import org.hypertrace.core.datamodel.StructuredTrace; import org.hypertrace.core.datamodel.TimestampRecord; -import java.util.concurrent.TimeUnit; - -/** - * Utility methods to track flow of data and its lag across the services and data pipelines. - */ +/** Utility methods to track flow of data and its lag across the services and data pipelines. */ public final class DataflowMetricUtils { public static final String SPAN_ARRIVAL_TIME = "span.arrival.time"; public static final String ARRIVAL_LAG = "arrival.lag"; /** - * Compares current time against span arrival time of trace and reports that a lag via supplied timer. + * Compares current time against span arrival time of trace and reports that a lag via supplied + * timer. + * * @param trace whose with creation time. * @param timer which will be used to report the lag. */ public static void reportArrivalLag(StructuredTrace trace, Timer timer) { - if (trace.getTimestamps() != null && trace.getTimestamps().getRecords() - .containsKey(SPAN_ARRIVAL_TIME)) { - timer.record(System.currentTimeMillis() - - trace.getTimestamps().getRecords().get(SPAN_ARRIVAL_TIME).getTimestamp(), + if (trace.getTimestamps() != null + && trace.getTimestamps().getRecords().containsKey(SPAN_ARRIVAL_TIME)) { + timer.record( + System.currentTimeMillis() + - trace.getTimestamps().getRecords().get(SPAN_ARRIVAL_TIME).getTimestamp(), TimeUnit.MILLISECONDS); } } /** - * Inserts given metric in the trace timeStamps record, with timestamp as current time. - * If insert new TimestampRecord only when SPAN_ARRIVAL_TIME is already present. + * Inserts given metric in the trace timeStamps record, with timestamp as current time. If insert + * new TimestampRecord only when SPAN_ARRIVAL_TIME is already present. + * * @param trace in which timestamp record will be added. * @param metricName against for which timestamp will be added. */ public static void insertTimestamp(StructuredTrace trace, String metricName) { - if (trace.getTimestamps() != null && trace.getTimestamps().getRecords() - .containsKey(SPAN_ARRIVAL_TIME)) { - trace.getTimestamps().getRecords().put(metricName, new TimestampRecord(metricName, System.currentTimeMillis())); + if (trace.getTimestamps() != null + && trace.getTimestamps().getRecords().containsKey(SPAN_ARRIVAL_TIME)) { + trace + .getTimestamps() + .getRecords() + .put(metricName, new TimestampRecord(metricName, System.currentTimeMillis())); } } /** * Wrapper to call reportArrivalLag and insertTimestamp. + * * @param trace trace * @param timer times * @param metricName metric */ - public static void reportArrivalLagAndInsertTimestamp(StructuredTrace trace, Timer timer, - String metricName) { + public static void reportArrivalLagAndInsertTimestamp( + StructuredTrace trace, Timer timer, String metricName) { reportArrivalLag(trace, timer); insertTimestamp(trace, metricName); } diff --git a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/SpanAttributeUtils.java b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/SpanAttributeUtils.java index cbf3782..0579468 100644 --- a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/SpanAttributeUtils.java +++ b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/SpanAttributeUtils.java @@ -24,18 +24,18 @@ public static boolean isLeafSpan(StructuredTraceGraph structuredTraceGraph, Even return childEvents == null || childEvents.isEmpty(); } - public static String getStringAttributeWithDefault(Event event, String attributeKey, - String defaultValue) { + public static String getStringAttributeWithDefault( + Event event, String attributeKey, String defaultValue) { String value = getStringAttribute(event, attributeKey); return value == null ? defaultValue : value; } - public static AttributeValue getAttributeValueWithDefault(Event event, String attributeKey, - String defaultValue) { + public static AttributeValue getAttributeValueWithDefault( + Event event, String attributeKey, String defaultValue) { AttributeValue attributeValue = getAttributeValue(event, attributeKey); - return attributeValue == null ? - AttributeValue.newBuilder().setValue(defaultValue).build() : - attributeValue; + return attributeValue == null + ? AttributeValue.newBuilder().setValue(defaultValue).build() + : attributeValue; } public static AttributeValue getAttributeValue(Event event, String attributeKey) { @@ -53,8 +53,8 @@ public static AttributeValue getAttributeValue(Event event, String attributeKey) public static AttributeValue getAttributeValue(Event.Builder eventBuilder, String attributeKey) { if (eventBuilder.getEnrichedAttributes() != null) { - AttributeValue value = eventBuilder.getEnrichedAttributes().getAttributeMap() - .get(attributeKey); + AttributeValue value = + eventBuilder.getEnrichedAttributes().getAttributeMap().get(attributeKey); if (value != null) { return value; } @@ -76,8 +76,9 @@ public static Optional getStringAttributeIgnoreKeyCase(Event event, Stri return Optional.empty(); } if (event.getEnrichedAttributes() != null) { - Optional value = AttributeSearch - .searchForAttributeIgnoreKeyCase(event.getEnrichedAttributes(), attributeKey); + Optional value = + AttributeSearch.searchForAttributeIgnoreKeyCase( + event.getEnrichedAttributes(), attributeKey); if (value.isPresent()) { return value; } @@ -131,18 +132,18 @@ private static boolean containsAttributeKey(Attributes attributes, String attrKe && attributes.getAttributeMap().containsKey(attrKey); } - public static Map getAttributesWithPrefixKey(Event event, - String attributeKeyPrefix) { - if (event.getEnrichedAttributes() != null && - event.getEnrichedAttributes().getAttributeMap() != null - ) { - Map enrichedAttributeMap = event.getEnrichedAttributes() - .getAttributeMap(); - Map filteredEnrichedAttributeMap = enrichedAttributeMap.entrySet() - .stream() - .filter( - entry -> entry.getKey().toLowerCase().startsWith(attributeKeyPrefix.toLowerCase())) - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + public static Map getAttributesWithPrefixKey( + Event event, String attributeKeyPrefix) { + if (event.getEnrichedAttributes() != null + && event.getEnrichedAttributes().getAttributeMap() != null) { + Map enrichedAttributeMap = + event.getEnrichedAttributes().getAttributeMap(); + Map filteredEnrichedAttributeMap = + enrichedAttributeMap.entrySet().stream() + .filter( + entry -> + entry.getKey().toLowerCase().startsWith(attributeKeyPrefix.toLowerCase())) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); if (!filteredEnrichedAttributeMap.isEmpty()) { return filteredEnrichedAttributeMap; @@ -151,11 +152,12 @@ public static Map getAttributesWithPrefixKey(Event event if (event.getAttributes() != null && event.getAttributes().getAttributeMap() != null) { Map attributeMap = event.getAttributes().getAttributeMap(); - Map filteredAttributeMap = attributeMap.entrySet() - .stream() - .filter( - entry -> entry.getKey().toLowerCase().startsWith(attributeKeyPrefix.toLowerCase())) - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + Map filteredAttributeMap = + attributeMap.entrySet().stream() + .filter( + entry -> + entry.getKey().toLowerCase().startsWith(attributeKeyPrefix.toLowerCase())) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); if (!filteredAttributeMap.isEmpty()) { return filteredAttributeMap; @@ -165,9 +167,10 @@ public static Map getAttributesWithPrefixKey(Event event } @Nullable - public static Event getParentSpan(Event span, - Map childToParentEventIds, - Map idToEvent) { + public static Event getParentSpan( + Event span, + Map childToParentEventIds, + Map idToEvent) { ByteBuffer parentSpanId = childToParentEventIds.get(span.getEventId()); return parentSpanId != null ? idToEvent.get(parentSpanId) : null; } diff --git a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/StructuredTraceGraph.java b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/StructuredTraceGraph.java index e664859..705c3a6 100644 --- a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/StructuredTraceGraph.java +++ b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/StructuredTraceGraph.java @@ -77,4 +77,4 @@ public TraceEventsGraph getTraceEventsGraph() { public TraceEntitiesGraph getTraceEntitiesGraph() { return traceEntitiesGraph; } -} \ No newline at end of file +} diff --git a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/TraceAttributeUtils.java b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/TraceAttributeUtils.java index 8d13eb2..c0fc0e5 100644 --- a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/TraceAttributeUtils.java +++ b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/TraceAttributeUtils.java @@ -5,9 +5,7 @@ import org.hypertrace.core.datamodel.Event; import org.hypertrace.core.datamodel.StructuredTrace; -/** - * Convenience methods to deal with StructuredTrace attributes. - */ +/** Convenience methods to deal with StructuredTrace attributes. */ public class TraceAttributeUtils { public static boolean containsAttribute(StructuredTrace trace, String attribute) { @@ -33,20 +31,21 @@ public static String getStringAttribute(StructuredTrace trace, String attribute) return value.getValue(); } - public static Optional getAttributeValueIncludeSearchInSpans(StructuredTrace trace, - String key) { + public static Optional getAttributeValueIncludeSearchInSpans( + StructuredTrace trace, String key) { if (trace == null) { return Optional.empty(); } - Optional value = AttributeSearch - .searchForAttributeIgnoreKeyCase(trace.getAttributes(), key); + Optional value = + AttributeSearch.searchForAttributeIgnoreKeyCase(trace.getAttributes(), key); if (value.isPresent()) { return value; } for (Event event : trace.getEventList()) { - // SpanAttributeUtils.getStringAttributeIgnoreKeyCase searches in both attributes and enriched attributes + // SpanAttributeUtils.getStringAttributeIgnoreKeyCase searches in both attributes and enriched + // attributes value = SpanAttributeUtils.getStringAttributeIgnoreKeyCase(event, key); if (value.isPresent()) { return value; diff --git a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/TraceEntitiesGraph.java b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/TraceEntitiesGraph.java index 34244e4..333dc3d 100644 --- a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/TraceEntitiesGraph.java +++ b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/TraceEntitiesGraph.java @@ -30,26 +30,20 @@ public class TraceEntitiesGraph { processEntities(trace); } - /** - * @return an immutable set containing the root entities - */ + /** @return an immutable set containing the root entities */ public Set getRootEntities() { return rootEntities; } - public List getParentEntities(Entity entity) { return childToParentEntities.get(entity.getEntityId()); } - public List getChildrenEntities(Entity entity) { return parentToChildrenEntities.get(entity.getEntityId()); } - /** - * @return an immutable map of entity ids to entities - */ + /** @return an immutable map of entity ids to entities */ public Map getEntityMap() { return entityMap; } @@ -69,9 +63,11 @@ private void buildParentChildRelationship(StructuredTrace trace) { Integer targetIndex = entityEdge.getTgtIndex(); Entity parentEntity = entities.get(sourceIndex); Entity childEntity = entities.get(targetIndex); - parentToChildrenEntities.computeIfAbsent(parentEntity.getEntityId(), k -> new ArrayList<>()) + parentToChildrenEntities + .computeIfAbsent(parentEntity.getEntityId(), k -> new ArrayList<>()) .add(childEntity); - childToParentEntities.computeIfAbsent(childEntity.getEntityId(), k -> new ArrayList<>()) + childToParentEntities + .computeIfAbsent(childEntity.getEntityId(), k -> new ArrayList<>()) .add(parentEntity); } } diff --git a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/TraceEventsGraph.java b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/TraceEventsGraph.java index 885a710..b0fddcb 100644 --- a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/TraceEventsGraph.java +++ b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/TraceEventsGraph.java @@ -9,7 +9,6 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; -import java.util.function.Function; import java.util.stream.Collectors; import org.hypertrace.core.datamodel.Edge; import org.hypertrace.core.datamodel.Event; @@ -34,19 +33,15 @@ public class TraceEventsGraph { processEvents(trace); } - /** - * @return an immutable set containing the root events - */ + /** @return an immutable set containing the root events */ public Set getRootEvents() { return rootEvents; } - public Event getParentEvent(Event event) { return childToParentEvents.get(event.getEventId()); } - public List getChildrenEvents(Event event) { return parentToChildrenEvents.get(event.getEventId()); } @@ -65,10 +60,10 @@ private void buildParentChildRelationship(StructuredTrace trace) { int targetIndex = edge.getTgtIndex(); Event parentEvent = events.get(sourceIndex); Event childEvent = events.get(targetIndex); - parentToChildrenEvents.computeIfAbsent(parentEvent.getEventId(), k -> new ArrayList<>()) + parentToChildrenEvents + .computeIfAbsent(parentEvent.getEventId(), k -> new ArrayList<>()) .add(childEvent); childToParentEvents.put(childEvent.getEventId(), parentEvent); - } } } @@ -84,27 +79,22 @@ private void processEvents(StructuredTrace trace) { } } - /** - * @return an immutable map of event ids to events - */ + /** @return an immutable map of event ids to events */ public Map getEventMap() { return eventMap; } public Map getChildIdsToParentIds() { return childToParentEvents.entrySet().stream() - .collect(Collectors.toMap( - Entry::getKey, - e -> getEventId(e.getValue()) - )); + .collect(Collectors.toMap(Entry::getKey, e -> getEventId(e.getValue()))); } public Map> getParentToChildEventIds() { return parentToChildrenEvents.entrySet().stream() - .collect(Collectors.toMap( - Entry::getKey, - e -> e.getValue().stream().map(this::getEventId).collect(Collectors.toList())) - ); + .collect( + Collectors.toMap( + Entry::getKey, + e -> e.getValue().stream().map(this::getEventId).collect(Collectors.toList()))); } private ByteBuffer getEventId(Event event) { diff --git a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/AttributeValueCreator.java b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/AttributeValueCreator.java index 336c682..9937ded 100644 --- a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/AttributeValueCreator.java +++ b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/AttributeValueCreator.java @@ -27,9 +27,10 @@ public static AttributeValue create(List values) { public static AttributeValue createFromByteBuffers(Set values) { List list = new ArrayList<>(); - values.forEach(value -> { - list.add(new String(HexUtils.getBytes(value))); - }); + values.forEach( + value -> { + list.add(new String(HexUtils.getBytes(value))); + }); return AttributeValue.newBuilder().setValueList(list).build(); } } diff --git a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/StructuredTraceBuilder.java b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/StructuredTraceBuilder.java index 14ad1a7..042fb4c 100644 --- a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/StructuredTraceBuilder.java +++ b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/StructuredTraceBuilder.java @@ -22,7 +22,6 @@ import org.hypertrace.core.datamodel.Entity; import org.hypertrace.core.datamodel.Event; import org.hypertrace.core.datamodel.EventRef; -import org.hypertrace.core.datamodel.EventRefType; import org.hypertrace.core.datamodel.MetricValue; import org.hypertrace.core.datamodel.Metrics; import org.hypertrace.core.datamodel.RawSpan; @@ -36,8 +35,7 @@ public class StructuredTraceBuilder { - private static final Logger LOGGER = - LoggerFactory.getLogger(StructuredTraceBuilder.class); + private static final Logger LOGGER = LoggerFactory.getLogger(StructuredTraceBuilder.class); private long traceStartTime; private long traceEndTime; private final String customerId; @@ -90,7 +88,7 @@ public StructuredTrace buildStructuredTrace() { long execStartTime = System.currentTimeMillis(); for (Event event : eventList) { - //Create the event Entity Map + // Create the event Entity Map eventMap.put(event.getEventId(), event); if (event.getStartTimeMillis() < traceStartTime) { traceStartTime = event.getStartTimeMillis(); @@ -100,22 +98,22 @@ public StructuredTrace buildStructuredTrace() { } } - //process the event ref's and build edges + // process the event ref's and build edges for (Event event : eventMap.values()) { for (String entityId : event.getEntityIdList()) { entityEventConnectionMap.putIfAbsent(entityId, new ArrayList<>()); entityEventConnectionMap.get(entityId).add(event.getEventId()); } - //create entity->entity Edge and event->event edge + // create entity->entity Edge and event->event edge processEventRefList(traceId, event); } - //Find root nodes for event and entity - //First add all nodes as root nodes - //then iterate through all edges and remove the child nodes - //at the end only the nodes without parents will be left. - //NOTE: the reason we can't just pick nodes with node spanRef as there can be missing spans - //So its possible to have multiple root nodes when isPartialTrace = true + // Find root nodes for event and entity + // First add all nodes as root nodes + // then iterate through all edges and remove the child nodes + // at the end only the nodes without parents will be left. + // NOTE: the reason we can't just pick nodes with node spanRef as there can be missing spans + // So its possible to have multiple root nodes when isPartialTrace = true Set rootEntityNodes = new HashSet<>(entityMap.keySet()); for (Entry> entry : entityEntityConnectionMap.entrySet()) { for (String entityId : entry.getValue()) { @@ -131,11 +129,11 @@ public StructuredTrace buildStructuredTrace() { } ArrayList rootEventNodesOrdered = new ArrayList<>(rootEventNodes); - //sort the rootEventNodes by time. Ideally, there should be only one root Event Node - //We can see multiple root nodes if some span events are missing from the trace + // sort the rootEventNodes by time. Ideally, there should be only one root Event Node + // We can see multiple root nodes if some span events are missing from the trace if (rootEventNodes.size() > 0) { - rootEventNodesOrdered - .sort(Comparator.comparingLong(e -> eventMap.get(e).getStartTimeMillis())); + rootEventNodesOrdered.sort( + Comparator.comparingLong(e -> eventMap.get(e).getStartTimeMillis())); } orderedEventNodes = new ArrayList<>(); @@ -144,10 +142,10 @@ public StructuredTrace buildStructuredTrace() { int eventIndex = 0; int entityIndex = 0; - //BFS order traversal + // BFS order traversal for (ByteBuffer rootEventId : rootEventNodesOrdered) { - //Build Graph for each root node - //queue for BFS traversal + // Build Graph for each root node + // queue for BFS traversal LinkedList eventIdQueue = new LinkedList<>(); eventIdQueue.add(rootEventId); while (!eventIdQueue.isEmpty()) { @@ -155,7 +153,7 @@ public StructuredTrace buildStructuredTrace() { orderedEventNodes.add(eventId); eventIdMapping.put(eventId, eventIndex++); - //assign entity id's as well if they are not assigned + // assign entity id's as well if they are not assigned for (String entityId : eventMap.get(eventId).getEntityIdList()) { if (!entityIdMapping.containsKey(entityId)) { entityIdMapping.put(entityId, entityIndex++); @@ -164,10 +162,9 @@ public StructuredTrace buildStructuredTrace() { } List childEventIdList = eventEventConnectionMap.get(eventId); if (childEventIdList != null && childEventIdList.size() > 0) { - //sort the childEventId by their start time - childEventIdList - .sort( - Comparator.comparingLong(e -> eventMap.get(e).getStartTimeMillis())); + // sort the childEventId by their start time + childEventIdList.sort( + Comparator.comparingLong(e -> eventMap.get(e).getStartTimeMillis())); eventIdQueue.addAll(childEventIdList); } } @@ -175,14 +172,16 @@ public StructuredTrace buildStructuredTrace() { StructuredTrace structuredTrace = build(); long execEndTime = System.currentTimeMillis(); if (LOGGER.isDebugEnabled()) { - LOGGER.debug("Generated structuredTrace from events list in {} ms. Output = {}", - (execEndTime - execStartTime), structuredTrace); + LOGGER.debug( + "Generated structuredTrace from events list in {} ms. Output = {}", + (execEndTime - execStartTime), + structuredTrace); } return structuredTrace; } private StructuredTrace build() { - //start building the Structured Trace Proto object + // start building the Structured Trace Proto object Builder builder = StructuredTrace.newBuilder(); builder.setCustomerId(customerId); builder.setTraceId(traceId); @@ -193,18 +192,18 @@ private StructuredTrace build() { builder.setEntityEventEdgeList(new ArrayList<>()); builder.setResourceList(this.resourceList); - //Node Builders - //Initialize EVENT NODE + // Node Builders + // Initialize EVENT NODE for (ByteBuffer eventId : orderedEventNodes) { builder.getEventList().add(eventMap.get(eventId)); } - //Initialize ENTITY NODE builders + // Initialize ENTITY NODE builders for (String entityId : orderedEntityNodes) { builder.getEntityList().add(entityMap.get(entityId)); } - //Build Event Edge + // Build Event Edge for (Entry> entry : eventEventConnectionMap.entrySet()) { ByteBuffer parentEventId = entry.getKey(); for (ByteBuffer childEventId : entry.getValue()) { @@ -213,7 +212,7 @@ private StructuredTrace build() { } } - //Build Entity-Entity Edge + // Build Entity-Entity Edge for (Entry> entry : entityEntityConnectionMap.entrySet()) { String parentEntityId = entry.getKey(); for (String childEntityId : entry.getValue()) { @@ -221,7 +220,7 @@ private StructuredTrace build() { builder.getEntityEdgeList().add(edge); } } - //Entity Event Edge + // Entity Event Edge for (Entry> entry : entityEventConnectionMap.entrySet()) { String entityId = entry.getKey(); for (ByteBuffer eventId : entry.getValue()) { @@ -230,20 +229,19 @@ private StructuredTrace build() { } } - //Trace attributes + // Trace attributes HashMap attributeMap = new HashMap<>(); if (isPartialTrace) { - attributeMap.put("isPartial", - AttributeValueCreator.create(isPartialTrace)); - attributeMap - .put("missingEventIds", AttributeValueCreator.createFromByteBuffers(missingEventIdSet)); + attributeMap.put("isPartial", AttributeValueCreator.create(isPartialTrace)); + attributeMap.put( + "missingEventIds", AttributeValueCreator.createFromByteBuffers(missingEventIdSet)); } - //Trace Metrics + // Trace Metrics HashMap metricMap = new HashMap<>(); long durationInMillis = traceEndTime - traceStartTime; - //todo: define constants for these things. - //todo: move these things to Annotation Framework + // todo: define constants for these things. + // todo: move these things to Annotation Framework metricMap.put("Duration", MetricValueCreator.create(durationInMillis)); metricMap.put("CallCount", MetricValueCreator.create(1)); @@ -263,18 +261,16 @@ private Edge buildEntityEdge(String parentEntityId, String childEntityId) { edgeBuilder.setTgtIndex(entityIdMapping.get(childEntityId)); Entity parentEntity = entityMap.get(parentEntityId); Entity childEntity = entityMap.get(childEntityId); - //todo: define constants for these things. - //todo: move these things to Annotation Framework + // todo: define constants for these things. + // todo: move these things to Annotation Framework HashMap Attributes = new HashMap<>(); - Attributes - .put("CallerEntityType", AttributeValueCreator.create(parentEntity.getEntityType())); - Attributes - .put("CallerEntityName", AttributeValueCreator.create(parentEntity.getEntityName())); + Attributes.put("CallerEntityType", AttributeValueCreator.create(parentEntity.getEntityType())); + Attributes.put("CallerEntityName", AttributeValueCreator.create(parentEntity.getEntityName())); Attributes.put("CalleeEntityType", AttributeValueCreator.create(childEntity.getEntityType())); Attributes.put("CalleeEntityName", AttributeValueCreator.create(childEntity.getEntityName())); edgeBuilder.setAttributes(new Attributes(Attributes)); - //metrics + // metrics edgeBuilder.setMetrics(new Metrics(new HashMap<>())); return edgeBuilder.build(); } @@ -300,7 +296,7 @@ private Edge buildEventEdge(ByteBuffer parentEventId, ByteBuffer childEventId) { edgeBuilder.setStartTimeMillis(parentEvent.getStartTimeMillis()); edgeBuilder.setEndTimeMillis(parentEvent.getEndTimeMillis()); - //edge attributes + // edge attributes HashMap attributeMap = new HashMap<>(); List callerEntityNames = new ArrayList<>(); @@ -313,19 +309,18 @@ private Edge buildEventEdge(ByteBuffer parentEventId, ByteBuffer childEventId) { calleeEntityNames.add(entityMap.get(entityId).getEntityName()); } - //todo: define constants for these things. - //todo: move these things to Annotation Framework + // todo: define constants for these things. + // todo: move these things to Annotation Framework attributeMap.put("CallerEntityNames", AttributeValueCreator.create(callerEntityNames)); attributeMap.put("CalleeEntityNames", AttributeValueCreator.create(calleeEntityNames)); edgeBuilder.setAttributes(new Attributes(attributeMap)); - //edge metrics, only duration and call count for now + // edge metrics, only duration and call count for now HashMap metricMap = new HashMap<>(); - long durationInMillis = - parentEvent.getEndTimeMillis() - parentEvent.getStartTimeMillis(); - //todo: define constants for these things. - //todo: move these things to Annotation Framework + long durationInMillis = parentEvent.getEndTimeMillis() - parentEvent.getStartTimeMillis(); + // todo: define constants for these things. + // todo: move these things to Annotation Framework metricMap.put("Duration", MetricValueCreator.create(durationInMillis)); metricMap.put("CallCount", MetricValueCreator.create(1)); edgeBuilder.setMetrics(new Metrics(metricMap)); @@ -340,14 +335,13 @@ private void processEventRefList(ByteBuffer traceId, Event event) { } for (EventRef eventRef : eventRefList) { if (!eventRef.getTraceId().equals(traceId)) { - //either this is an incomplete trace - //or this referenced event belongs to another trace as of now + // either this is an incomplete trace + // or this referenced event belongs to another trace as of now if (LOGGER.isDebugEnabled()) { LOGGER.debug( "Skipping referenced event since it belongs to another trace. EventRef.TraceId = {} event.TraceId={}", HexUtils.getHex(eventRef.getTraceId()), - HexUtils.getHex(traceId) - ); + HexUtils.getHex(traceId)); } continue; @@ -357,35 +351,35 @@ private void processEventRefList(ByteBuffer traceId, Event event) { LOGGER.debug( "Referenced eventId:{} is not part of the Trace {}. May be partial trace.", HexUtils.getHex(eventRef.getEventId()), - HexUtils.getHex(eventRef.getTraceId()) - ); + HexUtils.getHex(eventRef.getTraceId())); } isPartialTrace = true; missingEventIdSet.add(eventRef.getEventId()); - //TODO: consider creating an empty event node? + // TODO: consider creating an empty event node? continue; } - /* - * For Follow from relationship: - * As, both the construct `child_of` and `follow_from` represent parent-child relation in common - * where in one case parent is interested in child span's result while in other case not. - * - * So, to support common behaviour, we will be establish link for `follow_from` as well. - * commenting out this part. - * - * if (!eventRef.getRefType().equals(EventRefType.CHILD_OF)) { - * - * continue; - * } - * Ref: https://github.com/hypertrace/hypertrace/issues/234. - Note: Ideally, an event should have a single parent. It would be either via child_of or using follow_from. - */ + /* + * For Follow from relationship: + * As, both the construct `child_of` and `follow_from` represent parent-child relation in common + * where in one case parent is interested in child span's result while in other case not. + * + * So, to support common behaviour, we will be establish link for `follow_from` as well. + * commenting out this part. + * + * if (!eventRef.getRefType().equals(EventRefType.CHILD_OF)) { + * + * continue; + * } + * Ref: https://github.com/hypertrace/hypertrace/issues/234. + Note: Ideally, an event should have a single parent. It would be either via child_of or using follow_from. + */ Event parentEvent = eventMap.get(eventRef.getEventId()); // Add Event to Event Edge. - eventEventConnectionMap.computeIfAbsent(parentEvent.getEventId(), e -> new ArrayList<>()) + eventEventConnectionMap + .computeIfAbsent(parentEvent.getEventId(), e -> new ArrayList<>()) .add(event.getEventId()); // Create edge between all entity types @@ -400,12 +394,14 @@ private void processEventRefList(ByteBuffer traceId, Event event) { // parent and child span belong to same entity. This should be solved later. if (parentEventEntityType != null && !parentEventEntityId.equals(childEventEntityId)) { // ADD ENTITY TO ENTITY EDGE - entityEntityConnectionMap.computeIfAbsent(parentEventEntityId, e -> new ArrayList<>()) + entityEntityConnectionMap + .computeIfAbsent(parentEventEntityId, e -> new ArrayList<>()) .add(childEventEntityId); } } } else { - LOGGER.warn("Unable to find the parent entity from ids in the event. " + LOGGER.warn( + "Unable to find the parent entity from ids in the event. " + "EventId: {}, EntityId: {}", HexUtils.getHex(parentEvent.getEventId()), parentEventEntityId); @@ -421,16 +417,13 @@ private void processEventRefList(ByteBuffer traceId, Event event) { * * @return StructuredTrace */ - public static StructuredTrace buildStructuredTraceFromRawSpans(List rawSpanList, - ByteBuffer traceId, - String customerId) { + public static StructuredTrace buildStructuredTraceFromRawSpans( + List rawSpanList, ByteBuffer traceId, String customerId) { return buildStructuredTraceFromRawSpans(rawSpanList, traceId, customerId, null); } - public static StructuredTrace buildStructuredTraceFromRawSpans(List rawSpanList, - ByteBuffer traceId, - String customerId, - Timestamps timestamps) { + public static StructuredTrace buildStructuredTraceFromRawSpans( + List rawSpanList, ByteBuffer traceId, String customerId, Timestamps timestamps) { Map entityMap = new HashMap<>(); // Relying on insertion ordered keyset LinkedHashMap resourceIndexMap = new LinkedHashMap<>(); @@ -458,13 +451,14 @@ public static StructuredTrace buildStructuredTraceFromRawSpans(List raw } } - StructuredTraceBuilder structuredTraceBuilder = new StructuredTraceBuilder( - eventList, - entityMap, - customerId, - traceId, - timestamps, - List.copyOf(resourceIndexMap.keySet())); + StructuredTraceBuilder structuredTraceBuilder = + new StructuredTraceBuilder( + eventList, + entityMap, + customerId, + traceId, + timestamps, + List.copyOf(resourceIndexMap.keySet())); return structuredTraceBuilder.buildStructuredTrace(); } @@ -476,8 +470,7 @@ public static StructuredTrace buildStructuredTraceFromRawSpans(List raw private static void mergeAttributes(Entity existingEntity, Entity newEntity) { Attributes mergedAttributes = new Attributes(new HashMap<>()); if (existingEntity.getAttributes() != null) { - mergedAttributes.getAttributeMap() - .putAll(existingEntity.getAttributes().getAttributeMap()); + mergedAttributes.getAttributeMap().putAll(existingEntity.getAttributes().getAttributeMap()); } if (newEntity.getAttributes() != null) { mergedAttributes.getAttributeMap().putAll(newEntity.getAttributes().getAttributeMap()); @@ -485,4 +478,3 @@ private static void mergeAttributes(Entity existingEntity, Entity newEntity) { existingEntity.setAttributes(mergedAttributes); } } - diff --git a/data-model/src/test/java/org/hypertrace/core/datamodel/shared/DataflowMetricUtilsTest.java b/data-model/src/test/java/org/hypertrace/core/datamodel/shared/DataflowMetricUtilsTest.java index 0896828..011fd24 100644 --- a/data-model/src/test/java/org/hypertrace/core/datamodel/shared/DataflowMetricUtilsTest.java +++ b/data-model/src/test/java/org/hypertrace/core/datamodel/shared/DataflowMetricUtilsTest.java @@ -1,15 +1,5 @@ package org.hypertrace.core.datamodel.shared; -import io.micrometer.core.instrument.Timer; -import org.hypertrace.core.datamodel.StructuredTrace; -import org.hypertrace.core.datamodel.TimestampRecord; -import org.hypertrace.core.datamodel.Timestamps; -import org.junit.jupiter.api.Test; - -import java.util.HashMap; -import java.util.Map; -import java.util.concurrent.TimeUnit; - import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; @@ -18,6 +8,15 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import io.micrometer.core.instrument.Timer; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import org.hypertrace.core.datamodel.StructuredTrace; +import org.hypertrace.core.datamodel.TimestampRecord; +import org.hypertrace.core.datamodel.Timestamps; +import org.junit.jupiter.api.Test; + class DataflowMetricUtilsTest { private static final String SPAN_ARRIVAL_TIME = "span.arrival.time"; @@ -46,4 +45,4 @@ public void test_insertTimestamp() { DataflowMetricUtils.insertTimestamp(trace, "test.metric"); assertTrue(trace.getTimestamps().getRecords().containsKey("test.metric")); } -} \ No newline at end of file +} diff --git a/data-model/src/test/java/org/hypertrace/core/datamodel/shared/StructuredTraceGraphTest.java b/data-model/src/test/java/org/hypertrace/core/datamodel/shared/StructuredTraceGraphTest.java index 363fb92..b5a5f08 100644 --- a/data-model/src/test/java/org/hypertrace/core/datamodel/shared/StructuredTraceGraphTest.java +++ b/data-model/src/test/java/org/hypertrace/core/datamodel/shared/StructuredTraceGraphTest.java @@ -18,7 +18,6 @@ import java.util.Map; import java.util.Set; import java.util.UUID; - import org.hypertrace.core.datamodel.Attributes; import org.hypertrace.core.datamodel.Edge; import org.hypertrace.core.datamodel.Entity; @@ -35,13 +34,11 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; - class StructuredTraceGraphTest { private static final String CUSTOMER_ID = "customer_id"; - @Mock - private StructuredTrace trace; + @Mock private StructuredTrace trace; private List events; private List entities; private List eventEdges; @@ -92,7 +89,8 @@ private void createGraph_shouldCreateCorrectGraph() { assertEquals(expectedRootEntities, graph.getRootEntities()); assertEquals(expectedRootEvents, graph.getRootEvents()); assertEquals(events.get(sourceIdx1), graph.getParentEvent(events.get(targetIdx1))); - assertEquals(Lists.newArrayList(entities.get(sourceIdx1)), + assertEquals( + Lists.newArrayList(entities.get(sourceIdx1)), graph.getParentEntities(entities.get(targetIdx1))); List root1Children = graph.getChildrenEntities(entities.get(rootIndex1)); assertEquals(Lists.newArrayList(entities.get(targetIdx1)), root1Children); @@ -103,14 +101,12 @@ private void createGraph_shouldCreateCorrectGraph() { private void setupEventAndEntityMocks(int totalEvent) { for (int index = 0; index < totalEvent; index++) { Event event = mock(Event.class); - when(event.getEventId()) - .thenReturn(ByteBuffer.wrap(String.valueOf(index).getBytes())); + when(event.getEventId()).thenReturn(ByteBuffer.wrap(String.valueOf(index).getBytes())); events.add(event); expectedEventMap.put(event.getEventId(), event); Entity entity = mock(Entity.class); - when(entity.getEntityId()) - .thenReturn(String.valueOf(index)); + when(entity.getEntityId()).thenReturn(String.valueOf(index)); entities.add(entity); } when(trace.getEventList()).thenReturn(events); @@ -131,37 +127,61 @@ void testOnMockedEvents() { String entityId1 = UUID.randomUUID().toString(); Event e1 = getEvent(generateRandomId(), entityId1); Entity entity1 = getEntity(entityId1, "DOCKER_CONTAINER"); - RawSpan rawSpan1 = RawSpan.newBuilder().setCustomerId(CUSTOMER_ID).setTraceId(traceId) - .setEvent(e1).setEntityList(List.of(entity1)).build(); + RawSpan rawSpan1 = + RawSpan.newBuilder() + .setCustomerId(CUSTOMER_ID) + .setTraceId(traceId) + .setEvent(e1) + .setEntityList(List.of(entity1)) + .build(); String entityId2 = UUID.randomUUID().toString(); Event e2 = getEvent(generateRandomId(), entityId2); Entity entity2 = getEntity(entityId2, "K8S_POD"); - RawSpan rawSpan2 = RawSpan.newBuilder().setCustomerId(CUSTOMER_ID).setTraceId(traceId) - .setEvent(e2).setEntityList(List.of(entity2)).build(); + RawSpan rawSpan2 = + RawSpan.newBuilder() + .setCustomerId(CUSTOMER_ID) + .setTraceId(traceId) + .setEvent(e2) + .setEntityList(List.of(entity2)) + .build(); String entityId3 = UUID.randomUUID().toString(); Event e3 = getEvent(generateRandomId(), entityId3); Entity entity3 = getEntity(entityId3, "DAEMONSET"); - RawSpan rawSpan3 = RawSpan.newBuilder().setCustomerId(CUSTOMER_ID).setTraceId(traceId) - .setEvent(e3).setEntityList(List.of(entity3)).build(); + RawSpan rawSpan3 = + RawSpan.newBuilder() + .setCustomerId(CUSTOMER_ID) + .setTraceId(traceId) + .setEvent(e3) + .setEntityList(List.of(entity3)) + .build(); // Make e2 as child of e1. ByteBuffer eventId1 = e1.getEventId(); - when(e2.getEventRefList()).thenReturn(Collections.singletonList( - EventRef.newBuilder().setEventId(eventId1).setRefType(EventRefType.CHILD_OF) - .setTraceId(traceId).build())); - - //Making e3 as child of e2, follow_from construct + when(e2.getEventRefList()) + .thenReturn( + Collections.singletonList( + EventRef.newBuilder() + .setEventId(eventId1) + .setRefType(EventRefType.CHILD_OF) + .setTraceId(traceId) + .build())); + + // Making e3 as child of e2, follow_from construct ByteBuffer eventId2 = e2.getEventId(); - when(e3.getEventRefList()).thenReturn(Collections.singletonList( - EventRef.newBuilder().setEventId(eventId2).setRefType(EventRefType.FOLLOWS_FROM) - .setTraceId(traceId).build())); - - StructuredTrace trace = StructuredTraceBuilder - .buildStructuredTraceFromRawSpans(List.of(rawSpan1, rawSpan2,rawSpan3), - traceId, - CUSTOMER_ID); + when(e3.getEventRefList()) + .thenReturn( + Collections.singletonList( + EventRef.newBuilder() + .setEventId(eventId2) + .setRefType(EventRefType.FOLLOWS_FROM) + .setTraceId(traceId) + .build())); + + StructuredTrace trace = + StructuredTraceBuilder.buildStructuredTraceFromRawSpans( + List.of(rawSpan1, rawSpan2, rawSpan3), traceId, CUSTOMER_ID); assertEquals(traceId, trace.getTraceId()); assertEquals(CUSTOMER_ID, trace.getCustomerId()); @@ -176,10 +196,14 @@ void testOnMockedEvents() { assertEquals(3, graph.getEventMap().size()); assertEquals(1, graph.getRootEntities().size()); assertEquals(1, graph.getRootEvents().size()); - Map childIdToParentIds = graph.getChildIdsToParentIds(); - Map> parentToChildEventIds = graph.getParentToChildEventIds(); - assertTrue(childIdToParentIds.containsKey(e2.getEventId()) && childIdToParentIds.containsKey(e3.getEventId())); - assertTrue(parentToChildEventIds.containsKey(e1.getEventId()) && parentToChildEventIds.containsKey(e2.getEventId())); + Map childIdToParentIds = graph.getChildIdsToParentIds(); + Map> parentToChildEventIds = graph.getParentToChildEventIds(); + assertTrue( + childIdToParentIds.containsKey(e2.getEventId()) + && childIdToParentIds.containsKey(e3.getEventId())); + assertTrue( + parentToChildEventIds.containsKey(e1.getEventId()) + && parentToChildEventIds.containsKey(e2.getEventId())); assertTrue(graph.getParentEntities(entity2).contains(entity1)); assertTrue(graph.getParentEntities(entity3).contains(entity2)); assertEquals(e1, graph.getParentEvent(e2)); @@ -191,8 +215,11 @@ private ByteBuffer generateRandomId() { } private Entity getEntity(String entityId, String entityType) { - return Entity.newBuilder().setEntityId(entityId).setEntityType(entityType) - .setEntityName(entityId).setCustomerId(CUSTOMER_ID) + return Entity.newBuilder() + .setEntityId(entityId) + .setEntityType(entityType) + .setEntityName(entityId) + .setCustomerId(CUSTOMER_ID) .setAttributes(Attributes.newBuilder().setAttributeMap(new HashMap<>()).build()) .build(); } @@ -205,8 +232,7 @@ private Event getEvent(ByteBuffer eventId, String entityId) { .thenReturn(Attributes.newBuilder().setAttributeMap(new HashMap<>()).build()); when(e.getEnrichedAttributes()) .thenReturn(Attributes.newBuilder().setAttributeMap(new HashMap<>()).build()); - when(e.getMetrics()) - .thenReturn(Metrics.newBuilder().setMetricMap(new HashMap<>()).build()); + when(e.getMetrics()).thenReturn(Metrics.newBuilder().setMetricMap(new HashMap<>()).build()); return e; } @@ -225,25 +251,39 @@ void test_recreate_partialGraph() { String entityId1 = UUID.randomUUID().toString(); Event e1 = getEvent(generateRandomId(), entityId1); Entity entity1 = getEntity(entityId1, "DOCKER_CONTAINER"); - RawSpan rawSpan1 = RawSpan.newBuilder().setCustomerId(CUSTOMER_ID).setTraceId(traceId) - .setEvent(e1).setEntityList(List.of(entity1)).build(); + RawSpan rawSpan1 = + RawSpan.newBuilder() + .setCustomerId(CUSTOMER_ID) + .setTraceId(traceId) + .setEvent(e1) + .setEntityList(List.of(entity1)) + .build(); String entityId2 = UUID.randomUUID().toString(); Event e2 = getEvent(generateRandomId(), entityId2); Entity entity2 = getEntity(entityId2, "K8S_POD"); - RawSpan rawSpan2 = RawSpan.newBuilder().setCustomerId(CUSTOMER_ID).setTraceId(traceId) - .setEvent(e2).setEntityList(List.of(entity2)).build(); + RawSpan rawSpan2 = + RawSpan.newBuilder() + .setCustomerId(CUSTOMER_ID) + .setTraceId(traceId) + .setEvent(e2) + .setEntityList(List.of(entity2)) + .build(); // Make e2 as child of e1. ByteBuffer eventId1 = e1.getEventId(); - when(e2.getEventRefList()).thenReturn(Collections.singletonList( - EventRef.newBuilder().setEventId(eventId1).setRefType(EventRefType.CHILD_OF) - .setTraceId(traceId).build())); - - StructuredTrace trace = StructuredTraceBuilder - .buildStructuredTraceFromRawSpans(List.of(rawSpan1, rawSpan2), - traceId, - CUSTOMER_ID); + when(e2.getEventRefList()) + .thenReturn( + Collections.singletonList( + EventRef.newBuilder() + .setEventId(eventId1) + .setRefType(EventRefType.CHILD_OF) + .setTraceId(traceId) + .build())); + + StructuredTrace trace = + StructuredTraceBuilder.buildStructuredTraceFromRawSpans( + List.of(rawSpan1, rawSpan2), traceId, CUSTOMER_ID); assertEquals(traceId, trace.getTraceId()); assertEquals(CUSTOMER_ID, trace.getCustomerId()); @@ -267,17 +307,26 @@ void test_recreate_partialGraph() { String entityId3 = UUID.randomUUID().toString(); Event e3 = getEvent(generateRandomId(), entityId3); Entity entity3 = getEntity(entityId3, "K8S_POD"); - RawSpan rawSpan3 = RawSpan.newBuilder().setCustomerId(CUSTOMER_ID).setTraceId(traceId) - .setEvent(e3).setEntityList(List.of(entity3)).build(); + RawSpan rawSpan3 = + RawSpan.newBuilder() + .setCustomerId(CUSTOMER_ID) + .setTraceId(traceId) + .setEvent(e3) + .setEntityList(List.of(entity3)) + .build(); // e3 child of e1 - when(e3.getEventRefList()).thenReturn(Collections.singletonList( - EventRef.newBuilder().setEventId(eventId1).setRefType(EventRefType.CHILD_OF) - .setTraceId(traceId).build())); - - trace = StructuredTraceBuilder - .buildStructuredTraceFromRawSpans(List.of(rawSpan1, rawSpan2, rawSpan3), - traceId, - CUSTOMER_ID); + when(e3.getEventRefList()) + .thenReturn( + Collections.singletonList( + EventRef.newBuilder() + .setEventId(eventId1) + .setRefType(EventRefType.CHILD_OF) + .setTraceId(traceId) + .build())); + + trace = + StructuredTraceBuilder.buildStructuredTraceFromRawSpans( + List.of(rawSpan1, rawSpan2, rawSpan3), traceId, CUSTOMER_ID); TraceEventsGraph traceEventsGraph = graph.getTraceEventsGraph(); TraceEntitiesGraph traceEntitiesGraph = graph.getTraceEntitiesGraph(); From 019a6924a447dba9cffeef19cfb2976b8eed8eca Mon Sep 17 00:00:00 2001 From: Laxman Ch Date: Wed, 27 Oct 2021 13:18:35 +0530 Subject: [PATCH 2/3] avro builder reflection cache utility added --- .../datamodel/shared/AvroBuilderCache.java | 70 +++++++++++++++++++ .../shared/AvroBuilderCacheTest.java | 16 +++++ 2 files changed, 86 insertions(+) create mode 100644 data-model/src/main/java/org/hypertrace/core/datamodel/shared/AvroBuilderCache.java create mode 100644 data-model/src/test/java/org/hypertrace/core/datamodel/shared/AvroBuilderCacheTest.java diff --git a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/AvroBuilderCache.java b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/AvroBuilderCache.java new file mode 100644 index 0000000..bf34948 --- /dev/null +++ b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/AvroBuilderCache.java @@ -0,0 +1,70 @@ +package org.hypertrace.core.datamodel.shared; + +import java.lang.reflect.Constructor; +import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import javax.annotation.Nonnull; +import org.apache.avro.specific.SpecificRecord; +import org.apache.avro.specific.SpecificRecordBuilderBase; +import org.apache.commons.lang3.tuple.Pair; + +/** + * Utility class to instantiate a new avro builder in a fast and efficient way. Default Avro + * implementation has a perf issue due to uncached reflection based implementation. For more + * details, https://issues.apache.org/jira/browse/AVRO-3048 + * + *

This avro perf bug is fixed in unreleased avro version (0.11.x). We can get rid of this + * utility class once we migrate to Avro 0.11.x or above + */ +@SuppressWarnings({"rawtypes", "unchecked"}) +public class AvroBuilderCache { + private static final Map< + Class, Pair> + AVRO_BUILDER_CACHE = new ConcurrentHashMap<>(); + + public static T fastNewAvroBuilder( + Class specificAvroBuilderClass) { + try { + Pair pair = + AVRO_BUILDER_CACHE.computeIfAbsent( + (Class) specificAvroBuilderClass, + AvroBuilderCache::newBuilderFor); + return (T) pair.getLeft().invoke(null, pair.getRight()); + } catch (Exception e) { + throw new RuntimeException( + "Unable to instantiate new builder for: " + specificAvroBuilderClass.getName(), e); + } + } + + private static Pair newBuilderFor( + @Nonnull Class specificAvroBuilderClass) { + try { + Class specificAvroClass = + (Class) specificAvroBuilderClass.getEnclosingClass(); + Method newBuilderMethod = findBuilderMethod(specificAvroClass); + + final Constructor declaredConstructor = specificAvroBuilderClass.getDeclaredConstructor(); + declaredConstructor.setAccessible(true); + SpecificRecordBuilderBase builderModel = + (SpecificRecordBuilderBase) declaredConstructor.newInstance(); + + return Pair.of(newBuilderMethod, builderModel); + } catch (Exception e) { + throw new RuntimeException( + "Unable to instantiate the model builder: " + specificAvroBuilderClass.getName(), e); + } + } + + private static Method findBuilderMethod(Class specificAvroClass) { + return Arrays.stream(specificAvroClass.getDeclaredMethods()) + .filter(method -> method.getName().equals("newBuilder")) + .filter(method -> method.getParameterTypes().length == 1) + .filter( + method -> + SpecificRecordBuilderBase.class.isAssignableFrom(method.getParameterTypes()[0])) + .findFirst() + .orElseThrow(IllegalArgumentException::new); + } +} diff --git a/data-model/src/test/java/org/hypertrace/core/datamodel/shared/AvroBuilderCacheTest.java b/data-model/src/test/java/org/hypertrace/core/datamodel/shared/AvroBuilderCacheTest.java new file mode 100644 index 0000000..4bcc5ff --- /dev/null +++ b/data-model/src/test/java/org/hypertrace/core/datamodel/shared/AvroBuilderCacheTest.java @@ -0,0 +1,16 @@ +package org.hypertrace.core.datamodel.shared; + +import org.hypertrace.core.datamodel.Metrics; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class AvroBuilderCacheTest { + + @Test + public void testGetBuilderFromCache() { + Metrics.Builder builder1 = AvroBuilderCache.fastNewAvroBuilder(Metrics.Builder.class); + Metrics.Builder builder2 = AvroBuilderCache.fastNewAvroBuilder(Metrics.Builder.class); + Assertions.assertNotSame( + builder1, builder2, "Builders can't be same. Every builder has to be a new instance"); + } +} From e8bac6aaa308c349d5eda058896d02ba538fcce5 Mon Sep 17 00:00:00 2001 From: Laxman Ch Date: Wed, 27 Oct 2021 13:43:53 +0530 Subject: [PATCH 3/3] avro builder reflection cache utility used in util class --- .../core/datamodel/shared/AvroBuilderCache.java | 2 +- .../core/datamodel/shared/SpanAttributeUtils.java | 4 +++- .../shared/trace/AttributeValueCreator.java | 12 +++++++----- .../shared/trace/MetricValueCreator.java | 8 +++++--- .../shared/trace/StructuredTraceBuilder.java | 15 +++++++++------ .../datamodel/shared/AvroBuilderCacheTest.java | 4 ++-- 6 files changed, 27 insertions(+), 18 deletions(-) diff --git a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/AvroBuilderCache.java b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/AvroBuilderCache.java index bf34948..cbf8299 100644 --- a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/AvroBuilderCache.java +++ b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/AvroBuilderCache.java @@ -24,7 +24,7 @@ public class AvroBuilderCache { Class, Pair> AVRO_BUILDER_CACHE = new ConcurrentHashMap<>(); - public static T fastNewAvroBuilder( + public static T fastNewBuilder( Class specificAvroBuilderClass) { try { Pair pair = diff --git a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/SpanAttributeUtils.java b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/SpanAttributeUtils.java index 0579468..bff8b36 100644 --- a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/SpanAttributeUtils.java +++ b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/SpanAttributeUtils.java @@ -1,5 +1,7 @@ package org.hypertrace.core.datamodel.shared; +import static org.hypertrace.core.datamodel.shared.AvroBuilderCache.fastNewBuilder; + import java.nio.ByteBuffer; import java.util.List; import java.util.Map; @@ -34,7 +36,7 @@ public static AttributeValue getAttributeValueWithDefault( Event event, String attributeKey, String defaultValue) { AttributeValue attributeValue = getAttributeValue(event, attributeKey); return attributeValue == null - ? AttributeValue.newBuilder().setValue(defaultValue).build() + ? fastNewBuilder(AttributeValue.Builder.class).setValue(defaultValue).build() : attributeValue; } diff --git a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/AttributeValueCreator.java b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/AttributeValueCreator.java index 9937ded..90004b7 100644 --- a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/AttributeValueCreator.java +++ b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/AttributeValueCreator.java @@ -1,5 +1,7 @@ package org.hypertrace.core.datamodel.shared.trace; +import static org.hypertrace.core.datamodel.shared.AvroBuilderCache.fastNewBuilder; + import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.List; @@ -10,19 +12,19 @@ public class AttributeValueCreator { public static AttributeValue create(String value) { - return AttributeValue.newBuilder().setValue(value).build(); + return fastNewBuilder(AttributeValue.Builder.class).setValue(value).build(); } public static AttributeValue create(boolean value) { - return AttributeValue.newBuilder().setValue(String.valueOf(value)).build(); + return fastNewBuilder(AttributeValue.Builder.class).setValue(String.valueOf(value)).build(); } public static AttributeValue create(int value) { - return AttributeValue.newBuilder().setValue(String.valueOf(value)).build(); + return fastNewBuilder(AttributeValue.Builder.class).setValue(String.valueOf(value)).build(); } public static AttributeValue create(List values) { - return AttributeValue.newBuilder().setValueList(values).build(); + return fastNewBuilder(AttributeValue.Builder.class).setValueList(values).build(); } public static AttributeValue createFromByteBuffers(Set values) { @@ -31,6 +33,6 @@ public static AttributeValue createFromByteBuffers(Set values) { value -> { list.add(new String(HexUtils.getBytes(value))); }); - return AttributeValue.newBuilder().setValueList(list).build(); + return fastNewBuilder(AttributeValue.Builder.class).setValueList(list).build(); } } diff --git a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/MetricValueCreator.java b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/MetricValueCreator.java index 4b4c6f8..6751dd2 100644 --- a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/MetricValueCreator.java +++ b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/MetricValueCreator.java @@ -1,18 +1,20 @@ package org.hypertrace.core.datamodel.shared.trace; +import static org.hypertrace.core.datamodel.shared.AvroBuilderCache.fastNewBuilder; + import org.hypertrace.core.datamodel.MetricValue; public class MetricValueCreator { public static MetricValue create(double value) { - return MetricValue.newBuilder().setValue(value).build(); + return fastNewBuilder(MetricValue.Builder.class).setValue(value).build(); } public static MetricValue create(long value) { - return MetricValue.newBuilder().setValue((double) value).build(); + return fastNewBuilder(MetricValue.Builder.class).setValue((double) value).build(); } public static MetricValue create(int value) { - return MetricValue.newBuilder().setValue((double) value).build(); + return fastNewBuilder(MetricValue.Builder.class).setValue((double) value).build(); } } diff --git a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/StructuredTraceBuilder.java b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/StructuredTraceBuilder.java index 042fb4c..252184b 100644 --- a/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/StructuredTraceBuilder.java +++ b/data-model/src/main/java/org/hypertrace/core/datamodel/shared/trace/StructuredTraceBuilder.java @@ -1,6 +1,7 @@ package org.hypertrace.core.datamodel.shared.trace; import static java.util.Objects.nonNull; +import static org.hypertrace.core.datamodel.shared.AvroBuilderCache.fastNewBuilder; import com.google.common.collect.BiMap; import com.google.common.collect.HashBiMap; @@ -182,7 +183,7 @@ public StructuredTrace buildStructuredTrace() { private StructuredTrace build() { // start building the Structured Trace Proto object - Builder builder = StructuredTrace.newBuilder(); + Builder builder = fastNewBuilder(StructuredTrace.Builder.class); builder.setCustomerId(customerId); builder.setTraceId(traceId); builder.setEventList(new ArrayList<>()); @@ -255,7 +256,7 @@ private StructuredTrace build() { } private Edge buildEntityEdge(String parentEntityId, String childEntityId) { - Edge.Builder edgeBuilder = Edge.newBuilder(); + Edge.Builder edgeBuilder = fastNewBuilder(Edge.Builder.class); edgeBuilder.setEdgeType(EdgeType.ENTITY_ENTITY); edgeBuilder.setSrcIndex(entityIdMapping.get(parentEntityId)); edgeBuilder.setTgtIndex(entityIdMapping.get(childEntityId)); @@ -276,17 +277,19 @@ private Edge buildEntityEdge(String parentEntityId, String childEntityId) { } private Edge buildEntityEventEdge(String entityId, ByteBuffer eventId) { - Edge.Builder edgeBuilder = Edge.newBuilder(); + Edge.Builder edgeBuilder = fastNewBuilder(Edge.Builder.class); edgeBuilder.setEdgeType(EdgeType.ENTITY_EVENT); edgeBuilder.setSrcIndex(entityIdMapping.get(entityId)); edgeBuilder.setTgtIndex(eventIdMapping.get(eventId)); - edgeBuilder.setAttributes(Attributes.newBuilder().setAttributeMap(new HashMap<>()).build()); - edgeBuilder.setMetrics(Metrics.newBuilder().setMetricMap(new HashMap<>()).build()); + edgeBuilder.setAttributes( + fastNewBuilder(Attributes.Builder.class).setAttributeMap(new HashMap<>()).build()); + edgeBuilder.setMetrics( + fastNewBuilder(Metrics.Builder.class).setMetricMap(new HashMap<>()).build()); return edgeBuilder.build(); } private Edge buildEventEdge(ByteBuffer parentEventId, ByteBuffer childEventId) { - Edge.Builder edgeBuilder = Edge.newBuilder(); + Edge.Builder edgeBuilder = fastNewBuilder(Edge.Builder.class); edgeBuilder.setSrcIndex(eventIdMapping.get(parentEventId)); edgeBuilder.setTgtIndex(eventIdMapping.get(childEventId)); edgeBuilder.setEdgeType(EdgeType.EVENT_EVENT); diff --git a/data-model/src/test/java/org/hypertrace/core/datamodel/shared/AvroBuilderCacheTest.java b/data-model/src/test/java/org/hypertrace/core/datamodel/shared/AvroBuilderCacheTest.java index 4bcc5ff..c7b156b 100644 --- a/data-model/src/test/java/org/hypertrace/core/datamodel/shared/AvroBuilderCacheTest.java +++ b/data-model/src/test/java/org/hypertrace/core/datamodel/shared/AvroBuilderCacheTest.java @@ -8,8 +8,8 @@ public class AvroBuilderCacheTest { @Test public void testGetBuilderFromCache() { - Metrics.Builder builder1 = AvroBuilderCache.fastNewAvroBuilder(Metrics.Builder.class); - Metrics.Builder builder2 = AvroBuilderCache.fastNewAvroBuilder(Metrics.Builder.class); + Metrics.Builder builder1 = AvroBuilderCache.fastNewBuilder(Metrics.Builder.class); + Metrics.Builder builder2 = AvroBuilderCache.fastNewBuilder(Metrics.Builder.class); Assertions.assertNotSame( builder1, builder2, "Builders can't be same. Every builder has to be a new instance"); }