Cherrypick the JFR fix and prepare for the 1.7.3 release (#657)

CHANGELOG.md

@@ -8,6 +8,12 @@ and this repository adheres to [Semantic Versioning](https://semver.org/spec/v2.
 
 ## Unreleased
 
+## v1.7.3 - 2022-02-02
+
+### Bugfixes
+
+- Fixed a bug that caused JFR events to appear out of order.
+
 ## v1.7.2 - 2022-01-31
 
 ### General

README.md

@@ -159,7 +159,7 @@ To extend the instrumentation with the OpenTelemetry Instrumentation for Java,
 you have to use a compatible API version.
 
 <!-- IMPORTANT: do not change comments or break those lines below -->
-The Splunk Distribution of OpenTelemetry Java version <!--SPLUNK_VERSION-->1.7.2<!--SPLUNK_VERSION--> is compatible
+The Splunk Distribution of OpenTelemetry Java version <!--SPLUNK_VERSION-->1.7.3<!--SPLUNK_VERSION--> is compatible
 with:
 
 * OpenTelemetry API version <!--OTEL_VERSION-->1.10.1<!--OTEL_VERSION-->

deployments/cloudfoundry/buildpack/README.md

@@ -40,7 +40,7 @@ If you want to use a specific version of the Java agent in your application, you
 environment variable before application deployment, either using `cf set-env` or the `manifest.yml` file:
 
 ```sh
-$ cf set-env SPLUNK_OTEL_JAVA_VERSION 1.7.2
+$ cf set-env SPLUNK_OTEL_JAVA_VERSION 1.7.3
 ```
 
 By default, the [latest](https://github.com/signalfx/splunk-otel-java/releases/latest) available agent version is used.

profiler/src/main/java/com/splunk/opentelemetry/profiler/EventProcessingChain.java

@@ -23,7 +23,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.PriorityQueue;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
 import jdk.jfr.EventType;
@@ -38,19 +37,27 @@ class EventProcessingChain {
   private final SpanContextualizer spanContextualizer;
   private final ThreadDumpProcessor threadDumpProcessor;
   private final TLABProcessor tlabProcessor;
-  private final PriorityQueue<RecordedEvent> buffer =
-      new PriorityQueue<>(Comparator.comparing(RecordedEvent::getStartTime));
+  private final List<RecordedEvent> buffer = new ArrayList<>();
   private final EventStats eventStats =
       logger.isDebugEnabled() ? new EventStatsImpl() : new NoOpEventStats();
-  private final ChunkTracker chunkTracker = new ChunkTracker();
+  private final ChunkTracker chunkTracker;
 
   EventProcessingChain(
       SpanContextualizer spanContextualizer,
       ThreadDumpProcessor threadDumpProcessor,
       TLABProcessor tlabProcessor) {
+    this(spanContextualizer, threadDumpProcessor, tlabProcessor, new ChunkTracker());
+  }
+
+  EventProcessingChain(
+      SpanContextualizer spanContextualizer,
+      ThreadDumpProcessor threadDumpProcessor,
+      TLABProcessor tlabProcessor,
+      ChunkTracker chunkTracker) {
     this.spanContextualizer = spanContextualizer;
     this.threadDumpProcessor = threadDumpProcessor;
     this.tlabProcessor = tlabProcessor;
+    this.chunkTracker = chunkTracker;
   }
 
   void accept(RecordedEvent event) {
@@ -83,7 +90,9 @@ void accept(RecordedEvent event) {
    * the buffer. After flushing, the buffer will be empty.
    */
   public void flushBuffer() {
-    buffer.forEach(dispatchContextualizedThreadDumps());
+    buffer.stream()
+        .sorted(Comparator.comparing(RecordedEvent::getStartTime))
+        .forEach(dispatchContextualizedThreadDumps());
     buffer.clear();
     chunkTracker.reset();
   }
@@ -203,7 +212,8 @@ public void close() {
    * type of context attach or thread dump event doesn't match what we have recorded. When chunk
    * change is detected process buffered events and clear the buffer.
    */
-  private static class ChunkTracker {
+  // visible for tests
+  static class ChunkTracker {
     private EventType contextAttachedEventType = null;
     private EventType threadDumpEventType = null;
 

profiler/src/test/java/com/splunk/opentelemetry/profiler/EventProcessingChainTest.java

@@ -16,7 +16,9 @@
 
 package com.splunk.opentelemetry.profiler;
 
+import static java.time.temporal.ChronoUnit.SECONDS;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.isA;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
@@ -155,6 +157,40 @@ void testFlushOnChunkChange() {
     }
   }
 
+  @Test
+  void eventsDispatchedInTimeOrder() {
+    EventType contextAttachedType = newEventType(ContextAttached.EVENT_NAME);
+    EventType threadDumpType = newEventType(ThreadDumpProcessor.EVENT_NAME);
+    Instant now = Instant.now();
+    RecordedEvent event1 = newEvent(contextAttachedType, now.plus(1, SECONDS));
+    RecordedEvent event2 = newEvent(contextAttachedType, now.plus(2, SECONDS));
+    RecordedEvent event3 = newEvent(threadDumpType, now.plus(3, SECONDS));
+    // new event type for a new chunk
+    RecordedEvent event4 = newEvent(newEventType(ThreadDumpProcessor.EVENT_NAME), null);
+
+    // Our events are in time order: context1, context2, threadDump1, threadDump2.
+    // However, we will send them to the chain out of order: context2, context1, threadDump.
+    // Expectation is that we see them dispatched in the correct order.
+    // The "chunk" is finished only after event4, and event4 is part of a new chunk so is not
+    // dispatched.
+
+    EventProcessingChain.ChunkTracker chunkTracker = mock(EventProcessingChain.ChunkTracker.class);
+    when(chunkTracker.contextAttached(isA(RecordedEvent.class))).thenReturn(false, false);
+    when(chunkTracker.threadDump(isA(RecordedEvent.class))).thenReturn(false, true);
+    EventProcessingChain chain =
+        new EventProcessingChain(contextualizer, threadDumpProcessor, tlabProcessor, chunkTracker);
+    chain.accept(event2); // Out of order
+    chain.accept(event1); // Out of order
+    chain.accept(event3);
+    chain.accept(event4);
+
+    InOrder ordered = inOrder(contextualizer, threadDumpProcessor);
+    ordered.verify(contextualizer).updateContext(event1);
+    ordered.verify(contextualizer).updateContext(event2);
+    ordered.verify(threadDumpProcessor).accept(event3);
+    ordered.verifyNoMoreInteractions();
+  }
+
   private EventType newEventType(String name) {
     EventType type = mock(EventType.class);
     when(type.getName()).thenReturn(name);
@@ -163,7 +199,9 @@ private EventType newEventType(String name) {
 
   private RecordedEvent newEvent(EventType eventType, Instant startTime) {
     RecordedEvent event = mock(RecordedEvent.class);
-    when(event.getEventType()).thenReturn(eventType);
+    if (eventType != null) {
+      when(event.getEventType()).thenReturn(eventType);
+    }
     if (startTime != null) {
       when(event.getStartTime()).thenReturn(startTime);
     }