From a9a48ee36e2614f4176ecad9fc17806af98980df Mon Sep 17 00:00:00 2001 From: rish691 Date: Tue, 20 Jul 2021 14:07:52 +0530 Subject: [PATCH 1/6] Add logs --- .../ApiBoundaryTypeAttributeEnricher.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java index cf4fdbda0..40ac8d497 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java @@ -30,6 +30,9 @@ * Entry / Exit span kind to determine if it's true API entry/exit. */ public class ApiBoundaryTypeAttributeEnricher extends AbstractTraceEnricher { + + private static final Logger LOG = LoggerFactory.getLogger(DefaultServiceEntityEnricher.class); + private static final String COLON = ":"; private static final Splitter COLON_SPLITTER = Splitter.on(COLON); private static final String HOST_HEADER = EnrichedSpanConstants.getValue(Http.HTTP_HOST); @@ -79,6 +82,28 @@ public void enrichEvent(StructuredTrace trace, Event event) { // does not need to build the full traversal graph, just get the parents mapping StructuredTraceGraph graph = buildGraph(trace); + if (null == graph.getTraceEntitiesGraph() || null == graph.getTraceEventsGraph()) { + LOG.info("StructuredTraceGraph is not built correctly, trace {}, Is events graph non-null: {}." + + " Is entities graph non-null: {}", + trace, (null != graph.getTraceEventsGraph()), (null != graph.getTraceEntitiesGraph())); + + // build the graph again and check + StructuredTraceGraph tempGraph = StructuredTraceGraph.createGraph(trace); + LOG.info("Recreating StructuredTraceGraph. Is events graph non-null: {}." + + " Is entities graph non-null: {}", + (null != tempGraph.getTraceEventsGraph()), (null != tempGraph.getTraceEntitiesGraph())); + + tempGraph = StructuredTraceGraph.reCreateTraceEventsGraph(trace); + LOG.info("Recreating events graph. Is events graph non-null: {}." + + " Is entities graph non-null: {}", + (null != tempGraph.getTraceEventsGraph()), (null != tempGraph.getTraceEntitiesGraph())); + + tempGraph = StructuredTraceGraph.reCreateTraceEntitiesGraph(trace); + LOG.info("Recreating entities graph. Is events graph non-null: {}." + + " Is entities graph non-null: {}", + (null != tempGraph.getTraceEventsGraph()), (null != tempGraph.getTraceEntitiesGraph())); + } + if (isEntrySpan) { /* Determines if this is entry span is an entry to api From f2767bd1663b03b4635b2eb048b51c1813d80b90 Mon Sep 17 00:00:00 2001 From: rish691 Date: Tue, 20 Jul 2021 14:09:45 +0530 Subject: [PATCH 2/6] spotless --- .../ApiBoundaryTypeAttributeEnricher.java | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java index 40ac8d497..a06f51aa3 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java @@ -24,6 +24,8 @@ import org.hypertrace.traceenricher.enrichedspan.constants.v1.Http; import org.hypertrace.traceenricher.enrichedspan.constants.v1.Protocol; import org.hypertrace.traceenricher.enrichment.AbstractTraceEnricher; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * This is to determine if the span is the entry / exit point for a particular API. We can't use the @@ -83,25 +85,34 @@ public void enrichEvent(StructuredTrace trace, Event event) { StructuredTraceGraph graph = buildGraph(trace); if (null == graph.getTraceEntitiesGraph() || null == graph.getTraceEventsGraph()) { - LOG.info("StructuredTraceGraph is not built correctly, trace {}, Is events graph non-null: {}." + LOG.info( + "StructuredTraceGraph is not built correctly, trace {}, Is events graph non-null: {}." + " Is entities graph non-null: {}", - trace, (null != graph.getTraceEventsGraph()), (null != graph.getTraceEntitiesGraph())); + trace, + (null != graph.getTraceEventsGraph()), + (null != graph.getTraceEntitiesGraph())); // build the graph again and check StructuredTraceGraph tempGraph = StructuredTraceGraph.createGraph(trace); - LOG.info("Recreating StructuredTraceGraph. Is events graph non-null: {}." + LOG.info( + "Recreating StructuredTraceGraph. Is events graph non-null: {}." + " Is entities graph non-null: {}", - (null != tempGraph.getTraceEventsGraph()), (null != tempGraph.getTraceEntitiesGraph())); + (null != tempGraph.getTraceEventsGraph()), + (null != tempGraph.getTraceEntitiesGraph())); tempGraph = StructuredTraceGraph.reCreateTraceEventsGraph(trace); - LOG.info("Recreating events graph. Is events graph non-null: {}." + LOG.info( + "Recreating events graph. Is events graph non-null: {}." + " Is entities graph non-null: {}", - (null != tempGraph.getTraceEventsGraph()), (null != tempGraph.getTraceEntitiesGraph())); + (null != tempGraph.getTraceEventsGraph()), + (null != tempGraph.getTraceEntitiesGraph())); tempGraph = StructuredTraceGraph.reCreateTraceEntitiesGraph(trace); - LOG.info("Recreating entities graph. Is events graph non-null: {}." + LOG.info( + "Recreating entities graph. Is events graph non-null: {}." + " Is entities graph non-null: {}", - (null != tempGraph.getTraceEventsGraph()), (null != tempGraph.getTraceEntitiesGraph())); + (null != tempGraph.getTraceEventsGraph()), + (null != tempGraph.getTraceEntitiesGraph())); } if (isEntrySpan) { From 3e6adcc6ce90f7fc0e1abbcdff4d387d30166e9c Mon Sep 17 00:00:00 2001 From: rish691 Date: Tue, 20 Jul 2021 15:45:04 +0530 Subject: [PATCH 3/6] Review comment --- .../util/StructuredTraceGraphBuilder.java | 37 +++++++++++++++++++ .../ApiBoundaryTypeAttributeEnricher.java | 36 ------------------ 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilder.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilder.java index a94e79a4e..2a5102aee 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilder.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilder.java @@ -31,6 +31,7 @@ public static StructuredTraceGraph buildGraph(StructuredTrace trace) { trace.getCustomerId()); cachedTraceThreadLocal.set(StructuredTrace.newBuilder(trace).build()); cachedGraphThreadLocal.set(graph); + debugGraph("Case: Rebuilding the graph.", graph, trace); return graph; } @@ -46,8 +47,44 @@ public static StructuredTraceGraph buildGraph(StructuredTrace trace) { trace.getCustomerId()); cachedTraceThreadLocal.set(StructuredTrace.newBuilder(trace).build()); cachedGraphThreadLocal.set(graph); + debugGraph("Case: Partially building the graph.", graph, trace); return graph; } + + debugGraph("Case: Not building the graph.", cachedGraphThreadLocal.get(), trace); return cachedGraphThreadLocal.get(); } + + private static void debugGraph(String logPrefix, StructuredTraceGraph graph, StructuredTrace trace) { + if (null == graph.getTraceEntitiesGraph() || null == graph.getTraceEventsGraph()) { + LOG.info( + logPrefix + "StructuredTraceGraph is not built correctly, trace {}, Is events graph non-null: {}." + + " Is entities graph non-null: {}", + trace, + (null != graph.getTraceEventsGraph()), + (null != graph.getTraceEntitiesGraph())); + + // build the graph again and check + StructuredTraceGraph tempGraph = StructuredTraceGraph.createGraph(trace); + LOG.info( + logPrefix + "Recreating StructuredTraceGraph. Is events graph non-null: {}." + + " Is entities graph non-null: {}", + (null != tempGraph.getTraceEventsGraph()), + (null != tempGraph.getTraceEntitiesGraph())); + + tempGraph = StructuredTraceGraph.reCreateTraceEventsGraph(trace); + LOG.info( + logPrefix + "Recreating events graph. Is events graph non-null: {}." + + " Is entities graph non-null: {}", + (null != tempGraph.getTraceEventsGraph()), + (null != tempGraph.getTraceEntitiesGraph())); + + tempGraph = StructuredTraceGraph.reCreateTraceEntitiesGraph(trace); + LOG.info( + logPrefix + "Recreating entities graph. Is events graph non-null: {}." + + " Is entities graph non-null: {}", + (null != tempGraph.getTraceEventsGraph()), + (null != tempGraph.getTraceEntitiesGraph())); + } + } } diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java index a06f51aa3..cf4fdbda0 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-impl/src/main/java/org/hypertrace/traceenricher/enrichment/enrichers/ApiBoundaryTypeAttributeEnricher.java @@ -24,17 +24,12 @@ import org.hypertrace.traceenricher.enrichedspan.constants.v1.Http; import org.hypertrace.traceenricher.enrichedspan.constants.v1.Protocol; import org.hypertrace.traceenricher.enrichment.AbstractTraceEnricher; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * This is to determine if the span is the entry / exit point for a particular API. We can't use the * Entry / Exit span kind to determine if it's true API entry/exit. */ public class ApiBoundaryTypeAttributeEnricher extends AbstractTraceEnricher { - - private static final Logger LOG = LoggerFactory.getLogger(DefaultServiceEntityEnricher.class); - private static final String COLON = ":"; private static final Splitter COLON_SPLITTER = Splitter.on(COLON); private static final String HOST_HEADER = EnrichedSpanConstants.getValue(Http.HTTP_HOST); @@ -84,37 +79,6 @@ public void enrichEvent(StructuredTrace trace, Event event) { // does not need to build the full traversal graph, just get the parents mapping StructuredTraceGraph graph = buildGraph(trace); - if (null == graph.getTraceEntitiesGraph() || null == graph.getTraceEventsGraph()) { - LOG.info( - "StructuredTraceGraph is not built correctly, trace {}, Is events graph non-null: {}." - + " Is entities graph non-null: {}", - trace, - (null != graph.getTraceEventsGraph()), - (null != graph.getTraceEntitiesGraph())); - - // build the graph again and check - StructuredTraceGraph tempGraph = StructuredTraceGraph.createGraph(trace); - LOG.info( - "Recreating StructuredTraceGraph. Is events graph non-null: {}." - + " Is entities graph non-null: {}", - (null != tempGraph.getTraceEventsGraph()), - (null != tempGraph.getTraceEntitiesGraph())); - - tempGraph = StructuredTraceGraph.reCreateTraceEventsGraph(trace); - LOG.info( - "Recreating events graph. Is events graph non-null: {}." - + " Is entities graph non-null: {}", - (null != tempGraph.getTraceEventsGraph()), - (null != tempGraph.getTraceEntitiesGraph())); - - tempGraph = StructuredTraceGraph.reCreateTraceEntitiesGraph(trace); - LOG.info( - "Recreating entities graph. Is events graph non-null: {}." - + " Is entities graph non-null: {}", - (null != tempGraph.getTraceEventsGraph()), - (null != tempGraph.getTraceEntitiesGraph())); - } - if (isEntrySpan) { /* Determines if this is entry span is an entry to api From a3dbff44c63ae1d2aff10d1bcd2c65ca643953dd Mon Sep 17 00:00:00 2001 From: rish691 Date: Tue, 20 Jul 2021 15:47:00 +0530 Subject: [PATCH 4/6] spotless --- .../trace/util/StructuredTraceGraphBuilder.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilder.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilder.java index 2a5102aee..fd0545d5b 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilder.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilder.java @@ -55,10 +55,12 @@ public static StructuredTraceGraph buildGraph(StructuredTrace trace) { return cachedGraphThreadLocal.get(); } - private static void debugGraph(String logPrefix, StructuredTraceGraph graph, StructuredTrace trace) { + private static void debugGraph( + String logPrefix, StructuredTraceGraph graph, StructuredTrace trace) { if (null == graph.getTraceEntitiesGraph() || null == graph.getTraceEventsGraph()) { LOG.info( - logPrefix + "StructuredTraceGraph is not built correctly, trace {}, Is events graph non-null: {}." + logPrefix + + "StructuredTraceGraph is not built correctly, trace {}, Is events graph non-null: {}." + " Is entities graph non-null: {}", trace, (null != graph.getTraceEventsGraph()), @@ -67,21 +69,24 @@ private static void debugGraph(String logPrefix, StructuredTraceGraph graph, Str // build the graph again and check StructuredTraceGraph tempGraph = StructuredTraceGraph.createGraph(trace); LOG.info( - logPrefix + "Recreating StructuredTraceGraph. Is events graph non-null: {}." + logPrefix + + "Recreating StructuredTraceGraph. Is events graph non-null: {}." + " Is entities graph non-null: {}", (null != tempGraph.getTraceEventsGraph()), (null != tempGraph.getTraceEntitiesGraph())); tempGraph = StructuredTraceGraph.reCreateTraceEventsGraph(trace); LOG.info( - logPrefix + "Recreating events graph. Is events graph non-null: {}." + logPrefix + + "Recreating events graph. Is events graph non-null: {}." + " Is entities graph non-null: {}", (null != tempGraph.getTraceEventsGraph()), (null != tempGraph.getTraceEntitiesGraph())); tempGraph = StructuredTraceGraph.reCreateTraceEntitiesGraph(trace); LOG.info( - logPrefix + "Recreating entities graph. Is events graph non-null: {}." + logPrefix + + "Recreating entities graph. Is events graph non-null: {}." + " Is entities graph non-null: {}", (null != tempGraph.getTraceEventsGraph()), (null != tempGraph.getTraceEntitiesGraph())); From 4cb9a64ba26cb3927e0c060f829f09c973195226 Mon Sep 17 00:00:00 2001 From: rish691 Date: Tue, 20 Jul 2021 16:26:53 +0530 Subject: [PATCH 5/6] fix test --- .../trace/util/StructuredTraceGraphBuilder.java | 2 +- .../util/StructuredTraceGraphBuilderTest.java | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilder.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilder.java index fd0545d5b..80efa851d 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilder.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilder.java @@ -57,7 +57,7 @@ public static StructuredTraceGraph buildGraph(StructuredTrace trace) { private static void debugGraph( String logPrefix, StructuredTraceGraph graph, StructuredTrace trace) { - if (null == graph.getTraceEntitiesGraph() || null == graph.getTraceEventsGraph()) { + if (null != graph && (null == graph.getTraceEntitiesGraph() || null == graph.getTraceEventsGraph())) { LOG.info( logPrefix + "StructuredTraceGraph is not built correctly, trace {}, Is events graph non-null: {}." diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/test/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilderTest.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/test/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilderTest.java index 509b52b18..eb7919190 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/test/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilderTest.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/test/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilderTest.java @@ -51,18 +51,25 @@ void testBuildGraph() { structuredTraceGraphMockedStatic .when(() -> StructuredTraceGraph.createGraph(underTestTrace)) .thenReturn(mockedStructuredTraceGraph); - + structuredTraceGraphMockedStatic + .when(() -> StructuredTraceGraph.reCreateTraceEntitiesGraph(underTestTrace)) + .thenReturn(mockedStructuredTraceGraph); + structuredTraceGraphMockedStatic + .when(() -> StructuredTraceGraph.reCreateTraceEventsGraph(underTestTrace)) + .thenReturn(mockedStructuredTraceGraph); // calls first time StructuredTraceGraph actual = StructuredTraceGraphBuilder.buildGraph(underTestTrace); Assertions.assertEquals(mockedStructuredTraceGraph, actual); structuredTraceGraphMockedStatic.verify( - () -> StructuredTraceGraph.createGraph(underTestTrace), times(1)); + // todo change times to 1 when logging is changed to debug level in StructuredTraceGraphBuilder#buildGraph + () -> StructuredTraceGraph.createGraph(underTestTrace), times(2)); // calls second time, this time it returns from cache StructuredTraceGraphBuilder.buildGraph(underTestTrace); structuredTraceGraphMockedStatic.verify( - () -> StructuredTraceGraph.createGraph(underTestTrace), times(1)); + // todo change times to 1 when logging is changed to debug level in StructuredTraceGraphBuilder#buildGraph + () -> StructuredTraceGraph.createGraph(underTestTrace), times(3)); } } } -} +} \ No newline at end of file From 21adf2941f949a104d706e6b916b0273508fd97d Mon Sep 17 00:00:00 2001 From: rish691 Date: Tue, 20 Jul 2021 16:27:27 +0530 Subject: [PATCH 6/6] fix test --- .../trace/util/StructuredTraceGraphBuilder.java | 3 ++- .../trace/util/StructuredTraceGraphBuilderTest.java | 8 +++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilder.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilder.java index 80efa851d..43d837469 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilder.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/main/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilder.java @@ -57,7 +57,8 @@ public static StructuredTraceGraph buildGraph(StructuredTrace trace) { private static void debugGraph( String logPrefix, StructuredTraceGraph graph, StructuredTrace trace) { - if (null != graph && (null == graph.getTraceEntitiesGraph() || null == graph.getTraceEventsGraph())) { + if (null != graph + && (null == graph.getTraceEntitiesGraph() || null == graph.getTraceEventsGraph())) { LOG.info( logPrefix + "StructuredTraceGraph is not built correctly, trace {}, Is events graph non-null: {}." diff --git a/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/test/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilderTest.java b/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/test/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilderTest.java index eb7919190..172da26af 100644 --- a/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/test/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilderTest.java +++ b/hypertrace-trace-enricher/hypertrace-trace-enricher-api/src/test/java/org/hypertrace/traceenricher/trace/util/StructuredTraceGraphBuilderTest.java @@ -61,15 +61,17 @@ void testBuildGraph() { StructuredTraceGraph actual = StructuredTraceGraphBuilder.buildGraph(underTestTrace); Assertions.assertEquals(mockedStructuredTraceGraph, actual); structuredTraceGraphMockedStatic.verify( - // todo change times to 1 when logging is changed to debug level in StructuredTraceGraphBuilder#buildGraph + // todo change times to 1 when logging is changed to debug level in + // StructuredTraceGraphBuilder#buildGraph () -> StructuredTraceGraph.createGraph(underTestTrace), times(2)); // calls second time, this time it returns from cache StructuredTraceGraphBuilder.buildGraph(underTestTrace); structuredTraceGraphMockedStatic.verify( - // todo change times to 1 when logging is changed to debug level in StructuredTraceGraphBuilder#buildGraph + // todo change times to 1 when logging is changed to debug level in + // StructuredTraceGraphBuilder#buildGraph () -> StructuredTraceGraph.createGraph(underTestTrace), times(3)); } } } -} \ No newline at end of file +}