From a64e43d03d0c033374e761433a52fe8434f26d7b Mon Sep 17 00:00:00 2001 From: Rishabh Singh Date: Mon, 11 Jan 2021 23:09:55 +0530 Subject: [PATCH 1/6] Share state between view generators --- .../trace/util/ApiTraceGraph.java | 12 ++- .../generators/BaseViewGenerator.java | 43 +++-------- .../generators/RawServiceViewGenerator.java | 3 +- .../generators/ServiceCallViewGenerator.java | 2 +- .../generators/ViewGeneratorState.java | 77 +++++++++++++++++++ 5 files changed, 97 insertions(+), 40 deletions(-) create mode 100644 hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/ViewGeneratorState.java diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/ApiTraceGraph.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/ApiTraceGraph.java index 69027113c..83e2b96f2 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/ApiTraceGraph.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/ApiTraceGraph.java @@ -32,10 +32,10 @@ public class ApiTraceGraph { private static final String UNKNOWN_SPAN_KIND_VALUE = EnrichedSpanConstants.getValue(AttributeValue.ATTRIBUTE_VALUE_UNKNOWN); - private List> nodeList; - private List apiNodeEventEdgeList; - private StructuredTrace trace; - private Map eventIdToIndexInTrace; + private final List> nodeList; + private final List apiNodeEventEdgeList; + private final StructuredTrace trace; + private final Map eventIdToIndexInTrace; public ApiTraceGraph(StructuredTrace trace) { this.trace = trace; @@ -44,6 +44,10 @@ public ApiTraceGraph(StructuredTrace trace) { eventIdToIndexInTrace = buildEventIdToIndexInTrace(trace); } + public StructuredTrace getTrace() { + return trace; + } + public List> getNodeList() { return nodeList; } diff --git a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/BaseViewGenerator.java b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/BaseViewGenerator.java index 1816e8051..b7d16d305 100644 --- a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/BaseViewGenerator.java +++ b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/BaseViewGenerator.java @@ -23,6 +23,7 @@ import org.hypertrace.core.viewgenerator.JavaCodeBasedViewGenerator; import org.hypertrace.traceenricher.enrichedspan.constants.EnrichedSpanConstants; import org.hypertrace.traceenricher.enrichedspan.constants.v1.CommonAttribute; +import org.hypertrace.viewgenerator.generators.ViewGeneratorState.TraceState; /** * Pre-processing for View Generators, for data that are mostly needed by the the view generators @@ -66,43 +67,17 @@ static String getTransactionName(StructuredTrace trace) { public List process(StructuredTrace trace) { DataflowMetricUtils.reportArrivalLagAndInsertTimestamp(trace, viewGeneratorArrivalTimer, VIEW_GENERATION_ARRIVAL_TIME); - Map entityMap = new HashMap<>(); - Map eventMap = new HashMap<>(); - Map> parentToChildrenEventIds = new HashMap<>(); - Map childToParentEventIds = new HashMap<>(); - - for (Entity entity : trace.getEntityList()) { - entityMap.put(entity.getEntityId(), entity); - } - for (Event event : trace.getEventList()) { - eventMap.put(event.getEventId(), event); - } - - for (Event event : trace.getEventList()) { - ByteBuffer childEventId = event.getEventId(); - List eventRefs = eventMap.get(childEventId).getEventRefList(); - if (eventRefs != null) { - eventRefs.stream() - .filter(eventRef -> EventRefType.CHILD_OF == eventRef.getRefType()) - .forEach( - eventRef -> { - ByteBuffer parentEventId = eventRef.getEventId(); - if (!parentToChildrenEventIds.containsKey(parentEventId)) { - parentToChildrenEventIds.put(parentEventId, new ArrayList<>()); - } - parentToChildrenEventIds.get(parentEventId).add(childEventId); - childToParentEventIds.put(childEventId, parentEventId); - }); - } - // expected only 1 childOf relationship - } + TraceState traceState = ViewGeneratorState.getTraceState(trace); + Map entityMap = traceState.entityMap; + Map eventMap = traceState.eventMap; + Map> parentToChildrenEventIds = traceState.parentToChildrenEventIds; + Map childToParentEventIds = traceState.childToParentEventIds; return generateView( trace, - Collections.unmodifiableMap(entityMap), - Collections.unmodifiableMap(eventMap), - Collections.unmodifiableMap(parentToChildrenEventIds), - Collections.unmodifiableMap(childToParentEventIds)); + entityMap, eventMap, + parentToChildrenEventIds, + childToParentEventIds); } abstract List generateView( diff --git a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/RawServiceViewGenerator.java b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/RawServiceViewGenerator.java index 9e6e97aca..4adcf2b82 100644 --- a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/RawServiceViewGenerator.java +++ b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/RawServiceViewGenerator.java @@ -29,7 +29,8 @@ List generateView( List list = new ArrayList<>(); // Construct ApiTraceGraph and look at all the head spans within each ApiNode - ApiTraceGraph apiTraceGraph = new ApiTraceGraph(structuredTrace); + ApiTraceGraph apiTraceGraph = ViewGeneratorState.getApiTraceGraph(structuredTrace); + apiTraceGraph = apiTraceGraph.build(); List> apiNodes = apiTraceGraph.getNodeList(); for (ApiNode apiNode : apiNodes) { diff --git a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/ServiceCallViewGenerator.java b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/ServiceCallViewGenerator.java index ee609d755..48d77ba99 100644 --- a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/ServiceCallViewGenerator.java +++ b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/ServiceCallViewGenerator.java @@ -63,7 +63,7 @@ List generateView( Map eventMap, Map> parentToChildrenEventIds, Map childToParentEventIds) { - ApiTraceGraph apiTraceGraph = new ApiTraceGraph(structuredTrace).build(); + ApiTraceGraph apiTraceGraph = ViewGeneratorState.getApiTraceGraph(structuredTrace); // Scenario #1: Go through the apiNode edges and create a record for each edge. Should be easy // to get to the events diff --git a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/ViewGeneratorState.java b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/ViewGeneratorState.java new file mode 100644 index 000000000..93b0f1c5f --- /dev/null +++ b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/ViewGeneratorState.java @@ -0,0 +1,77 @@ +package org.hypertrace.viewgenerator.generators; + +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +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.StructuredTrace; +import org.hypertrace.traceenricher.trace.util.ApiTraceGraph; + +public class ViewGeneratorState { + + private static final ThreadLocal traceStateThreadLocal = new ThreadLocal<>(); + private static final ThreadLocal apiTraceGraphThreadLocal = new ThreadLocal<>(); + + public static ApiTraceGraph getApiTraceGraph(StructuredTrace trace) { + if (apiTraceGraphThreadLocal.get() == null + || isDifferentTrace(apiTraceGraphThreadLocal.get().getTrace(), trace)) { + apiTraceGraphThreadLocal.set(new ApiTraceGraph(trace).build()); + } + return apiTraceGraphThreadLocal.get(); + } + + public static TraceState getTraceState(StructuredTrace trace) { + if (traceStateThreadLocal.get() == null + || isDifferentTrace(traceStateThreadLocal.get().trace, trace)) { + traceStateThreadLocal.set(new TraceState(trace)); + } + return traceStateThreadLocal.get(); + } + + private static boolean isDifferentTrace(StructuredTrace cached, StructuredTrace trace) { + return !cached.getCustomerId().equals(trace.getCustomerId()) + || !cached.getTraceId().equals(trace.getTraceId()); + } + + public static class TraceState { + private final StructuredTrace trace; + final Map entityMap = new HashMap<>(); + final Map eventMap = new HashMap<>(); + final Map> parentToChildrenEventIds = new HashMap<>(); + final Map childToParentEventIds = new HashMap<>(); + + public TraceState(StructuredTrace trace) { + this.trace = trace; + for (Entity entity : trace.getEntityList()) { + entityMap.put(entity.getEntityId(), entity); + } + for (Event event : trace.getEventList()) { + eventMap.put(event.getEventId(), event); + } + + for (Event event : trace.getEventList()) { + ByteBuffer childEventId = event.getEventId(); + List eventRefs = eventMap.get(childEventId).getEventRefList(); + if (eventRefs != null) { + eventRefs.stream() + .filter(eventRef -> EventRefType.CHILD_OF == eventRef.getRefType()) + .forEach( + eventRef -> { + ByteBuffer parentEventId = eventRef.getEventId(); + if (!parentToChildrenEventIds.containsKey(parentEventId)) { + parentToChildrenEventIds.put(parentEventId, new ArrayList<>()); + } + parentToChildrenEventIds.get(parentEventId).add(childEventId); + childToParentEventIds.put(childEventId, parentEventId); + }); + } + // expected only 1 childOf relationship + } + } + } +} From 7db93f7138a66ae6f1b5e093024742159fbca5e4 Mon Sep 17 00:00:00 2001 From: Rishabh Singh Date: Mon, 11 Jan 2021 23:11:59 +0530 Subject: [PATCH 2/6] Fix build --- .../org/hypertrace/traceenricher/trace/util/ApiTraceGraph.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/ApiTraceGraph.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/ApiTraceGraph.java index 83e2b96f2..4e3f27273 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/ApiTraceGraph.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/ApiTraceGraph.java @@ -32,7 +32,7 @@ public class ApiTraceGraph { private static final String UNKNOWN_SPAN_KIND_VALUE = EnrichedSpanConstants.getValue(AttributeValue.ATTRIBUTE_VALUE_UNKNOWN); - private final List> nodeList; + private List> nodeList; private final List apiNodeEventEdgeList; private final StructuredTrace trace; private final Map eventIdToIndexInTrace; From c6653d9a7d0251438c2f62fc75caf2116640a553 Mon Sep 17 00:00:00 2001 From: Rishabh Singh Date: Mon, 11 Jan 2021 23:53:43 +0530 Subject: [PATCH 3/6] Minor change --- .../viewgenerator/generators/BaseViewGenerator.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/BaseViewGenerator.java b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/BaseViewGenerator.java index b7d16d305..21543af15 100644 --- a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/BaseViewGenerator.java +++ b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/BaseViewGenerator.java @@ -68,10 +68,10 @@ public List process(StructuredTrace trace) { DataflowMetricUtils.reportArrivalLagAndInsertTimestamp(trace, viewGeneratorArrivalTimer, VIEW_GENERATION_ARRIVAL_TIME); TraceState traceState = ViewGeneratorState.getTraceState(trace); - Map entityMap = traceState.entityMap; - Map eventMap = traceState.eventMap; - Map> parentToChildrenEventIds = traceState.parentToChildrenEventIds; - Map childToParentEventIds = traceState.childToParentEventIds; + Map entityMap = Collections.unmodifiableMap(traceState.entityMap); + Map eventMap = Collections.unmodifiableMap(traceState.eventMap); + Map> parentToChildrenEventIds = Collections.unmodifiableMap(traceState.parentToChildrenEventIds); + Map childToParentEventIds = Collections.unmodifiableMap(traceState.childToParentEventIds); return generateView( trace, From f27447a77529f7f26488306992bc2a5f375f2ac1 Mon Sep 17 00:00:00 2001 From: Rishabh Singh Date: Tue, 12 Jan 2021 15:15:24 +0530 Subject: [PATCH 4/6] Add test --- .../generators/BaseViewGenerator.java | 8 +- .../generators/ViewGeneratorState.java | 36 +++-- .../generators/ViewGeneratorStateTest.java | 130 ++++++++++++++++++ 3 files changed, 161 insertions(+), 13 deletions(-) create mode 100644 hypertrace-view-generator/hypertrace-view-generator/src/test/java/org/hypertrace/viewgenerator/generators/ViewGeneratorStateTest.java diff --git a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/BaseViewGenerator.java b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/BaseViewGenerator.java index 21543af15..3fce703ba 100644 --- a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/BaseViewGenerator.java +++ b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/BaseViewGenerator.java @@ -68,10 +68,10 @@ public List process(StructuredTrace trace) { DataflowMetricUtils.reportArrivalLagAndInsertTimestamp(trace, viewGeneratorArrivalTimer, VIEW_GENERATION_ARRIVAL_TIME); TraceState traceState = ViewGeneratorState.getTraceState(trace); - Map entityMap = Collections.unmodifiableMap(traceState.entityMap); - Map eventMap = Collections.unmodifiableMap(traceState.eventMap); - Map> parentToChildrenEventIds = Collections.unmodifiableMap(traceState.parentToChildrenEventIds); - Map childToParentEventIds = Collections.unmodifiableMap(traceState.childToParentEventIds); + Map entityMap = Collections.unmodifiableMap(traceState.getEntityMap()); + Map eventMap = Collections.unmodifiableMap(traceState.getEventMap()); + Map> parentToChildrenEventIds = Collections.unmodifiableMap(traceState.getParentToChildrenEventIds()); + Map childToParentEventIds = Collections.unmodifiableMap(traceState.getChildToParentEventIds()); return generateView( trace, diff --git a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/ViewGeneratorState.java b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/ViewGeneratorState.java index 93b0f1c5f..6a323765c 100644 --- a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/ViewGeneratorState.java +++ b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/ViewGeneratorState.java @@ -27,7 +27,7 @@ public static ApiTraceGraph getApiTraceGraph(StructuredTrace trace) { public static TraceState getTraceState(StructuredTrace trace) { if (traceStateThreadLocal.get() == null - || isDifferentTrace(traceStateThreadLocal.get().trace, trace)) { + || isDifferentTrace(traceStateThreadLocal.get().getTrace(), trace)) { traceStateThreadLocal.set(new TraceState(trace)); } return traceStateThreadLocal.get(); @@ -40,10 +40,10 @@ private static boolean isDifferentTrace(StructuredTrace cached, StructuredTrace public static class TraceState { private final StructuredTrace trace; - final Map entityMap = new HashMap<>(); - final Map eventMap = new HashMap<>(); - final Map> parentToChildrenEventIds = new HashMap<>(); - final Map childToParentEventIds = new HashMap<>(); + private final Map entityMap = new HashMap<>(); + private final Map eventMap = new HashMap<>(); + private final Map> parentToChildrenEventIds = new HashMap<>(); + private final Map childToParentEventIds = new HashMap<>(); public TraceState(StructuredTrace trace) { this.trace = trace; @@ -63,15 +63,33 @@ public TraceState(StructuredTrace trace) { .forEach( eventRef -> { ByteBuffer parentEventId = eventRef.getEventId(); - if (!parentToChildrenEventIds.containsKey(parentEventId)) { - parentToChildrenEventIds.put(parentEventId, new ArrayList<>()); - } - parentToChildrenEventIds.get(parentEventId).add(childEventId); + parentToChildrenEventIds.computeIfAbsent( + parentEventId, v -> new ArrayList<>()).add(childEventId); childToParentEventIds.put(childEventId, parentEventId); }); } // expected only 1 childOf relationship } } + + public StructuredTrace getTrace() { + return trace; + } + + public Map getEntityMap() { + return entityMap; + } + + public Map getEventMap() { + return eventMap; + } + + public Map> getParentToChildrenEventIds() { + return parentToChildrenEventIds; + } + + public Map getChildToParentEventIds() { + return childToParentEventIds; + } } } diff --git a/hypertrace-view-generator/hypertrace-view-generator/src/test/java/org/hypertrace/viewgenerator/generators/ViewGeneratorStateTest.java b/hypertrace-view-generator/hypertrace-view-generator/src/test/java/org/hypertrace/viewgenerator/generators/ViewGeneratorStateTest.java new file mode 100644 index 000000000..03aa0d833 --- /dev/null +++ b/hypertrace-view-generator/hypertrace-view-generator/src/test/java/org/hypertrace/viewgenerator/generators/ViewGeneratorStateTest.java @@ -0,0 +1,130 @@ +package org.hypertrace.viewgenerator.generators; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.nio.ByteBuffer; +import java.util.Arrays; +import java.util.Collections; +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.StructuredTrace; +import org.hypertrace.traceenricher.trace.util.ApiTraceGraph; +import org.hypertrace.viewgenerator.generators.ViewGeneratorState.TraceState; +import org.junit.jupiter.api.Test; + +public class ViewGeneratorStateTest { + + ByteBuffer span1 = ByteBuffer.wrap(("span-1".getBytes())), span2 = ByteBuffer.wrap(("span-2".getBytes())); + String customerId = "customer-1"; + ByteBuffer traceId1 = ByteBuffer.wrap(("trace-1".getBytes())), traceId2 = ByteBuffer.wrap(("trace-2".getBytes())); + + @Test + public void testTraceState() { + TraceState traceState = new TraceState(getTestTrace(customerId, traceId1)); + assertEquals(1, traceState.getEntityMap().size()); + assertEquals(2, traceState.getEventMap().size()); + assertEquals(1, traceState.getChildToParentEventIds().size()); + assertEquals(1, traceState.getParentToChildrenEventIds().size()); + + assertTrue(traceState.getParentToChildrenEventIds().containsKey(span1)); + assertEquals(1, traceState.getParentToChildrenEventIds().get(span1).size()); + assertTrue(traceState.getChildToParentEventIds().containsKey(span2)); + assertEquals(span1, traceState.getChildToParentEventIds().get(span2)); + } + + @Test + public void testGetTraceState() { + StructuredTrace trace = getTestTrace(customerId, traceId1); + TraceState traceState = ViewGeneratorState.getTraceState(trace); + assertNotNull(traceState); + assertEquals(trace, traceState.getTrace()); + + StructuredTrace modifiedTrace = getTestTrace(customerId, traceId1); + modifiedTrace.setEntityList(Arrays.asList( + Entity.newBuilder() + .setCustomerId(customerId) + .setEntityId("entity-2") + .setEntityName("entity-2") + .setEntityType("service") + .build())); + + // same instance should still be returned + TraceState sameTraceState = ViewGeneratorState.getTraceState(modifiedTrace); + assertEquals(traceState, sameTraceState); + + StructuredTrace differentTrace = getTestTrace(customerId, traceId2); + TraceState differentTraceState = ViewGeneratorState.getTraceState(differentTrace); + assertEquals(differentTrace, differentTraceState.getTrace()); + } + + @Test + public void testGetApiTraceGraph() { + StructuredTrace trace = getTestTrace(customerId, traceId1); + ApiTraceGraph apiTraceGraph = ViewGeneratorState.getApiTraceGraph(trace); + assertNotNull(apiTraceGraph); + + StructuredTrace modifiedTrace = getTestTrace(customerId, traceId1); + modifiedTrace.setEntityList(Arrays.asList( + Entity.newBuilder() + .setCustomerId(customerId) + .setEntityId("entity-2") + .setEntityName("entity-2") + .setEntityType("service") + .build())); + + // same instance should still be returned + ApiTraceGraph sameApiTraceGraph = ViewGeneratorState.getApiTraceGraph(modifiedTrace); + assertEquals(apiTraceGraph, sameApiTraceGraph); + + StructuredTrace differentTrace = getTestTrace(customerId, traceId2); + ApiTraceGraph differentApiTraceGraph = ViewGeneratorState.getApiTraceGraph(differentTrace); + assertNotEquals(apiTraceGraph, differentApiTraceGraph); + } + + private StructuredTrace getTestTrace(String customerId, ByteBuffer traceId) { + return StructuredTrace.newBuilder() + .setCustomerId(customerId) + .setTraceId(traceId) + .setStartTimeMillis(20) + .setEndTimeMillis(30) + .setEntityList(Arrays.asList( + Entity.newBuilder() + .setCustomerId(customerId) + .setEntityId("entity-1") + .setEntityName("entity-1") + .setEntityType("service") + .build())) + .setEventList(Arrays.asList( + Event.newBuilder() + .setCustomerId(customerId) + .setEventId(ByteBuffer.wrap(("span-1".getBytes()))) + .setEventName("span-1") + .build(), + Event.newBuilder() + .setCustomerId(customerId) + .setEventId(ByteBuffer.wrap(("span-2".getBytes()))) + .setEventName("span-2") + .setEventRefList(Arrays.asList( + EventRef.newBuilder() + .setTraceId(traceId) + .setRefType(EventRefType.CHILD_OF) + .setEventId(ByteBuffer.wrap(("span-1".getBytes()))) + .build() + )) + .build() + )) + .setEntityEdgeList(Collections.EMPTY_LIST) + .setEntityEventEdgeList(Collections.EMPTY_LIST) + .setEventEdgeList(Collections.EMPTY_LIST) + .setEntityEntityGraph(null) + .setEventEventGraph(null) + .setEntityEventGraph(null) + .setMetrics(null) + .build(); + } +} From 690769591f35a8b7c41a197926d97fb5828d4c64 Mon Sep 17 00:00:00 2001 From: Rishabh Singh Date: Tue, 12 Jan 2021 21:26:43 +0530 Subject: [PATCH 5/6] Check for object reference to see if the trace has changed or not --- .../viewgenerator/generators/RawServiceViewGenerator.java | 1 - .../viewgenerator/generators/ViewGeneratorState.java | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/RawServiceViewGenerator.java b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/RawServiceViewGenerator.java index 4adcf2b82..9ceb7b411 100644 --- a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/RawServiceViewGenerator.java +++ b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/RawServiceViewGenerator.java @@ -31,7 +31,6 @@ List generateView( // Construct ApiTraceGraph and look at all the head spans within each ApiNode ApiTraceGraph apiTraceGraph = ViewGeneratorState.getApiTraceGraph(structuredTrace); - apiTraceGraph = apiTraceGraph.build(); List> apiNodes = apiTraceGraph.getNodeList(); for (ApiNode apiNode : apiNodes) { Event event = apiNode.getHeadEvent(); diff --git a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/ViewGeneratorState.java b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/ViewGeneratorState.java index 6a323765c..6872953ac 100644 --- a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/ViewGeneratorState.java +++ b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/ViewGeneratorState.java @@ -34,8 +34,7 @@ public static TraceState getTraceState(StructuredTrace trace) { } private static boolean isDifferentTrace(StructuredTrace cached, StructuredTrace trace) { - return !cached.getCustomerId().equals(trace.getCustomerId()) - || !cached.getTraceId().equals(trace.getTraceId()); + return cached != trace; } public static class TraceState { From b4eb66469f1394606bb53f8b5373d64f501b2e53 Mon Sep 17 00:00:00 2001 From: Rishabh Singh Date: Tue, 12 Jan 2021 21:35:30 +0530 Subject: [PATCH 6/6] Fix build --- .../generators/ViewGeneratorStateTest.java | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/hypertrace-view-generator/hypertrace-view-generator/src/test/java/org/hypertrace/viewgenerator/generators/ViewGeneratorStateTest.java b/hypertrace-view-generator/hypertrace-view-generator/src/test/java/org/hypertrace/viewgenerator/generators/ViewGeneratorStateTest.java index 03aa0d833..da5fc5806 100644 --- a/hypertrace-view-generator/hypertrace-view-generator/src/test/java/org/hypertrace/viewgenerator/generators/ViewGeneratorStateTest.java +++ b/hypertrace-view-generator/hypertrace-view-generator/src/test/java/org/hypertrace/viewgenerator/generators/ViewGeneratorStateTest.java @@ -44,6 +44,9 @@ public void testGetTraceState() { assertNotNull(traceState); assertEquals(trace, traceState.getTrace()); + TraceState sameTraceState = ViewGeneratorState.getTraceState(trace); + assertEquals(sameTraceState, traceState); + StructuredTrace modifiedTrace = getTestTrace(customerId, traceId1); modifiedTrace.setEntityList(Arrays.asList( Entity.newBuilder() @@ -53,13 +56,13 @@ public void testGetTraceState() { .setEntityType("service") .build())); - // same instance should still be returned - TraceState sameTraceState = ViewGeneratorState.getTraceState(modifiedTrace); - assertEquals(traceState, sameTraceState); + // same trace id but different object should result in rebuilding of trace state + TraceState differentTraceState1 = ViewGeneratorState.getTraceState(modifiedTrace); + assertNotEquals(traceState, differentTraceState1); StructuredTrace differentTrace = getTestTrace(customerId, traceId2); - TraceState differentTraceState = ViewGeneratorState.getTraceState(differentTrace); - assertEquals(differentTrace, differentTraceState.getTrace()); + TraceState differentTraceState2 = ViewGeneratorState.getTraceState(differentTrace); + assertEquals(differentTrace, differentTraceState2.getTrace()); } @Test @@ -68,6 +71,9 @@ public void testGetApiTraceGraph() { ApiTraceGraph apiTraceGraph = ViewGeneratorState.getApiTraceGraph(trace); assertNotNull(apiTraceGraph); + ApiTraceGraph sameApiTraceGraph = ViewGeneratorState.getApiTraceGraph(trace); + assertEquals(sameApiTraceGraph, apiTraceGraph); + StructuredTrace modifiedTrace = getTestTrace(customerId, traceId1); modifiedTrace.setEntityList(Arrays.asList( Entity.newBuilder() @@ -77,13 +83,14 @@ public void testGetApiTraceGraph() { .setEntityType("service") .build())); - // same instance should still be returned - ApiTraceGraph sameApiTraceGraph = ViewGeneratorState.getApiTraceGraph(modifiedTrace); - assertEquals(apiTraceGraph, sameApiTraceGraph); + + // same trace id but different object should result in rebuilding of trace state + ApiTraceGraph differentApiTraceGraph1 = ViewGeneratorState.getApiTraceGraph(modifiedTrace); + assertNotEquals(apiTraceGraph, differentApiTraceGraph1); StructuredTrace differentTrace = getTestTrace(customerId, traceId2); - ApiTraceGraph differentApiTraceGraph = ViewGeneratorState.getApiTraceGraph(differentTrace); - assertNotEquals(apiTraceGraph, differentApiTraceGraph); + ApiTraceGraph differentApiTraceGraph2 = ViewGeneratorState.getApiTraceGraph(differentTrace); + assertNotEquals(apiTraceGraph, differentApiTraceGraph2); } private StructuredTrace getTestTrace(String customerId, ByteBuffer traceId) {