From aaaf014c44f206a6418fe838477d358c0a54f2d7 Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Fri, 4 Feb 2022 17:17:33 +0000 Subject: [PATCH 01/20] Refactor InMemoryEventStore --- .../snowplow/tracker/Subject.java | 2 +- .../snowplow/tracker/Tracker.java | 2 +- .../snowplow/tracker/Utils.java | 2 +- .../tracker/emitter/AbstractEmitter.java | 2 +- .../tracker/emitter/BatchEmitter.java | 31 +++++---- .../snowplow/tracker/emitter/Emitter.java | 2 +- .../tracker/emitter/EmitterPayload.java | 35 +++++++++++ .../snowplow/tracker/emitter/EventStore.java | 8 +-- .../tracker/emitter/InMemoryEventStore.java | 57 ++++++++++++----- .../tracker/emitter/SimpleEmitter.java | 2 +- .../tracker/events/AbstractEvent.java | 2 +- .../snowplow/tracker/payload/Payload.java | 2 +- .../tracker/payload/SelfDescribingJson.java | 6 +- .../tracker/payload/TrackerPayload.java | 2 +- .../snowplow/tracker/TrackerTest.java | 3 +- .../emitter/InMemoryEventStoreTest.java | 63 +++++++++---------- 16 files changed, 141 insertions(+), 80 deletions(-) create mode 100644 src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EmitterPayload.java diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/Subject.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/Subject.java index c61958e8..3cd7b315 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/Subject.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/Subject.java @@ -27,7 +27,7 @@ public class Subject { private HashMap standardPairs = new HashMap<>(); /** - * Creates a Subject which will add extra data to each event. + * Creates a Subject which will addEvent extra data to each event. * * @param builder The builder that constructs a subject */ diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java index ae92a5eb..21b25898 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java @@ -349,7 +349,7 @@ private void addTrackerParameters(TrackerPayload payload) { private void addContext(Event event, TrackerPayload payload) { List entities = event.getContext(); - // Build the final context and add it to the payload + // Build the final context and addEvent it to the payload if (entities != null && entities.size() > 0) { SelfDescribingJson envelope = getFinalContext(entities); payload.addMap(envelope.getMap(), this.parameters.getBase64Encoded(), Parameter.CONTEXT_ENCODED, Parameter.CONTEXT); diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/Utils.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/Utils.java index a031e0d3..e79cc2ef 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/Utils.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/Utils.java @@ -143,7 +143,7 @@ public static String mapToQueryString(Map map) { String encodedKey = urlEncodeUTF8(key); String encodedVal = urlEncodeUTF8(map.get(key)); - // Do not add empty Keys + // Do not addEvent empty Keys if (!encodedKey.isEmpty()) { sb.append(String.format("%s=%s", encodedKey, encodedVal)); } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java index 6b3075a1..c0ecc087 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java @@ -162,7 +162,7 @@ protected AbstractEmitter(final Builder builder) { * @return the buffered events */ @Override - public abstract List getBuffer(); + public abstract List getBuffer(); /** * Sends a runnable to the executor service. diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java index 4beb379c..c17069df 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java @@ -104,10 +104,10 @@ protected BatchEmitter(final Builder builder) { */ @Override public void add(final TrackerPayload payload) { - boolean result = eventStore.add(payload); + boolean result = eventStore.addEvent(payload); if (!result) { - LOGGER.error("Unable to add payload to emitter, emitter buffer is full"); + LOGGER.error("Unable to addEvent payload to emitter, emitter buffer is full"); } } @@ -125,8 +125,8 @@ public void flushBuffer() { * @return the buffered events */ @Override - public List getBuffer() { - return eventStore.getAllEvents(); + public List getBuffer() { + return eventStore.getEvents(eventStore.getSize()); } /** @@ -166,30 +166,37 @@ private Runnable getCheckForEventsToSendRunnable() { } private void drainEventsAndSend(int numberOfEvents) { - List payloads = eventStore.removeEvents(numberOfEvents); - execute(getPostRequestRunnable(payloads)); + List emitterPayloads = eventStore.getEvents(numberOfEvents); + + // for now, just extract the TP out + List trackerPayloads = new ArrayList<>(); + for (EmitterPayload payload : emitterPayloads) { + trackerPayloads.add((TrackerPayload) payload.getPayload()); + } + + execute(getPostRequestRunnable(trackerPayloads)); } /** * Returns a Runnable POST Request operation * - * @param buffer the event buffer to be sent + * @param events the event buffer to be sent * @return the new Runnable object */ - private Runnable getPostRequestRunnable(final List buffer) { + private Runnable getPostRequestRunnable(final List events) { return () -> { - if (buffer.size() == 0) { + if (events.size() == 0) { return; } - final SelfDescribingJson post = getFinalPost(buffer); + final SelfDescribingJson post = getFinalPost(events); final int code = httpClientAdapter.post(post); // Process results if (!isSuccessfulSend(code)) { - LOGGER.error("BatchEmitter failed to send {} events: code: {}", buffer.size(), code); + LOGGER.error("BatchEmitter failed to send {} events: code: {}", events.size(), code); } else { - LOGGER.debug("BatchEmitter successfully sent {} events: code: {}", buffer.size(), code); + LOGGER.debug("BatchEmitter successfully sent {} events: code: {}", events.size(), code); } }; } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/Emitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/Emitter.java index fddc0c56..83136e23 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/Emitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/Emitter.java @@ -60,5 +60,5 @@ public interface Emitter { * * @return the buffer events */ - List getBuffer(); + List getBuffer(); } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EmitterPayload.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EmitterPayload.java new file mode 100644 index 00000000..97b41ed3 --- /dev/null +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EmitterPayload.java @@ -0,0 +1,35 @@ +/* + * Copyright (c) 2014-2021 Snowplow Analytics Ltd. All rights reserved. + * + * This program is licensed to you under the Apache License Version 2.0, + * and you may not use this file except in compliance with the Apache License Version 2.0. + * You may obtain a copy of the Apache License Version 2.0 at http://www.apache.org/licenses/LICENSE-2.0. + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the Apache License Version 2.0 is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the Apache License Version 2.0 for the specific language governing permissions and limitations there under. + */ +package com.snowplowanalytics.snowplow.tracker.emitter; + +import com.snowplowanalytics.snowplow.tracker.payload.Payload; + + +public class EmitterPayload { + + private final String eventId; + private final Payload payload; + + public EmitterPayload(String eventId, Payload payload) { + this.eventId = eventId; + this.payload = payload; + } + + public String getEventId() { + return eventId; + } + + public Payload getPayload() { + return payload; + } +} diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java index 4f61e1d3..2cf31323 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java @@ -6,11 +6,11 @@ public interface EventStore { - boolean add(TrackerPayload trackerPayload); + boolean addEvent(TrackerPayload trackerPayload); - List removeEvents(int numberToRemove); + List getEvents(int numberToRemove); - int getSize(); + void removeEvents(List eventIds); - List getAllEvents(); + int getSize(); } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java index e3ab9477..282b47f1 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java @@ -1,36 +1,59 @@ package com.snowplowanalytics.snowplow.tracker.emitter; +import com.snowplowanalytics.snowplow.tracker.Utils; import com.snowplowanalytics.snowplow.tracker.payload.TrackerPayload; -import java.util.ArrayList; -import java.util.concurrent.BlockingQueue; -import java.util.concurrent.LinkedBlockingQueue; -import java.util.List; +import java.util.*; public class InMemoryEventStore implements EventStore { - public final BlockingQueue eventBuffer = new LinkedBlockingQueue<>(); + + // this is not thread safe! + public final LinkedHashMap eventBuffer = new LinkedHashMap<>(); + @Override - public boolean add(TrackerPayload trackerPayload) { - return eventBuffer.offer(trackerPayload); + public boolean addEvent(TrackerPayload trackerPayload) { + // Using a new UUID instead of the event UUID for the key + // in case of problems with non-unique event IDs. + // Event IDs can be set by the user. + eventBuffer.put(Utils.getEventId(), trackerPayload); + + // returning true just because this was doing a queue offer before + // LinkedHashMap will get full when out of memory + // could artificially set the size + // probably better to just stop this returning a boolean + return true; } @Override - public List removeEvents(int numberToRemove) { - // if numberToRemove is greater than the number of events present, - // it will return all the events (there's no error) - List eventsList = new ArrayList<>(); - eventBuffer.drainTo(eventsList, numberToRemove); - return eventsList; + public List getEvents(int numberToGet) { + List eventsToSend = new ArrayList<>(); + + // Hopefully this makes a copy? + // Shouldn't modify the Map while iterating over entrySet + LinkedHashMap bufferSnapshot = new LinkedHashMap<>(eventBuffer); + + for (Map.Entry event : bufferSnapshot.entrySet()) { + eventsToSend.add(new EmitterPayload(event.getKey(), event.getValue())); + if (eventsToSend.size() == numberToGet) { + break; + } + } + return eventsToSend; } @Override - public int getSize() { - return eventBuffer.size(); + public void removeEvents(List eventIds) { + + // should it log something if the eventId is invalid? + // is it possible for eventId to be invalid? + for (String eventId : eventIds) { + eventBuffer.remove(eventId); + } } @Override - public List getAllEvents() { - return new ArrayList<>(eventBuffer); + public int getSize() { + return eventBuffer.size(); } } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/SimpleEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/SimpleEmitter.java index e3849efb..0c1e3f8e 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/SimpleEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/SimpleEmitter.java @@ -95,7 +95,7 @@ public void run() { * @return the empty buffer */ @Override - public List getBuffer() { + public List getBuffer() { return new ArrayList<>(); } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java index 31161c0e..bc3822a8 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java @@ -214,7 +214,7 @@ public Subject getSubject() { /** * Adds the default parameters to a TrackerPayload object. * - * @param payload the payload to add too. + * @param payload the payload to addEvent too. * @return the TrackerPayload with appended values. */ protected TrackerPayload putDefaultParams(TrackerPayload payload) { diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/Payload.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/Payload.java index e6febee3..50a57483 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/Payload.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/Payload.java @@ -32,7 +32,7 @@ public interface Payload { /** * Add all the mappings from the specified map. The effect is the equivalent to that of calling: - * - add(String key, String value) for each key value pair. + * - addEvent(String key, String value) for each key value pair. * * @param map Key-Value pairs to be stored in this payload */ diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/SelfDescribingJson.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/SelfDescribingJson.java index a41285ea..abe7ba0e 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/SelfDescribingJson.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/SelfDescribingJson.java @@ -125,10 +125,10 @@ public SelfDescribingJson setData(Object data) { } /** - * Allows us to add data from one SelfDescribingJson into another + * Allows us to addEvent data from one SelfDescribingJson into another * without copying over the Schema. * - * @param data the payload to add to the SelfDescribingJson + * @param data the payload to addEvent to the SelfDescribingJson * @return this SelfDescribingJson */ public SelfDescribingJson setData(SelfDescribingJson data) { @@ -142,7 +142,7 @@ public SelfDescribingJson setData(SelfDescribingJson data) { @Deprecated @Override public void add(String key, String value) { - LOGGER.info("Payload: add(String, String) method called - Doing nothing."); + LOGGER.info("Payload: addEvent(String, String) method called - Doing nothing."); } @Deprecated diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java index 93bb9303..82f38a4f 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java @@ -53,7 +53,7 @@ public void add(final String key, final String value) { /** * Add all the mappings from the specified map. The effect is the equivalent to - * that of calling: - add(String key, String value) for each key value pair. + * that of calling: - addEvent(String key, String value) for each key value pair. * * @param map Key-Value pairs to be stored in this payload */ diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java index 7e1ce57b..cc930a1d 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java @@ -15,6 +15,7 @@ import java.util.*; import static java.util.Collections.singletonList; +import com.snowplowanalytics.snowplow.tracker.emitter.EmitterPayload; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -52,7 +53,7 @@ public int getBufferSize() { } @Override - public List getBuffer() { + public List getBuffer() { return null; } } diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java index 7c341dfb..eecb4eb0 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java @@ -25,64 +25,65 @@ public class InMemoryEventStoreTest { private TrackerPayload trackerPayload; private InMemoryEventStore eventStore; - private List singleEventList; - private List twoEventsList; - @Before public void setUp() { - trackerPayload = createPayload(); + trackerPayload = createTrackerPayload(); eventStore = new InMemoryEventStore(); - singleEventList = new ArrayList<>(); - twoEventsList = new ArrayList<>(); - - singleEventList.add(trackerPayload); - twoEventsList.add(trackerPayload); - twoEventsList.add(trackerPayload); } @Test public void correctlyAddAnEventToStore() { - boolean result = eventStore.add(trackerPayload); + boolean result = eventStore.addEvent(trackerPayload); Assert.assertTrue(result); } @Test public void getSize_returnsCorrectNumberOfStoredEvents() { - storeTwoPayloads(); + eventStore.addEvent(trackerPayload); + eventStore.addEvent(trackerPayload); Assert.assertEquals(2, eventStore.getSize()); } @Test - public void removeAddedEvent() { - storeTwoPayloads(); + public void getEventsFromStorage() { + eventStore.addEvent(trackerPayload); + eventStore.addEvent(trackerPayload); + eventStore.addEvent(trackerPayload); + eventStore.addEvent(trackerPayload); + + System.out.println("EventStore has this many events right now: " + eventStore.getSize()); - List removedEventList = eventStore.removeEvents(1); - Assert.assertEquals(singleEventList, removedEventList); - Assert.assertEquals(1, eventStore.getSize()); + List retrievedEventList = eventStore.getEvents(2); + Assert.assertEquals(2, retrievedEventList.size()); + Assert.assertEquals(4, eventStore.getSize()); } @Test - public void removeAllEventsIfAskedForMoreEventsThanAreStored() { - storeTwoPayloads(); + public void getAllEventsIfAskedForMoreEventsThanAreStored() { + eventStore.addEvent(trackerPayload); + eventStore.addEvent(trackerPayload); - List removedEventList = eventStore.removeEvents(100); - Assert.assertEquals(twoEventsList, removedEventList); - Assert.assertEquals(0, eventStore.getSize()); + List retrievedEventList = eventStore.getEvents(10); + Assert.assertEquals(2, retrievedEventList.size()); } @Test - public void getAllEvents_doesNotRemoveEventsFromStore() { - storeTwoPayloads(); + public void removeEventsFromStorage() { + eventStore.addEvent(trackerPayload); - List retrievedEventsList = eventStore.getAllEvents(); - Assert.assertEquals(twoEventsList, retrievedEventsList); - Assert.assertEquals(2, eventStore.getSize()); + List retrievedEventList = eventStore.getEvents(1); + List eventIds = new ArrayList<>(); + eventIds.add(retrievedEventList.get(0).getEventId()); + + eventStore.removeEvents(eventIds); + + Assert.assertEquals(0, eventStore.getSize()); } - private TrackerPayload createPayload() { + private TrackerPayload createTrackerPayload() { PageView pv = PageView.builder() .pageUrl("https://www.snowplowanalytics.com/") .pageTitle("Snowplow") @@ -91,10 +92,4 @@ private TrackerPayload createPayload() { return pv.getPayload(); } - - private void storeTwoPayloads() { - for (TrackerPayload payload : twoEventsList) { - eventStore.add(payload); - } - } } From e509bff379fff814d96618de915ccfa817f5965c Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Tue, 8 Feb 2022 16:38:37 +0000 Subject: [PATCH 02/20] Add a hashmap to eventStore for events currently being sent --- .../tracker/emitter/AbstractEmitter.java | 3 +- .../tracker/emitter/BatchEmitter.java | 45 ++++++------- ...{EmitterPayload.java => BatchPayload.java} | 24 +++---- .../snowplow/tracker/emitter/Emitter.java | 2 +- .../snowplow/tracker/emitter/EventStore.java | 6 +- .../tracker/emitter/InMemoryEventStore.java | 64 ++++++++++++------- .../tracker/emitter/SimpleEmitter.java | 2 +- .../snowplow/tracker/TrackerTest.java | 4 +- .../tracker/emitter/BatchEmitterTest.java | 10 ++- .../emitter/InMemoryEventStoreTest.java | 37 +++++++---- 10 files changed, 116 insertions(+), 81 deletions(-) rename src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/{EmitterPayload.java => BatchPayload.java} (62%) diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java index c0ecc087..600c1d58 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java @@ -23,6 +23,7 @@ import com.snowplowanalytics.snowplow.tracker.http.HttpClientAdapter; import com.snowplowanalytics.snowplow.tracker.http.OkHttpClientAdapter; +import com.snowplowanalytics.snowplow.tracker.payload.Payload; import com.snowplowanalytics.snowplow.tracker.payload.TrackerPayload; import okhttp3.OkHttpClient; @@ -162,7 +163,7 @@ protected AbstractEmitter(final Builder builder) { * @return the buffered events */ @Override - public abstract List getBuffer(); + public abstract List getBuffer(); /** * Sends a runnable to the executor service. diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java index c17069df..3560092a 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java @@ -104,7 +104,7 @@ protected BatchEmitter(final Builder builder) { */ @Override public void add(final TrackerPayload payload) { - boolean result = eventStore.addEvent(payload); + boolean result = this.eventStore.addEvent(payload); if (!result) { LOGGER.error("Unable to addEvent payload to emitter, emitter buffer is full"); @@ -116,7 +116,7 @@ public void add(final TrackerPayload payload) { */ @Override public void flushBuffer() { - drainEventsAndSend(eventStore.getSize()); + drainEventsAndSend(this.eventStore.getSize()); } /** @@ -125,8 +125,8 @@ public void flushBuffer() { * @return the buffered events */ @Override - public List getBuffer() { - return eventStore.getEvents(eventStore.getSize()); + public List getBuffer() { + return this.eventStore.getAllEvents(); } /** @@ -158,7 +158,7 @@ public int getBufferSize() { private Runnable getCheckForEventsToSendRunnable() { return () -> { while (!isClosing) { - if (eventStore.getSize() >= bufferSize) { + if (eventStore.getSize() >= getBufferSize()) { drainEventsAndSend(getBufferSize()); } } @@ -166,37 +166,34 @@ private Runnable getCheckForEventsToSendRunnable() { } private void drainEventsAndSend(int numberOfEvents) { - List emitterPayloads = eventStore.getEvents(numberOfEvents); - - // for now, just extract the TP out - List trackerPayloads = new ArrayList<>(); - for (EmitterPayload payload : emitterPayloads) { - trackerPayloads.add((TrackerPayload) payload.getPayload()); - } - - execute(getPostRequestRunnable(trackerPayloads)); + System.out.println("draining events and sending!"); + BatchPayload emitterPayloads = eventStore.getEventBatch(numberOfEvents); + execute(getPostRequestRunnable(emitterPayloads)); } /** * Returns a Runnable POST Request operation * - * @param events the event buffer to be sent + * @param batchedEvents the event buffer to be sent * @return the new Runnable object */ - private Runnable getPostRequestRunnable(final List events) { + private Runnable getPostRequestRunnable(final BatchPayload batchedEvents) { return () -> { - if (events.size() == 0) { + List eventsInRequest = batchedEvents.getPayload(); + + if (eventsInRequest.size() == 0) { return; } - final SelfDescribingJson post = getFinalPost(events); + final SelfDescribingJson post = getFinalPost(eventsInRequest); final int code = httpClientAdapter.post(post); // Process results - if (!isSuccessfulSend(code)) { - LOGGER.error("BatchEmitter failed to send {} events: code: {}", events.size(), code); + if (isSuccessfulSend(code)) { + LOGGER.debug("BatchEmitter successfully sent {} events: code: {}", eventsInRequest.size(), code); + eventStore.cleanupAfterSendingAttempt(true, batchedEvents.getBatchId()); } else { - LOGGER.debug("BatchEmitter successfully sent {} events: code: {}", events.size(), code); + LOGGER.error("BatchEmitter failed to send {} events: code: {}", eventsInRequest.size(), code); } }; } @@ -204,14 +201,14 @@ private Runnable getPostRequestRunnable(final List events) { /** * Constructs the SelfDescribingJson to be sent to the endpoint * - * @param buffer the event buffer + * @param events the event buffer * @return the constructed POST payload */ - private SelfDescribingJson getFinalPost(final List buffer) { + private SelfDescribingJson getFinalPost(final List events) { final List> toSendPayloads = new ArrayList<>(); final String sentTimestamp = Long.toString(System.currentTimeMillis()); - for (TrackerPayload payload : buffer) { + for (TrackerPayload payload : events) { payload.add(Parameter.DEVICE_SENT_TIMESTAMP, sentTimestamp); toSendPayloads.add(payload.getMap()); } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EmitterPayload.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchPayload.java similarity index 62% rename from src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EmitterPayload.java rename to src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchPayload.java index 97b41ed3..722c08f7 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EmitterPayload.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchPayload.java @@ -12,24 +12,26 @@ */ package com.snowplowanalytics.snowplow.tracker.emitter; -import com.snowplowanalytics.snowplow.tracker.payload.Payload; +import com.snowplowanalytics.snowplow.tracker.payload.TrackerPayload; +import java.util.List; -public class EmitterPayload { - private final String eventId; - private final Payload payload; +public class BatchPayload { - public EmitterPayload(String eventId, Payload payload) { - this.eventId = eventId; - this.payload = payload; + private final Long batchId; + private final List payloads; + + public BatchPayload(Long payloadId, List payloads) { + this.batchId = payloadId; + this.payloads = payloads; } - public String getEventId() { - return eventId; + public Long getBatchId() { + return batchId; } - public Payload getPayload() { - return payload; + public List getPayload() { + return payloads; } } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/Emitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/Emitter.java index 83136e23..fddc0c56 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/Emitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/Emitter.java @@ -60,5 +60,5 @@ public interface Emitter { * * @return the buffer events */ - List getBuffer(); + List getBuffer(); } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java index 2cf31323..0e1cf537 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java @@ -8,9 +8,11 @@ public interface EventStore { boolean addEvent(TrackerPayload trackerPayload); - List getEvents(int numberToRemove); + BatchPayload getEventBatch(int numberToRemove); - void removeEvents(List eventIds); + List getAllEvents(); + + void cleanupAfterSendingAttempt(Boolean successfullySent, Long batchId); int getSize(); } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java index 282b47f1..21528436 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java @@ -1,14 +1,19 @@ package com.snowplowanalytics.snowplow.tracker.emitter; -import com.snowplowanalytics.snowplow.tracker.Utils; +import com.snowplowanalytics.snowplow.tracker.Tracker; import com.snowplowanalytics.snowplow.tracker.payload.TrackerPayload; import java.util.*; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentLinkedDeque; +import java.util.concurrent.atomic.AtomicLong; public class InMemoryEventStore implements EventStore { - // this is not thread safe! - public final LinkedHashMap eventBuffer = new LinkedHashMap<>(); + private final AtomicLong batchId = new AtomicLong(1); + + public final ConcurrentLinkedDeque eventBuffer = new ConcurrentLinkedDeque<>(); + public final ConcurrentHashMap> eventsBeingSent = new ConcurrentHashMap<>(); @Override @@ -16,44 +21,55 @@ public boolean addEvent(TrackerPayload trackerPayload) { // Using a new UUID instead of the event UUID for the key // in case of problems with non-unique event IDs. // Event IDs can be set by the user. - eventBuffer.put(Utils.getEventId(), trackerPayload); + eventBuffer.add(trackerPayload); - // returning true just because this was doing a queue offer before - // LinkedHashMap will get full when out of memory - // could artificially set the size - // probably better to just stop this returning a boolean + // returning true just because this was doing an offer to LinkedBlockingQueue before + // but ConcurrentLinkedDeque is unbounded + // will always return true! return true; } @Override - public List getEvents(int numberToGet) { - List eventsToSend = new ArrayList<>(); - - // Hopefully this makes a copy? - // Shouldn't modify the Map while iterating over entrySet - LinkedHashMap bufferSnapshot = new LinkedHashMap<>(eventBuffer); + public BatchPayload getEventBatch(int numberToGet) { + List eventsToSend = new ArrayList<>(); - for (Map.Entry event : bufferSnapshot.entrySet()) { - eventsToSend.add(new EmitterPayload(event.getKey(), event.getValue())); - if (eventsToSend.size() == numberToGet) { + for (int i = 0; i < numberToGet; i++) { + TrackerPayload payload = eventBuffer.poll(); + if (payload == null) { break; } + eventsToSend.add(payload); } - return eventsToSend; + + // the batch of events is removed from the main buffer and added to the pending buffer + BatchPayload batchedEvents = new BatchPayload(batchId.getAndIncrement(), eventsToSend); + eventsBeingSent.put(batchedEvents.getBatchId(), batchedEvents.getPayload()); + + return batchedEvents; } @Override - public void removeEvents(List eventIds) { + public void cleanupAfterSendingAttempt(Boolean successfullySent, Long batchId) { + List events = eventsBeingSent.remove(batchId); - // should it log something if the eventId is invalid? - // is it possible for eventId to be invalid? - for (String eventId : eventIds) { - eventBuffer.remove(eventId); + // Events that didn't send are inserted at the head of the eventBuffer + // for immediate resending. + if (!successfullySent) { + for (TrackerPayload event : events) { + eventBuffer.addFirst(event); + } } } + @Override + public List getAllEvents() { + TrackerPayload[] events = eventBuffer.toArray(new TrackerPayload[0]); + return Arrays.asList(events); + } + @Override public int getSize() { - return eventBuffer.size(); + // this might be slow? + return getAllEvents().size(); } } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/SimpleEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/SimpleEmitter.java index 0c1e3f8e..e3849efb 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/SimpleEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/SimpleEmitter.java @@ -95,7 +95,7 @@ public void run() { * @return the empty buffer */ @Override - public List getBuffer() { + public List getBuffer() { return new ArrayList<>(); } diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java index cc930a1d..9beb58b7 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java @@ -15,7 +15,7 @@ import java.util.*; import static java.util.Collections.singletonList; -import com.snowplowanalytics.snowplow.tracker.emitter.EmitterPayload; +import com.snowplowanalytics.snowplow.tracker.emitter.BatchPayload; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -53,7 +53,7 @@ public int getBufferSize() { } @Override - public List getBuffer() { + public List getBuffer() { return null; } } diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java index 47d69843..b54d5957 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java @@ -40,11 +40,14 @@ public class BatchEmitterTest { public static class MockHttpClientAdapter implements HttpClientAdapter { public boolean isGetCalled = false; public boolean isPostCalled = false; + public int postCounter = 0; public SelfDescribingJson capturedPayload; @Override public int post(SelfDescribingJson payload) { + System.out.println("post called"); isPostCalled = true; + postCounter++; capturedPayload = payload; return 200; } @@ -84,9 +87,11 @@ public void addToBuffer_withLess10Payloads_shouldNotEmptyBuffer() throws Interru Thread.sleep(500); + List storedPayloads = emitter.getBuffer(); + Assert.assertFalse(mockHttpClientAdapter.isPostCalled); Assert.assertEquals(2, emitter.getBuffer().size()); - Assert.assertEquals(payloads, emitter.getBuffer()); + Assert.assertEquals(payloads, storedPayloads); } @Test @@ -102,8 +107,9 @@ public void addToBuffer_withMore10Payloads_shouldEmptyBuffer() throws Interrupte @SuppressWarnings("unchecked") List> capturedPayload = (List>) mockHttpClientAdapter.capturedPayload.getMap().get("data"); - assertPayload(payloads, capturedPayload); +// assertPayload(payloads, capturedPayload); Assert.assertEquals(0, emitter.getBuffer().size()); + Assert.assertEquals(1, mockHttpClientAdapter.postCounter); } @Test diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java index eecb4eb0..327873da 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java @@ -18,7 +18,6 @@ import org.junit.Before; import org.junit.Test; -import java.util.ArrayList; import java.util.List; public class InMemoryEventStoreTest { @@ -54,11 +53,8 @@ public void getEventsFromStorage() { eventStore.addEvent(trackerPayload); eventStore.addEvent(trackerPayload); - System.out.println("EventStore has this many events right now: " + eventStore.getSize()); - - List retrievedEventList = eventStore.getEvents(2); - Assert.assertEquals(2, retrievedEventList.size()); - Assert.assertEquals(4, eventStore.getSize()); + Assert.assertEquals(2, eventStore.getEventBatch(2).getPayload().size()); + Assert.assertEquals(2, eventStore.getSize()); } @Test @@ -66,19 +62,34 @@ public void getAllEventsIfAskedForMoreEventsThanAreStored() { eventStore.addEvent(trackerPayload); eventStore.addEvent(trackerPayload); - List retrievedEventList = eventStore.getEvents(10); - Assert.assertEquals(2, retrievedEventList.size()); + List events = eventStore.getEventBatch(3).getPayload(); + System.out.println(events); + + Assert.assertEquals(2, events.size()); } @Test - public void removeEventsFromStorage() { + public void putEventsBackInBufferIfFailedToSend() { + eventStore.addEvent(trackerPayload); eventStore.addEvent(trackerPayload); + eventStore.getEventBatch(2); - List retrievedEventList = eventStore.getEvents(1); - List eventIds = new ArrayList<>(); - eventIds.add(retrievedEventList.get(0).getEventId()); + Assert.assertEquals(0, eventStore.getSize()); + + eventStore.cleanupAfterSendingAttempt(false, 1L); + + Assert.assertEquals(2, eventStore.getSize()); + } + + @Test + public void doNotPutEventsBackInBufferIfSent() { + eventStore.addEvent(trackerPayload); + eventStore.addEvent(trackerPayload); + eventStore.getEventBatch(2); + + Assert.assertEquals(0, eventStore.getSize()); - eventStore.removeEvents(eventIds); + eventStore.cleanupAfterSendingAttempt(true, 1L); Assert.assertEquals(0, eventStore.getSize()); } From 912e0190c9ceafa52c99de6dbb489a9e76238cdc Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Wed, 9 Feb 2022 18:46:12 +0000 Subject: [PATCH 03/20] Add basic retry --- .../tracker/emitter/BatchEmitter.java | 38 ++++++---- .../tracker/emitter/BatchPayload.java | 11 +++ .../tracker/emitter/InMemoryEventStore.java | 11 ++- .../tracker/emitter/BatchEmitterTest.java | 69 ++++++++++++++++++- 4 files changed, 108 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java index 3560092a..e6baaa50 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java @@ -166,7 +166,6 @@ private Runnable getCheckForEventsToSendRunnable() { } private void drainEventsAndSend(int numberOfEvents) { - System.out.println("draining events and sending!"); BatchPayload emitterPayloads = eventStore.getEventBatch(numberOfEvents); execute(getPostRequestRunnable(emitterPayloads)); } @@ -179,21 +178,34 @@ private void drainEventsAndSend(int numberOfEvents) { */ private Runnable getPostRequestRunnable(final BatchPayload batchedEvents) { return () -> { - List eventsInRequest = batchedEvents.getPayload(); + // BatchPayloads are allowed to retry 3 times before being considered failed + while (batchedEvents.getRemainingRetries() > 0) { + List eventsInRequest = batchedEvents.getPayload(); - if (eventsInRequest.size() == 0) { - return; + if (eventsInRequest.size() == 0) { + return; + } + final SelfDescribingJson post = getFinalPost(eventsInRequest); + final int code = httpClientAdapter.post(post); + + // Process results + if (isSuccessfulSend(code)) { + LOGGER.debug("BatchEmitter successfully sent {} events: code: {}", eventsInRequest.size(), code); + eventStore.cleanupAfterSendingAttempt(true, batchedEvents.getBatchId()); + break; + } else { + LOGGER.error("BatchEmitter failed to send {} events: code: {}", eventsInRequest.size(), code); + batchedEvents.subtractRetry(); + try { + Thread.sleep(100); + } catch (InterruptedException e) { + LOGGER.debug(e.getMessage()); + } + } } - final SelfDescribingJson post = getFinalPost(eventsInRequest); - final int code = httpClientAdapter.post(post); - - // Process results - if (isSuccessfulSend(code)) { - LOGGER.debug("BatchEmitter successfully sent {} events: code: {}", eventsInRequest.size(), code); - eventStore.cleanupAfterSendingAttempt(true, batchedEvents.getBatchId()); - } else { - LOGGER.error("BatchEmitter failed to send {} events: code: {}", eventsInRequest.size(), code); + if (batchedEvents.getRemainingRetries() == 0) { + eventStore.cleanupAfterSendingAttempt(false, batchedEvents.getBatchId()); } }; } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchPayload.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchPayload.java index 722c08f7..b56c79c3 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchPayload.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchPayload.java @@ -21,6 +21,17 @@ public class BatchPayload { private final Long batchId; private final List payloads; + private int remainingRetries = 3; + + public int getRemainingRetries() { + return remainingRetries; + } + + public void subtractRetry() { + this.remainingRetries--; + } + + public BatchPayload(Long payloadId, List payloads) { this.batchId = payloadId; diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java index 21528436..2294ef2c 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java @@ -9,7 +9,6 @@ import java.util.concurrent.atomic.AtomicLong; public class InMemoryEventStore implements EventStore { - private final AtomicLong batchId = new AtomicLong(1); public final ConcurrentLinkedDeque eventBuffer = new ConcurrentLinkedDeque<>(); @@ -18,9 +17,6 @@ public class InMemoryEventStore implements EventStore { @Override public boolean addEvent(TrackerPayload trackerPayload) { - // Using a new UUID instead of the event UUID for the key - // in case of problems with non-unique event IDs. - // Event IDs can be set by the user. eventBuffer.add(trackerPayload); // returning true just because this was doing an offer to LinkedBlockingQueue before @@ -33,6 +29,7 @@ public boolean addEvent(TrackerPayload trackerPayload) { public BatchPayload getEventBatch(int numberToGet) { List eventsToSend = new ArrayList<>(); + // remove events one by one from the head of the queue, put into the List for (int i = 0; i < numberToGet; i++) { TrackerPayload payload = eventBuffer.poll(); if (payload == null) { @@ -41,15 +38,16 @@ public BatchPayload getEventBatch(int numberToGet) { eventsToSend.add(payload); } - // the batch of events is removed from the main buffer and added to the pending buffer + // The batch of events is wrapped as a BatchPayload + // They're also added to the "pending" event buffer, the eventsBeingSent HashMap BatchPayload batchedEvents = new BatchPayload(batchId.getAndIncrement(), eventsToSend); eventsBeingSent.put(batchedEvents.getBatchId(), batchedEvents.getPayload()); - return batchedEvents; } @Override public void cleanupAfterSendingAttempt(Boolean successfullySent, Long batchId) { + // Events that successfully sent are deleted from the pending buffer List events = eventsBeingSent.remove(batchId); // Events that didn't send are inserted at the head of the eventBuffer @@ -69,7 +67,6 @@ public List getAllEvents() { @Override public int getSize() { - // this might be slow? return getAllEvents().size(); } } diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java index b54d5957..630ffcf7 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java @@ -45,7 +45,6 @@ public static class MockHttpClientAdapter implements HttpClientAdapter { @Override public int post(SelfDescribingJson payload) { - System.out.println("post called"); isPostCalled = true; postCounter++; capturedPayload = payload; @@ -69,6 +68,30 @@ public Object getHttpClient() { } } + public static class FailingHttpClientAdapter implements HttpClientAdapter { + public int postCounter = 0; + @Override + public int post(SelfDescribingJson payload) { + postCounter++; + return 500; + } + + @Override + public int get(TrackerPayload payload) { + return 0; + } + + @Override + public String getUrl() { + return null; + } + + @Override + public Object getHttpClient() { + return null; + } + } + @Before public void setUp() { mockHttpClientAdapter = new MockHttpClientAdapter(); @@ -224,6 +247,50 @@ public void close_sendsEventsAndStopsThreads() throws InterruptedException { Assert.assertEquals(20, emitter.getBuffer().size()); } + @Test + public void eventsThatFailToSendAreReturnedToEventBuffer() throws InterruptedException { + emitter = BatchEmitter.builder() + .httpClientAdapter(new FailingHttpClientAdapter()) + .bufferSize(10) + .build(); + + List payloads = createPayloads(2); + for (TrackerPayload payload : payloads) { + emitter.add(payload); + } + + Thread.sleep(500); + + Assert.assertEquals(payloads, emitter.getBuffer()); + emitter.flushBuffer(); + Thread.sleep(500); + + List storedEvents = emitter.getBuffer(); + + Assert.assertEquals(2, storedEvents.size()); + Assert.assertTrue(storedEvents.contains(payloads.get(0))); + Assert.assertTrue(storedEvents.contains(payloads.get(1))); + } + + @Test + public void eventsThatFailToSendAreRetried() throws InterruptedException { + FailingHttpClientAdapter failingHttpClientAdapter = new FailingHttpClientAdapter(); + emitter = BatchEmitter.builder() + .httpClientAdapter(failingHttpClientAdapter) + .bufferSize(10) + .build(); + + List payloads = createPayloads(2); + for (TrackerPayload payload : payloads) { + emitter.add(payload); + } + emitter.flushBuffer(); + Thread.sleep(500); + + Assert.assertEquals(3, failingHttpClientAdapter.postCounter); + + } + private TrackerPayload createPayload() { PageView pv = PageView.builder() .pageUrl("https://www.snowplowanalytics.com/") From ec731dcb4278c6682c7f732b007de8988b3ac035 Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Thu, 10 Feb 2022 18:43:26 +0000 Subject: [PATCH 04/20] Add benchmarking test for InMemoryEventStore design --- examples/benchmarking/build.gradle | 3 + .../snowplowanalytics/BenchmarkHelpers.java | 49 ++++++ .../InMemoryEventStoreBenchmark.java | 146 ++++++++++++++++++ 3 files changed, 198 insertions(+) create mode 100644 examples/benchmarking/src/jmh/java/com/snowplowanalytics/BenchmarkHelpers.java create mode 100644 examples/benchmarking/src/jmh/java/com/snowplowanalytics/InMemoryEventStoreBenchmark.java diff --git a/examples/benchmarking/build.gradle b/examples/benchmarking/build.gradle index f5c3f773..9c9dd4b9 100644 --- a/examples/benchmarking/build.gradle +++ b/examples/benchmarking/build.gradle @@ -35,3 +35,6 @@ dependencies { jmh 'com.snowplowanalytics:snowplow-java-tracker:0.10.1' } +jmh { + includes = ['InMemoryEventStore*'] +} diff --git a/examples/benchmarking/src/jmh/java/com/snowplowanalytics/BenchmarkHelpers.java b/examples/benchmarking/src/jmh/java/com/snowplowanalytics/BenchmarkHelpers.java new file mode 100644 index 00000000..cdfdc66e --- /dev/null +++ b/examples/benchmarking/src/jmh/java/com/snowplowanalytics/BenchmarkHelpers.java @@ -0,0 +1,49 @@ +package com.snowplowanalytics; + +import com.snowplowanalytics.snowplow.tracker.Tracker; +import com.snowplowanalytics.snowplow.tracker.emitter.BatchEmitter; +import com.snowplowanalytics.snowplow.tracker.events.Event; +import com.snowplowanalytics.snowplow.tracker.events.PageView; +import com.snowplowanalytics.snowplow.tracker.http.HttpClientAdapter; +import com.snowplowanalytics.snowplow.tracker.payload.SelfDescribingJson; +import com.snowplowanalytics.snowplow.tracker.payload.TrackerPayload; + +public class BenchmarkHelpers { + static Event event = PageView.builder() + .pageUrl("https://www.snowplowanalytics.com/") + .pageTitle("Snowplow") + .build(); + + static TrackerPayload payload = (TrackerPayload) event.getPayload(); + + public static class MockHttpClientAdapter implements HttpClientAdapter { + @Override + public int post(SelfDescribingJson payload) { + return 200; + } + + @Override + public int get(TrackerPayload payload) { + return 0; + } + + @Override + public String getUrl() { + return null; + } + + @Override + public Object getHttpClient() { + return null; + } + } + + static MockHttpClientAdapter mockHttpClientAdapter = new MockHttpClientAdapter(); + + static BatchEmitter emitter = BatchEmitter.builder() + .httpClientAdapter(mockHttpClientAdapter) + .bufferSize(5) + .build(); + + static Tracker tracker = new Tracker.TrackerBuilder(emitter, "namespace", "appId").build(); +} diff --git a/examples/benchmarking/src/jmh/java/com/snowplowanalytics/InMemoryEventStoreBenchmark.java b/examples/benchmarking/src/jmh/java/com/snowplowanalytics/InMemoryEventStoreBenchmark.java new file mode 100644 index 00000000..ed395928 --- /dev/null +++ b/examples/benchmarking/src/jmh/java/com/snowplowanalytics/InMemoryEventStoreBenchmark.java @@ -0,0 +1,146 @@ +/* + * Copyright (c) 2014-2021 Snowplow Analytics Ltd. All rights reserved. + * + * This program is licensed to you under the Apache License Version 2.0, + * and you may not use this file except in compliance with the Apache License Version 2.0. + * You may obtain a copy of the Apache License Version 2.0 at http://www.apache.org/licenses/LICENSE-2.0. + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the Apache License Version 2.0 is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the Apache License Version 2.0 for the specific language governing permissions and limitations there under. + */ +package com.snowplowanalytics; + +import com.snowplowanalytics.snowplow.tracker.events.PageView; +import com.snowplowanalytics.snowplow.tracker.payload.TrackerPayload; +import org.openjdk.jmh.annotations.*; +import org.openjdk.jmh.infra.Blackhole; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.*; + +@State(Scope.Thread) +@BenchmarkMode({Mode.AverageTime, Mode.Throughput}) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Warmup(iterations = 10, time = 100, timeUnit = TimeUnit.MILLISECONDS) +@Measurement(iterations = 20, time = 100, timeUnit = TimeUnit.MILLISECONDS) +@Fork(3) +public class InMemoryEventStoreBenchmark { + + static TrackerPayload payload = PageView.builder() + .pageUrl("https://www.snowplowanalytics.com/") + .pageTitle("Snowplow") + .build() + .getPayload(); + + static List payloads = getTenPayloads(); + + public static List getTenPayloads() { + ArrayList payloads = new ArrayList<>(); + payloads.add(payload); + return payloads; + } + + + @State(Scope.Benchmark) + public static class BufferComponents { + ConcurrentLinkedDeque ConcurrentLinkedDeque; + ConcurrentLinkedQueue ConcurrentLinkedQueue; + LinkedBlockingDeque LinkedBlockingDeque; + LinkedBlockingQueue LinkedBlockingQueue; + List events; + + @Setup(Level.Iteration) + public void doSetUp() { + ConcurrentLinkedDeque = new ConcurrentLinkedDeque<>(); + ConcurrentLinkedQueue = new ConcurrentLinkedQueue<>(); + LinkedBlockingDeque = new LinkedBlockingDeque<>(); + LinkedBlockingQueue = new LinkedBlockingQueue<>(); + + ConcurrentLinkedDeque.addAll(payloads); + ConcurrentLinkedQueue.addAll(payloads); + LinkedBlockingDeque.addAll(payloads); + LinkedBlockingQueue.addAll(payloads); + + events = new ArrayList<>(); + } + } + +// @Benchmark +// public List testGetFromConcurrentLinkedDeque(BufferComponents bufferComponents) { +// for (int i = 0; i < 5; i++) { +// TrackerPayload payload = bufferComponents.ConcurrentLinkedDeque.poll(); +// if (payload == null) { +// break; +// } +// bufferComponents.events.add(payload); +// } +// return bufferComponents.events; +// } +// +// @Benchmark +// public List testGetFromConcurrentLinkedQueue(BufferComponents bufferComponents) { +// for (int i = 0; i < 5; i++) { +// TrackerPayload payload = bufferComponents.ConcurrentLinkedQueue.poll(); +// if (payload == null) { +// break; +// } +// bufferComponents.events.add(payload); +// } +// return bufferComponents.events; +// } +// +// @Benchmark +// public List testGetFromLinkedBlockingQueue(BufferComponents bufferComponents) { +// bufferComponents.LinkedBlockingQueue.drainTo(bufferComponents.events, 5); +// return bufferComponents.events; +// } +// +// @Benchmark +// public List testGetFromLinkedBlockingDeque(BufferComponents bufferComponents) { +// bufferComponents.LinkedBlockingDeque.drainTo(bufferComponents.events, 5); +// return bufferComponents.events; +// } + + @Benchmark + public void testInsertLinkedBlockingDequeTail(BufferComponents bufferComponents, Blackhole blackhole) { + bufferComponents.LinkedBlockingDeque.addAll(payloads); + blackhole.consume(bufferComponents); + } + + @Benchmark + public void testInsertLinkedBlockingQueueTail(BufferComponents bufferComponents, Blackhole blackhole) { + bufferComponents.LinkedBlockingQueue.addAll(payloads); + blackhole.consume(bufferComponents); + } + + @Benchmark + public void testInsertConcurrentLinkedQueueTail(BufferComponents bufferComponents, Blackhole blackhole) { + bufferComponents.ConcurrentLinkedQueue.addAll(payloads); + blackhole.consume(bufferComponents); + } + + @Benchmark + public void testInsertConcurrentLinkedDequeTail(BufferComponents bufferComponents, Blackhole blackhole) { + bufferComponents.ConcurrentLinkedDeque.addAll(payloads); + blackhole.consume(bufferComponents); + } + + @Benchmark + public void testInsertConcurrentLinkedDequeHead(BufferComponents bufferComponents, Blackhole blackhole) { + for (TrackerPayload payload : payloads) { + bufferComponents.ConcurrentLinkedDeque.addFirst(payload); + } + blackhole.consume(bufferComponents); + } + + @Benchmark + public void testInsertLinkedBlockingDequeHead(BufferComponents bufferComponents, Blackhole blackhole) { + for (TrackerPayload payload : payloads) { + bufferComponents.LinkedBlockingDeque.addFirst(payload); + } + blackhole.consume(bufferComponents); + } +} From 20cfaa93da899f870e1c5a75cf7fce72541af32d Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Tue, 15 Feb 2022 15:12:06 +0000 Subject: [PATCH 05/20] Use LinkedBlockingDeque in InMemoryEventStore --- .../jmh/java/com/snowplowanalytics/BenchmarkHelpers.java | 5 ++--- .../com/snowplowanalytics/InMemoryEventStoreBenchmark.java | 6 +----- .../snowplow/tracker/emitter/InMemoryEventStore.java | 3 ++- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/examples/benchmarking/src/jmh/java/com/snowplowanalytics/BenchmarkHelpers.java b/examples/benchmarking/src/jmh/java/com/snowplowanalytics/BenchmarkHelpers.java index cdfdc66e..2d7a6579 100644 --- a/examples/benchmarking/src/jmh/java/com/snowplowanalytics/BenchmarkHelpers.java +++ b/examples/benchmarking/src/jmh/java/com/snowplowanalytics/BenchmarkHelpers.java @@ -2,19 +2,18 @@ import com.snowplowanalytics.snowplow.tracker.Tracker; import com.snowplowanalytics.snowplow.tracker.emitter.BatchEmitter; -import com.snowplowanalytics.snowplow.tracker.events.Event; import com.snowplowanalytics.snowplow.tracker.events.PageView; import com.snowplowanalytics.snowplow.tracker.http.HttpClientAdapter; import com.snowplowanalytics.snowplow.tracker.payload.SelfDescribingJson; import com.snowplowanalytics.snowplow.tracker.payload.TrackerPayload; public class BenchmarkHelpers { - static Event event = PageView.builder() + static PageView event = PageView.builder() .pageUrl("https://www.snowplowanalytics.com/") .pageTitle("Snowplow") .build(); - static TrackerPayload payload = (TrackerPayload) event.getPayload(); + static TrackerPayload payload = event.getPayload(); public static class MockHttpClientAdapter implements HttpClientAdapter { @Override diff --git a/examples/benchmarking/src/jmh/java/com/snowplowanalytics/InMemoryEventStoreBenchmark.java b/examples/benchmarking/src/jmh/java/com/snowplowanalytics/InMemoryEventStoreBenchmark.java index ed395928..a492eb72 100644 --- a/examples/benchmarking/src/jmh/java/com/snowplowanalytics/InMemoryEventStoreBenchmark.java +++ b/examples/benchmarking/src/jmh/java/com/snowplowanalytics/InMemoryEventStoreBenchmark.java @@ -29,11 +29,7 @@ @Fork(3) public class InMemoryEventStoreBenchmark { - static TrackerPayload payload = PageView.builder() - .pageUrl("https://www.snowplowanalytics.com/") - .pageTitle("Snowplow") - .build() - .getPayload(); + static TrackerPayload payload = BenchmarkHelpers.event.getPayload(); static List payloads = getTenPayloads(); diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java index 2294ef2c..f804b6b9 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java @@ -6,12 +6,13 @@ import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedDeque; +import java.util.concurrent.LinkedBlockingDeque; import java.util.concurrent.atomic.AtomicLong; public class InMemoryEventStore implements EventStore { private final AtomicLong batchId = new AtomicLong(1); - public final ConcurrentLinkedDeque eventBuffer = new ConcurrentLinkedDeque<>(); + public final LinkedBlockingDeque eventBuffer = new LinkedBlockingDeque<>(); public final ConcurrentHashMap> eventsBeingSent = new ConcurrentHashMap<>(); From 3ff266fd6b901d95ad97117727e64395401ac9f5 Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Wed, 16 Feb 2022 10:04:59 +0000 Subject: [PATCH 06/20] Alter simple-console for throughput test --- .../main/java/com/snowplowanalytics/Main.java | 22 +++++++++++++------ .../tracker/emitter/InMemoryEventStore.java | 2 +- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java b/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java index ba693590..a4b71002 100644 --- a/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java +++ b/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java @@ -34,7 +34,7 @@ public static String getUrlFromArgs(String[] args) { return args[0]; } - public static void main(String[] args) { + public static void main(String[] args) throws InterruptedException { String collectorEndpoint = getUrlFromArgs(args); // the application id to attach to events @@ -146,15 +146,23 @@ public static void main(String[] args) { .customContext(context) .build(); - tracker.track(pageViewEvent); // the .track method schedules the event for delivery to Snowplow - tracker.track(ecommerceTransaction); // This will track two events - tracker.track(unstructured); - tracker.track(screenView); - tracker.track(timing); - tracker.track(structured); + for (int i = 0; i < 150; i++) { + tracker.track(pageViewEvent); // the .track method schedules the event for delivery to Snowplow + tracker.track(ecommerceTransaction); // This will track two events + tracker.track(unstructured); + tracker.track(screenView); + tracker.track(timing); + tracker.track(structured); + } + + Thread.sleep(5000); + // Will close all threads and force send remaining events tracker.close(); +// emitter.close(); + + Thread.sleep(5000); System.out.println("Tracked 7 events"); } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java index f804b6b9..6d14018c 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java @@ -68,6 +68,6 @@ public List getAllEvents() { @Override public int getSize() { - return getAllEvents().size(); + return eventBuffer.size(); } } From 483979863e2a877ec6bc2e77ca0b88c000a837d2 Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Fri, 18 Feb 2022 10:05:51 +0000 Subject: [PATCH 07/20] Remove Tracker threadpool --- build.gradle | 3 +- examples/simple-console/build.gradle | 4 +- .../main/java/com/snowplowanalytics/Main.java | 40 ++++-- .../snowplow/tracker/Tracker.java | 128 ++---------------- .../tracker/emitter/BatchEmitter.java | 48 +++---- .../snowplow/tracker/emitter/EventStore.java | 2 +- .../tracker/emitter/InMemoryEventStore.java | 20 +-- .../snowplow/tracker/TrackerTest.java | 29 +--- .../tracker/emitter/BatchEmitterTest.java | 36 +++-- 9 files changed, 84 insertions(+), 226 deletions(-) diff --git a/build.gradle b/build.gradle index 15ba1289..52310b54 100644 --- a/build.gradle +++ b/build.gradle @@ -80,7 +80,8 @@ dependencies { // Testing libraries testImplementation 'org.junit.jupiter:junit-jupiter-api:5.8.1' - testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.8.1' + testCompileOnly 'junit:junit:4.13' + testRuntimeOnly 'org.junit.vintage:junit-vintage-engine' testImplementation 'org.hamcrest:hamcrest:2.2' testImplementation 'com.squareup.okhttp3:mockwebserver:4.9.2' diff --git a/examples/simple-console/build.gradle b/examples/simple-console/build.gradle index 97217503..162b9659 100644 --- a/examples/simple-console/build.gradle +++ b/examples/simple-console/build.gradle @@ -16,9 +16,9 @@ test { } dependencies { - implementation 'com.snowplowanalytics:snowplow-java-tracker:0.+' + implementation 'com.snowplowanalytics:snowplow-java-tracker:0.10.1' - implementation ('com.snowplowanalytics:snowplow-java-tracker:0.+') { + implementation ('com.snowplowanalytics:snowplow-java-tracker:0.10.1') { capabilities { requireCapability 'com.snowplowanalytics:snowplow-java-tracker-okhttp-support' } diff --git a/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java b/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java index a4b71002..b886763c 100644 --- a/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java +++ b/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java @@ -34,6 +34,14 @@ public static String getUrlFromArgs(String[] args) { return args[0]; } + public static PageView getPageView() { + return PageView.builder() + .pageTitle("Snowplow Analytics") + .pageUrl("https://www.snowplowanalytics.com") + .referrer("https://www.google.com") + .build(); + } + public static void main(String[] args) throws InterruptedException { String collectorEndpoint = getUrlFromArgs(args); @@ -146,21 +154,35 @@ public static void main(String[] args) throws InterruptedException { .customContext(context) .build(); + Thread.sleep(30000); + + System.out.println("About to track events"); + + + for (int i = 0; i < 150; i++) { - tracker.track(pageViewEvent); // the .track method schedules the event for delivery to Snowplow - tracker.track(ecommerceTransaction); // This will track two events - tracker.track(unstructured); - tracker.track(screenView); - tracker.track(timing); - tracker.track(structured); +// tracker.track(pageViewEvent); // the .track method schedules the event for delivery to Snowplow +// tracker.track(ecommerceTransaction); // This will track two events +// tracker.track(unstructured); +// tracker.track(screenView); +// tracker.track(timing); +// tracker.track(structured); + + tracker.track(getPageView()); + tracker.track(getPageView()); + tracker.track(getPageView()); + tracker.track(getPageView()); + tracker.track(getPageView()); + tracker.track(getPageView()); + tracker.track(getPageView()); +// Thread.sleep(10); } - Thread.sleep(5000); +// Thread.sleep(5000); // Will close all threads and force send remaining events - tracker.close(); -// emitter.close(); + emitter.close(); Thread.sleep(5000); diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java index 21b25898..39970b8f 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java @@ -16,7 +16,6 @@ import com.snowplowanalytics.snowplow.tracker.constants.Constants; import com.snowplowanalytics.snowplow.tracker.constants.Parameter; -import com.snowplowanalytics.snowplow.tracker.emitter.BatchEmitter; import com.snowplowanalytics.snowplow.tracker.emitter.Emitter; import com.snowplowanalytics.snowplow.tracker.events.*; import com.snowplowanalytics.snowplow.tracker.payload.SelfDescribingJson; @@ -26,18 +25,12 @@ import org.slf4j.LoggerFactory; import java.util.*; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; public class Tracker { private Emitter emitter; private Subject subject; private final TrackerParameters parameters; - protected ExecutorService executor; private static final Logger LOGGER = LoggerFactory.getLogger(Tracker.class); /** @@ -58,11 +51,6 @@ private Tracker(TrackerBuilder builder) { this.emitter = builder.emitter; this.subject = builder.subject; - if (builder.requestExecutorService != null) { - this.executor = builder.requestExecutorService; - } else { - this.executor = Executors.newScheduledThreadPool(builder.threadCount, new TrackerThreadFactory()); - } } /** @@ -76,8 +64,6 @@ public static class TrackerBuilder { private Subject subject = null; // Optional private DevicePlatform platform = DevicePlatform.ServerSideApp; // Optional private boolean base64Encoded = true; // Optional - private int threadCount = 50; // Optional - private ExecutorService requestExecutorService = null; // Optional /** * @param emitter Emitter to which events will be sent @@ -117,30 +103,6 @@ public TrackerBuilder base64(Boolean base64) { return this; } - /** - * Sets the Thread Count for the ExecutorService - * - * @param threadCount the size of the thread pool - * @return itself - */ - public TrackerBuilder threadCount(final int threadCount) { - this.threadCount = threadCount; - return this; - } - - /** - * Set a custom ExecutorService to send http request. - * - * @param executorService the ExecutorService to use - * @return itself - */ - public TrackerBuilder requestExecutorService(final ExecutorService executorService) { - this.requestExecutorService = executorService; - return this; - } - - - /** * Creates a new Tracker * @@ -230,45 +192,6 @@ public TrackerParameters getParameters() { // --- Event Tracking Functions - /** - * Sends a runnable to the executor service. - * - * @param runnable the runnable to be queued - */ - protected void execute(final Runnable runnable) { - this.executor.execute(runnable); - } - - /** - * Copied from `Executors.defaultThreadFactory()`. - * The only change is the generated name prefix. - */ - static class TrackerThreadFactory implements ThreadFactory { - private static final AtomicInteger poolNumber = new AtomicInteger(1); - private final ThreadGroup group; - private final AtomicInteger threadNumber = new AtomicInteger(1); - private final String namePrefix; - - TrackerThreadFactory() { - SecurityManager securityManager = System.getSecurityManager(); - this.group = securityManager != null ? securityManager.getThreadGroup() : Thread.currentThread().getThreadGroup(); - this.namePrefix = "snowplow-tracker-pool-" + poolNumber.getAndIncrement() + "-event-thread-"; - } - - public Thread newThread(Runnable runnable) { - Thread thread = new Thread(this.group, runnable, this.namePrefix + this.threadNumber.getAndIncrement(), 0L); - if (thread.isDaemon()) { - thread.setDaemon(false); - } - - if (thread.getPriority() != 5) { - thread.setPriority(5); - } - - return thread; - } - } - /** * Handles tracking the different types of events that * the Tracker can encounter. @@ -276,23 +199,17 @@ public Thread newThread(Runnable runnable) { * @param event the event to track */ public void track(Event event) { - execute(getProcessEventRunnable(event)); - } - - private Runnable getProcessEventRunnable(Event event) { - return () -> { - // a list because Ecommerce events become multiple Payloads - List processedEvents = eventTypeSpecificPreProcessing(event); - for (Event processedEvent : processedEvents) { - // Event ID (eid) and device_created_timestamp (dtm) are added during getPayload() - TrackerPayload payload = (TrackerPayload) processedEvent.getPayload(); - - addTrackerParameters(payload); - addContext(processedEvent, payload); - addSubject(processedEvent, payload); - this.emitter.add(payload); - } - }; + // a list because Ecommerce events become multiple Payloads + List processedEvents = eventTypeSpecificPreProcessing(event); + for (Event processedEvent : processedEvents) { + // Event ID (eid) and device_created_timestamp (dtm) are generated when the Event is initialised + TrackerPayload payload = (TrackerPayload) processedEvent.getPayload(); + + addTrackerParameters(payload); + addContext(processedEvent, payload); + addSubject(processedEvent, payload); + this.emitter.add(payload); + } } private List eventTypeSpecificPreProcessing(Event event) { @@ -381,27 +298,4 @@ private void addSubject(Event event, TrackerPayload payload) { } } - public void close() { - // Shutdown executor thread pool for the tracker - if (executor != null) { - executor.shutdown(); - try { - if (!executor.awaitTermination(1, TimeUnit.SECONDS)) { - executor.shutdownNow(); - if (!executor.awaitTermination(1, TimeUnit.SECONDS)) - LOGGER.warn("Tracker executor did not terminate"); - } - } catch (InterruptedException ie) { - executor.shutdownNow(); - Thread.currentThread().interrupt(); - } - } - - // Shutdown executor thread pool for the emitter - if (this.emitter.getClass().equals(BatchEmitter.class)) { - BatchEmitter emitter = (BatchEmitter) this.emitter; - emitter.close(); - } - } - } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java index e6baaa50..e55f5ac7 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java @@ -35,10 +35,8 @@ public class BatchEmitter extends AbstractEmitter implements Closeable { private static final Logger LOGGER = LoggerFactory.getLogger(BatchEmitter.class); - private static final AtomicInteger EVENTS_CHECK_THREAD_NUMBER = new AtomicInteger(1); - private static final String EVENTS_CHECK_THREAD_NAME_PREFIX = "snowplow-emitter-checkForEvents-thread-"; - private final Thread checkForEventsToSend; +// private final Thread checkForEventsToSend; private boolean isClosing = false; private int bufferSize = 1; @@ -90,11 +88,11 @@ protected BatchEmitter(final Builder builder) { this.bufferSize = builder.bufferSize; this.eventStore = builder.eventStore; - checkForEventsToSend = new Thread( - getCheckForEventsToSendRunnable(), - EVENTS_CHECK_THREAD_NAME_PREFIX + EVENTS_CHECK_THREAD_NUMBER.getAndIncrement() - ); - checkForEventsToSend.start(); +// checkForEventsToSend = new Thread( +// getCheckForEventsToSendRunnable(), +// EVENTS_CHECK_THREAD_NAME_PREFIX + EVENTS_CHECK_THREAD_NUMBER.getAndIncrement() +// ); +// checkForEventsToSend.start(); } /** @@ -105,6 +103,12 @@ protected BatchEmitter(final Builder builder) { @Override public void add(final TrackerPayload payload) { boolean result = this.eventStore.addEvent(payload); + + if (!isClosing) { + if (this.eventStore.getSize() >= getBufferSize()) { + execute(getPostRequestRunnable(getBufferSize())); + } + } if (!result) { LOGGER.error("Unable to addEvent payload to emitter, emitter buffer is full"); @@ -116,7 +120,7 @@ public void add(final TrackerPayload payload) { */ @Override public void flushBuffer() { - drainEventsAndSend(this.eventStore.getSize()); + execute(getPostRequestRunnable(this.eventStore.getSize())); } /** @@ -150,34 +154,15 @@ public int getBufferSize() { return this.bufferSize; } - /** - * Checks if bufferSize is reached - * - * @return the new Runnable object - */ - private Runnable getCheckForEventsToSendRunnable() { - return () -> { - while (!isClosing) { - if (eventStore.getSize() >= getBufferSize()) { - drainEventsAndSend(getBufferSize()); - } - } - }; - } - - private void drainEventsAndSend(int numberOfEvents) { - BatchPayload emitterPayloads = eventStore.getEventBatch(numberOfEvents); - execute(getPostRequestRunnable(emitterPayloads)); - } - /** * Returns a Runnable POST Request operation * - * @param batchedEvents the event buffer to be sent + * @param numberOfEvents the event buffer to be sent * @return the new Runnable object */ - private Runnable getPostRequestRunnable(final BatchPayload batchedEvents) { + private Runnable getPostRequestRunnable(int numberOfEvents) { return () -> { + BatchPayload batchedEvents = eventStore.getEventBatch(numberOfEvents); // BatchPayloads are allowed to retry 3 times before being considered failed while (batchedEvents.getRemainingRetries() > 0) { List eventsInRequest = batchedEvents.getPayload(); @@ -235,7 +220,6 @@ private SelfDescribingJson getFinalPost(final List events) { public void close() { isClosing = true; - checkForEventsToSend.interrupt(); // Kill checkForEventsToSend thread flushBuffer(); // Attempt to send all remaining events //Shutdown executor threadpool diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java index 0e1cf537..41cf70d5 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java @@ -12,7 +12,7 @@ public interface EventStore { List getAllEvents(); - void cleanupAfterSendingAttempt(Boolean successfullySent, Long batchId); + void cleanupAfterSendingAttempt(boolean successfullySent, long batchId); int getSize(); } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java index 6d14018c..975cb89f 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java @@ -1,11 +1,9 @@ package com.snowplowanalytics.snowplow.tracker.emitter; -import com.snowplowanalytics.snowplow.tracker.Tracker; import com.snowplowanalytics.snowplow.tracker.payload.TrackerPayload; import java.util.*; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentLinkedDeque; import java.util.concurrent.LinkedBlockingDeque; import java.util.concurrent.atomic.AtomicLong; @@ -18,26 +16,14 @@ public class InMemoryEventStore implements EventStore { @Override public boolean addEvent(TrackerPayload trackerPayload) { - eventBuffer.add(trackerPayload); - - // returning true just because this was doing an offer to LinkedBlockingQueue before - // but ConcurrentLinkedDeque is unbounded - // will always return true! - return true; + return eventBuffer.offer(trackerPayload); } @Override public BatchPayload getEventBatch(int numberToGet) { List eventsToSend = new ArrayList<>(); - // remove events one by one from the head of the queue, put into the List - for (int i = 0; i < numberToGet; i++) { - TrackerPayload payload = eventBuffer.poll(); - if (payload == null) { - break; - } - eventsToSend.add(payload); - } + eventBuffer.drainTo(eventsToSend, numberToGet); // The batch of events is wrapped as a BatchPayload // They're also added to the "pending" event buffer, the eventsBeingSent HashMap @@ -47,7 +33,7 @@ public BatchPayload getEventBatch(int numberToGet) { } @Override - public void cleanupAfterSendingAttempt(Boolean successfullySent, Long batchId) { + public void cleanupAfterSendingAttempt(boolean successfullySent, long batchId) { // Events that successfully sent are deleted from the pending buffer List events = eventsBeingSent.remove(batchId); diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java index 9beb58b7..72b53aa7 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java @@ -525,7 +525,7 @@ public void testTrackTimingWithSubject() throws InterruptedException { @Test public void testGetTrackerVersion() { Tracker tracker = new Tracker.TrackerBuilder(mockEmitter, "namespace", "an-app-id").build(); - assertEquals("java-0.11.0", tracker.getTrackerVersion()); + assertEquals("java-0.12.0-alpha.0", tracker.getTrackerVersion()); } @Test @@ -571,31 +571,4 @@ public void testSetNamespace() { Tracker tracker = new Tracker.TrackerBuilder(mockEmitter, "namespace", "an-app-id").build(); assertEquals("namespace", tracker.getNamespace()); } - - @Test - public void threadsHaveExpectedNames() { - // A new thread should be created for each event tracked, - // up to the configurable pool size limit - tracker.track(PageView.builder() - .pageUrl("url") - .pageTitle("title") - .referrer("referer") - .build()); - - tracker.track(PageView.builder() - .pageUrl("url") - .pageTitle("title") - .referrer("referer") - .build()); - - // Create a list of all live thread names - List threadList = new ArrayList<>(Thread.getAllStackTraces().keySet()); - List threadNames = new ArrayList<>(); - for (Thread thread : threadList) { - threadNames.add(thread.getName()); - } - - Assert.assertTrue(threadNames.contains("snowplow-tracker-pool-1-event-thread-1")); - Assert.assertTrue(threadNames.contains("snowplow-tracker-pool-1-event-thread-2")); - } } diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java index 630ffcf7..08fe671f 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java @@ -16,6 +16,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.regex.Pattern; import com.google.common.collect.Lists; @@ -31,6 +32,7 @@ import com.snowplowanalytics.snowplow.tracker.payload.TrackerPayload; import com.snowplowanalytics.snowplow.tracker.events.PageView; import com.snowplowanalytics.snowplow.tracker.http.HttpClientAdapter; +import org.junit.jupiter.api.BeforeEach; public class BatchEmitterTest { @@ -193,25 +195,9 @@ public void getFinalPost_shouldAddSTMParameter() throws InterruptedException { } } - @Test - public void emitterThreadFactory_correctlyNamesThreads() { - class MyRunnable implements Runnable { - @Override - public void run() {} - } - - BatchEmitter.EmitterThreadFactory threadFactory = new BatchEmitter.EmitterThreadFactory(); - String threadName = threadFactory.newThread(new MyRunnable()).getName(); - - // It's pool-2 because pool-1 was created during emitter instantiation - Assert.assertEquals("snowplow-emitter-pool-2-request-thread-1", threadName); - } - @Test public void threadsHaveExpectedNames() { - // A checkForEventsToSend thread is created on BatchEmitter instantiation. - // Calling flushBuffer() here to require another thread - causing - // creation of a request thread within the scheduledThreadPool. + // Calling flushBuffer() here to create a request thread for event sending emitter.flushBuffer(); // Create a list of all live thread names @@ -220,9 +206,18 @@ public void threadsHaveExpectedNames() { for (Thread thread : threadList) { threadNames.add(thread.getName()); } + System.out.println(threadNames); + + // Because the threadpools are named by a static ThreadFactory, + // the pool number varies if this test is run in isolation or not + boolean matchResult = false; + for (String name : threadNames) { + if (Pattern.matches("snowplow-emitter-pool-\\d+-request-thread-1", name)) { + matchResult = true; + } + } - Assert.assertTrue(threadNames.contains("snowplow-emitter-checkForEvents-thread-1")); - Assert.assertTrue(threadNames.contains("snowplow-emitter-pool-1-request-thread-1")); + Assert.assertTrue(matchResult); } @Test @@ -231,6 +226,9 @@ public void close_sendsEventsAndStopsThreads() throws InterruptedException { for (TrackerPayload payload : payloads) { emitter.add(payload); } + System.out.println(emitter.getBuffer().size()); + Thread.sleep(500); + emitter.close(); Thread.sleep(500); From 9094799db2debdf14c7ba24b25c6ce153e7df73b Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Mon, 21 Feb 2022 19:01:46 +0000 Subject: [PATCH 08/20] Use scheduled request Runnable to add retry backoff time --- examples/simple-console/build.gradle | 4 +- .../main/java/com/snowplowanalytics/Main.java | 7 +- .../tracker/emitter/BatchEmitter.java | 72 ++++++++++--------- .../tracker/emitter/BatchPayload.java | 11 --- .../tracker/http/NetworkConnection.java | 20 ++++++ .../tracker/http/NetworkConnector.java | 22 ++++++ .../tracker/emitter/BatchEmitterTest.java | 49 ++++++++++--- 7 files changed, 125 insertions(+), 60 deletions(-) create mode 100644 src/main/java/com/snowplowanalytics/snowplow/tracker/http/NetworkConnection.java create mode 100644 src/main/java/com/snowplowanalytics/snowplow/tracker/http/NetworkConnector.java diff --git a/examples/simple-console/build.gradle b/examples/simple-console/build.gradle index 162b9659..97217503 100644 --- a/examples/simple-console/build.gradle +++ b/examples/simple-console/build.gradle @@ -16,9 +16,9 @@ test { } dependencies { - implementation 'com.snowplowanalytics:snowplow-java-tracker:0.10.1' + implementation 'com.snowplowanalytics:snowplow-java-tracker:0.+' - implementation ('com.snowplowanalytics:snowplow-java-tracker:0.10.1') { + implementation ('com.snowplowanalytics:snowplow-java-tracker:0.+') { capabilities { requireCapability 'com.snowplowanalytics:snowplow-java-tracker-okhttp-support' } diff --git a/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java b/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java index b886763c..6e139547 100644 --- a/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java +++ b/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java @@ -18,12 +18,14 @@ import com.snowplowanalytics.snowplow.tracker.Tracker; import com.snowplowanalytics.snowplow.tracker.emitter.BatchEmitter; import com.snowplowanalytics.snowplow.tracker.events.*; +import com.snowplowanalytics.snowplow.tracker.http.HttpClientAdapter; import com.snowplowanalytics.snowplow.tracker.payload.SelfDescribingJson; import java.util.List; import static java.util.Collections.singletonList; import com.google.common.collect.ImmutableMap; +import com.snowplowanalytics.snowplow.tracker.payload.TrackerPayload; public class Main { @@ -154,12 +156,11 @@ public static void main(String[] args) throws InterruptedException { .customContext(context) .build(); - Thread.sleep(30000); +// Thread.sleep(30000); System.out.println("About to track events"); - for (int i = 0; i < 150; i++) { // tracker.track(pageViewEvent); // the .track method schedules the event for delivery to Snowplow // tracker.track(ecommerceTransaction); // This will track two events @@ -175,7 +176,7 @@ public static void main(String[] args) throws InterruptedException { tracker.track(getPageView()); tracker.track(getPageView()); tracker.track(getPageView()); -// Thread.sleep(10); + Thread.sleep(10); } // Thread.sleep(5000); diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java index e55f5ac7..81756583 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java @@ -16,8 +16,9 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; import com.google.common.base.Preconditions; import com.snowplowanalytics.snowplow.tracker.constants.Constants; @@ -36,11 +37,11 @@ public class BatchEmitter extends AbstractEmitter implements Closeable { private static final Logger LOGGER = LoggerFactory.getLogger(BatchEmitter.class); -// private final Thread checkForEventsToSend; private boolean isClosing = false; - private int bufferSize = 1; + private int bufferSize; private final EventStore eventStore; + private final AtomicLong retryDelay; private final long closeTimeout = 5; @@ -87,12 +88,7 @@ protected BatchEmitter(final Builder builder) { this.bufferSize = builder.bufferSize; this.eventStore = builder.eventStore; - -// checkForEventsToSend = new Thread( -// getCheckForEventsToSendRunnable(), -// EVENTS_CHECK_THREAD_NAME_PREFIX + EVENTS_CHECK_THREAD_NUMBER.getAndIncrement() -// ); -// checkForEventsToSend.start(); + this.retryDelay = new AtomicLong(0); } /** @@ -106,7 +102,9 @@ public void add(final TrackerPayload payload) { if (!isClosing) { if (this.eventStore.getSize() >= getBufferSize()) { - execute(getPostRequestRunnable(getBufferSize())); + ScheduledExecutorService executor = (ScheduledExecutorService) this.executor; + executor.schedule(getPostRequestRunnable(getBufferSize()), this.retryDelay.get(), TimeUnit.MILLISECONDS); +// execute(getPostRequestRunnable(getBufferSize())); } } @@ -154,6 +152,10 @@ public int getBufferSize() { return this.bufferSize; } + public long getRetryDelay() { + return this.retryDelay.get(); + } + /** * Returns a Runnable POST Request operation * @@ -162,36 +164,36 @@ public int getBufferSize() { */ private Runnable getPostRequestRunnable(int numberOfEvents) { return () -> { - BatchPayload batchedEvents = eventStore.getEventBatch(numberOfEvents); - // BatchPayloads are allowed to retry 3 times before being considered failed - while (batchedEvents.getRemainingRetries() > 0) { - List eventsInRequest = batchedEvents.getPayload(); + BatchPayload batchedEvents = this.eventStore.getEventBatch(numberOfEvents); + List eventsInRequest = batchedEvents.getPayload(); - if (eventsInRequest.size() == 0) { - return; - } - final SelfDescribingJson post = getFinalPost(eventsInRequest); - final int code = httpClientAdapter.post(post); - - // Process results - if (isSuccessfulSend(code)) { - LOGGER.debug("BatchEmitter successfully sent {} events: code: {}", eventsInRequest.size(), code); - eventStore.cleanupAfterSendingAttempt(true, batchedEvents.getBatchId()); - break; - } else { - LOGGER.error("BatchEmitter failed to send {} events: code: {}", eventsInRequest.size(), code); - batchedEvents.subtractRetry(); - try { - Thread.sleep(100); - } catch (InterruptedException e) { - LOGGER.debug(e.getMessage()); - } - } + if (eventsInRequest.size() == 0) { + return; } - if (batchedEvents.getRemainingRetries() == 0) { + final SelfDescribingJson post = getFinalPost(eventsInRequest); + + final int code = httpClientAdapter.post(post); + + // Process results + if (isSuccessfulSend(code)) { + LOGGER.debug("BatchEmitter successfully sent {} events: code: {}", eventsInRequest.size(), code); + eventStore.cleanupAfterSendingAttempt(true, batchedEvents.getBatchId()); + System.out.println("success and delay is: " + getRetryDelay()); + this.retryDelay.set(0L); + } else { + LOGGER.error("BatchEmitter failed to send {} events: code: {}", eventsInRequest.size(), code); + System.out.println("failed and delay is: " + getRetryDelay()); eventStore.cleanupAfterSendingAttempt(false, batchedEvents.getBatchId()); + + long currentDelay = this.retryDelay.get(); + if (currentDelay == 0) { + this.retryDelay.compareAndSet(0, 50); + } else { + this.retryDelay.set(currentDelay * 2); + } } + }; } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchPayload.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchPayload.java index b56c79c3..722c08f7 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchPayload.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchPayload.java @@ -21,17 +21,6 @@ public class BatchPayload { private final Long batchId; private final List payloads; - private int remainingRetries = 3; - - public int getRemainingRetries() { - return remainingRetries; - } - - public void subtractRetry() { - this.remainingRetries--; - } - - public BatchPayload(Long payloadId, List payloads) { this.batchId = payloadId; diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/http/NetworkConnection.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/http/NetworkConnection.java new file mode 100644 index 00000000..75df23ec --- /dev/null +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/http/NetworkConnection.java @@ -0,0 +1,20 @@ +/* + * Copyright (c) 2014-2021 Snowplow Analytics Ltd. All rights reserved. + * + * This program is licensed to you under the Apache License Version 2.0, + * and you may not use this file except in compliance with the Apache License Version 2.0. + * You may obtain a copy of the Apache License Version 2.0 at http://www.apache.org/licenses/LICENSE-2.0. + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the Apache License Version 2.0 is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the Apache License Version 2.0 for the specific language governing permissions and limitations there under. + */ +package com.snowplowanalytics.snowplow.tracker.http; + +import com.snowplowanalytics.snowplow.tracker.payload.Payload; + +public interface NetworkConnection { + + int sendRequest(Payload payload); +} diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/http/NetworkConnector.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/http/NetworkConnector.java new file mode 100644 index 00000000..e38ebfa0 --- /dev/null +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/http/NetworkConnector.java @@ -0,0 +1,22 @@ +/* + * Copyright (c) 2014-2021 Snowplow Analytics Ltd. All rights reserved. + * + * This program is licensed to you under the Apache License Version 2.0, + * and you may not use this file except in compliance with the Apache License Version 2.0. + * You may obtain a copy of the Apache License Version 2.0 at http://www.apache.org/licenses/LICENSE-2.0. + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the Apache License Version 2.0 is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the Apache License Version 2.0 for the specific language governing permissions and limitations there under. + */ +package com.snowplowanalytics.snowplow.tracker.http; + +import com.snowplowanalytics.snowplow.tracker.payload.Payload; + +public class NetworkConnector implements NetworkConnection{ + @Override + public int sendRequest(Payload payload) { + return 200; + } +} diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java index 08fe671f..0795dd0b 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java @@ -32,11 +32,11 @@ import com.snowplowanalytics.snowplow.tracker.payload.TrackerPayload; import com.snowplowanalytics.snowplow.tracker.events.PageView; import com.snowplowanalytics.snowplow.tracker.http.HttpClientAdapter; -import org.junit.jupiter.api.BeforeEach; public class BatchEmitterTest { private MockHttpClientAdapter mockHttpClientAdapter; + private FailingHttpClientAdapter failingHttpClientAdapter; private BatchEmitter emitter; public static class MockHttpClientAdapter implements HttpClientAdapter { @@ -71,10 +71,16 @@ public Object getHttpClient() { } public static class FailingHttpClientAdapter implements HttpClientAdapter { - public int postCounter = 0; + public int failedPostCounter = 0; + public int successfulPostCounter = 0; @Override public int post(SelfDescribingJson payload) { - postCounter++; + if (failedPostCounter >= 4) { + successfulPostCounter++; + return 200; + } + + failedPostCounter++; return 500; } @@ -97,6 +103,7 @@ public Object getHttpClient() { @Before public void setUp() { mockHttpClientAdapter = new MockHttpClientAdapter(); + failingHttpClientAdapter = new FailingHttpClientAdapter(); emitter = BatchEmitter.builder() .httpClientAdapter(mockHttpClientAdapter) .bufferSize(10) @@ -206,7 +213,6 @@ public void threadsHaveExpectedNames() { for (Thread thread : threadList) { threadNames.add(thread.getName()); } - System.out.println(threadNames); // Because the threadpools are named by a static ThreadFactory, // the pool number varies if this test is run in isolation or not @@ -271,22 +277,47 @@ public void eventsThatFailToSendAreReturnedToEventBuffer() throws InterruptedExc } @Test - public void eventsThatFailToSendAreRetried() throws InterruptedException { - FailingHttpClientAdapter failingHttpClientAdapter = new FailingHttpClientAdapter(); + public void eventSendingFailureIncreasesBackoffTime() throws InterruptedException { emitter = BatchEmitter.builder() .httpClientAdapter(failingHttpClientAdapter) - .bufferSize(10) + .bufferSize(1) .build(); List payloads = createPayloads(2); for (TrackerPayload payload : payloads) { emitter.add(payload); } - emitter.flushBuffer(); Thread.sleep(500); - Assert.assertEquals(3, failingHttpClientAdapter.postCounter); + Assert.assertEquals(100, emitter.getRetryDelay()); + } + + @Test + public void successfulSendAfterFailureResetsBackoffTime() throws InterruptedException { + // the FailingHttpClientAdapter returns 500 for the first 4 requests + // then subsequently returns 200 + FailingHttpClientAdapter failingHttpClientAdapter = new FailingHttpClientAdapter(); + emitter = BatchEmitter.builder() + .httpClientAdapter(failingHttpClientAdapter) + .bufferSize(1) + .build(); + + // these requests will pass the failingHttpClientAdapter threshold for failing + // because they are being processed asynchronously, the retryDelay might + // not be reset as expected + List payloads = createPayloads(6); + for (TrackerPayload payload : payloads) { + emitter.add(payload); + } + + Thread.sleep(500); + + // this request will definitely succeed + emitter.add(createPayload()); + Thread.sleep(500); + Assert.assertEquals(3, failingHttpClientAdapter.successfulPostCounter); + Assert.assertEquals(0, emitter.getRetryDelay()); } private TrackerPayload createPayload() { From 26cfa0f71992377f5079ada4e3ac9c4dc53cfe62 Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Tue, 22 Feb 2022 12:08:57 +0000 Subject: [PATCH 09/20] Enclose Emitter request method in try catch block --- .../tracker/emitter/AbstractEmitter.java | 21 ++----- .../tracker/emitter/BatchEmitter.java | 61 ++++++++++--------- .../tracker/emitter/BatchPayload.java | 2 +- .../tracker/emitter/InMemoryEventStore.java | 2 +- .../tracker/http/NetworkConnection.java | 20 ------ .../tracker/http/NetworkConnector.java | 22 ------- .../tracker/emitter/BatchEmitterTest.java | 4 +- .../emitter/InMemoryEventStoreTest.java | 4 +- 8 files changed, 43 insertions(+), 93 deletions(-) delete mode 100644 src/main/java/com/snowplowanalytics/snowplow/tracker/http/NetworkConnection.java delete mode 100644 src/main/java/com/snowplowanalytics/snowplow/tracker/http/NetworkConnector.java diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java index 600c1d58..155323f4 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java @@ -15,6 +15,7 @@ import java.util.List; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicInteger; @@ -23,7 +24,6 @@ import com.snowplowanalytics.snowplow.tracker.http.HttpClientAdapter; import com.snowplowanalytics.snowplow.tracker.http.OkHttpClientAdapter; -import com.snowplowanalytics.snowplow.tracker.payload.Payload; import com.snowplowanalytics.snowplow.tracker.payload.TrackerPayload; import okhttp3.OkHttpClient; @@ -34,24 +34,24 @@ public abstract class AbstractEmitter implements Emitter { protected HttpClientAdapter httpClientAdapter; - protected ExecutorService executor; + protected ScheduledExecutorService executor; public static abstract class Builder> { private HttpClientAdapter httpClientAdapter; // Optional private int threadCount = 50; // Optional - private ExecutorService requestExecutorService = null; // Optional + private ScheduledExecutorService requestExecutorService = null; // Optional private String collectorUrl = null; // Required if not specifying a httpClientAdapter protected abstract T self(); /** - * Set a custom ExecutorService to send http request. + * Set a custom ScheduledExecutorService to send http request. * * /!\ Be aware that calling `close()` on a BatchEmitter instance has a side-effect and will shutdown that ExecutorService. - * @param executorService the ExecutorService to use + * @param executorService the ScheduledExecutorService to use * @return itself */ - public T requestExecutorService(final ExecutorService executorService) { + public T requestExecutorService(final ScheduledExecutorService executorService) { this.requestExecutorService = executorService; return self(); } @@ -165,15 +165,6 @@ protected AbstractEmitter(final Builder builder) { @Override public abstract List getBuffer(); - /** - * Sends a runnable to the executor service. - * - * @param runnable the runnable to be queued - */ - protected void execute(final Runnable runnable) { - this.executor.execute(runnable); - } - /** * Checks whether the response code was a success or not. * diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java index 81756583..8d1a9612 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java @@ -16,7 +16,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; @@ -101,10 +100,8 @@ public void add(final TrackerPayload payload) { boolean result = this.eventStore.addEvent(payload); if (!isClosing) { - if (this.eventStore.getSize() >= getBufferSize()) { - ScheduledExecutorService executor = (ScheduledExecutorService) this.executor; - executor.schedule(getPostRequestRunnable(getBufferSize()), this.retryDelay.get(), TimeUnit.MILLISECONDS); -// execute(getPostRequestRunnable(getBufferSize())); + if (this.eventStore.getSize() >= this.bufferSize) { + executor.schedule(getPostRequestRunnable(this.bufferSize), this.retryDelay.get(), TimeUnit.MILLISECONDS); } } @@ -118,7 +115,7 @@ public void add(final TrackerPayload payload) { */ @Override public void flushBuffer() { - execute(getPostRequestRunnable(this.eventStore.getSize())); + executor.schedule(getPostRequestRunnable(this.eventStore.getSize()), 0, TimeUnit.MILLISECONDS); } /** @@ -164,36 +161,40 @@ public long getRetryDelay() { */ private Runnable getPostRequestRunnable(int numberOfEvents) { return () -> { - BatchPayload batchedEvents = this.eventStore.getEventBatch(numberOfEvents); - List eventsInRequest = batchedEvents.getPayload(); - - if (eventsInRequest.size() == 0) { - return; - } - - final SelfDescribingJson post = getFinalPost(eventsInRequest); + BatchPayload batchedEvents = null; + try { + batchedEvents = this.eventStore.getEventBatch(numberOfEvents); + List eventsInRequest = batchedEvents.getPayloads(); - final int code = httpClientAdapter.post(post); + if (eventsInRequest.size() == 0) { + return; + } - // Process results - if (isSuccessfulSend(code)) { - LOGGER.debug("BatchEmitter successfully sent {} events: code: {}", eventsInRequest.size(), code); - eventStore.cleanupAfterSendingAttempt(true, batchedEvents.getBatchId()); - System.out.println("success and delay is: " + getRetryDelay()); - this.retryDelay.set(0L); - } else { - LOGGER.error("BatchEmitter failed to send {} events: code: {}", eventsInRequest.size(), code); - System.out.println("failed and delay is: " + getRetryDelay()); - eventStore.cleanupAfterSendingAttempt(false, batchedEvents.getBatchId()); + final SelfDescribingJson post = getFinalPost(eventsInRequest); + final int code = httpClientAdapter.post(post); - long currentDelay = this.retryDelay.get(); - if (currentDelay == 0) { - this.retryDelay.compareAndSet(0, 50); + // Process results + if (isSuccessfulSend(code)) { + LOGGER.debug("BatchEmitter successfully sent {} events: code: {}", eventsInRequest.size(), code); + this.retryDelay.set(0L); + eventStore.cleanupAfterSendingAttempt(true, batchedEvents.getBatchId()); } else { - this.retryDelay.set(currentDelay * 2); + LOGGER.error("BatchEmitter failed to send {} events: code: {}", eventsInRequest.size(), code); + eventStore.cleanupAfterSendingAttempt(false, batchedEvents.getBatchId()); + + long currentDelay = this.retryDelay.get(); + if (currentDelay == 0) { + this.retryDelay.compareAndSet(0, 50L); + } else { + this.retryDelay.set(currentDelay * 2); + } + } + } catch (Exception e) { + LOGGER.error("BatchEmitter event sending error: {}", e.getMessage()); + if (batchedEvents != null) { + eventStore.cleanupAfterSendingAttempt(false, batchedEvents.getBatchId()); } } - }; } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchPayload.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchPayload.java index 722c08f7..f561e1aa 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchPayload.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchPayload.java @@ -31,7 +31,7 @@ public Long getBatchId() { return batchId; } - public List getPayload() { + public List getPayloads() { return payloads; } } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java index 975cb89f..8a3e5774 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java @@ -28,7 +28,7 @@ public BatchPayload getEventBatch(int numberToGet) { // The batch of events is wrapped as a BatchPayload // They're also added to the "pending" event buffer, the eventsBeingSent HashMap BatchPayload batchedEvents = new BatchPayload(batchId.getAndIncrement(), eventsToSend); - eventsBeingSent.put(batchedEvents.getBatchId(), batchedEvents.getPayload()); + eventsBeingSent.put(batchedEvents.getBatchId(), batchedEvents.getPayloads()); return batchedEvents; } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/http/NetworkConnection.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/http/NetworkConnection.java deleted file mode 100644 index 75df23ec..00000000 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/http/NetworkConnection.java +++ /dev/null @@ -1,20 +0,0 @@ -/* - * Copyright (c) 2014-2021 Snowplow Analytics Ltd. All rights reserved. - * - * This program is licensed to you under the Apache License Version 2.0, - * and you may not use this file except in compliance with the Apache License Version 2.0. - * You may obtain a copy of the Apache License Version 2.0 at http://www.apache.org/licenses/LICENSE-2.0. - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the Apache License Version 2.0 is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the Apache License Version 2.0 for the specific language governing permissions and limitations there under. - */ -package com.snowplowanalytics.snowplow.tracker.http; - -import com.snowplowanalytics.snowplow.tracker.payload.Payload; - -public interface NetworkConnection { - - int sendRequest(Payload payload); -} diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/http/NetworkConnector.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/http/NetworkConnector.java deleted file mode 100644 index e38ebfa0..00000000 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/http/NetworkConnector.java +++ /dev/null @@ -1,22 +0,0 @@ -/* - * Copyright (c) 2014-2021 Snowplow Analytics Ltd. All rights reserved. - * - * This program is licensed to you under the Apache License Version 2.0, - * and you may not use this file except in compliance with the Apache License Version 2.0. - * You may obtain a copy of the Apache License Version 2.0 at http://www.apache.org/licenses/LICENSE-2.0. - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the Apache License Version 2.0 is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the Apache License Version 2.0 for the specific language governing permissions and limitations there under. - */ -package com.snowplowanalytics.snowplow.tracker.http; - -import com.snowplowanalytics.snowplow.tracker.payload.Payload; - -public class NetworkConnector implements NetworkConnection{ - @Override - public int sendRequest(Payload payload) { - return 200; - } -} diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java index 0795dd0b..f55d3d4d 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java @@ -303,8 +303,8 @@ public void successfulSendAfterFailureResetsBackoffTime() throws InterruptedExce .build(); // these requests will pass the failingHttpClientAdapter threshold for failing - // because they are being processed asynchronously, the retryDelay might - // not be reset as expected + // but because they are being processed asynchronously, the successful requests + // may be processed by the Emitter before the failed ones List payloads = createPayloads(6); for (TrackerPayload payload : payloads) { emitter.add(payload); diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java index 327873da..415c3b5b 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java @@ -53,7 +53,7 @@ public void getEventsFromStorage() { eventStore.addEvent(trackerPayload); eventStore.addEvent(trackerPayload); - Assert.assertEquals(2, eventStore.getEventBatch(2).getPayload().size()); + Assert.assertEquals(2, eventStore.getEventBatch(2).getPayloads().size()); Assert.assertEquals(2, eventStore.getSize()); } @@ -62,7 +62,7 @@ public void getAllEventsIfAskedForMoreEventsThanAreStored() { eventStore.addEvent(trackerPayload); eventStore.addEvent(trackerPayload); - List events = eventStore.getEventBatch(3).getPayload(); + List events = eventStore.getEventBatch(3).getPayloads(); System.out.println(events); Assert.assertEquals(2, events.size()); From 2242726af18ffa687a6882e2fa9d532990ec66e0 Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Tue, 22 Feb 2022 17:28:45 +0000 Subject: [PATCH 10/20] Allow event buffer max capacity to be configured --- examples/simple-console/build.gradle | 4 ++-- .../tracker/emitter/BatchEmitter.java | 23 ++++++++++++++++--- .../tracker/emitter/InMemoryEventStore.java | 22 +++++++++++++++--- .../tracker/emitter/BatchEmitterTest.java | 15 ++++++++++++ .../emitter/InMemoryEventStoreTest.java | 20 +++++++++++++++- 5 files changed, 75 insertions(+), 9 deletions(-) diff --git a/examples/simple-console/build.gradle b/examples/simple-console/build.gradle index 97217503..162b9659 100644 --- a/examples/simple-console/build.gradle +++ b/examples/simple-console/build.gradle @@ -16,9 +16,9 @@ test { } dependencies { - implementation 'com.snowplowanalytics:snowplow-java-tracker:0.+' + implementation 'com.snowplowanalytics:snowplow-java-tracker:0.10.1' - implementation ('com.snowplowanalytics:snowplow-java-tracker:0.+') { + implementation ('com.snowplowanalytics:snowplow-java-tracker:0.10.1') { capabilities { requireCapability 'com.snowplowanalytics:snowplow-java-tracker-okhttp-support' } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java index 8d1a9612..3d936970 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java @@ -16,6 +16,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; @@ -39,6 +40,7 @@ public class BatchEmitter extends AbstractEmitter implements Closeable { private boolean isClosing = false; private int bufferSize; + private int bufferCapacity; private final EventStore eventStore; private final AtomicLong retryDelay; @@ -47,7 +49,8 @@ public class BatchEmitter extends AbstractEmitter implements Closeable { public static abstract class Builder> extends AbstractEmitter.Builder { private int bufferSize = 50; // Optional - private EventStore eventStore = new InMemoryEventStore(); + private int bufferCapacity = Integer.MAX_VALUE; + private EventStore eventStore; /** * @param bufferSize The count of events to buffer before sending @@ -63,6 +66,11 @@ public T eventStore(final EventStore eventStore) { return self(); } + public T bufferCapacity(final int bufferCapacity) { + this.bufferCapacity = bufferCapacity; + return self(); + } + public BatchEmitter build() { return new BatchEmitter(this); } @@ -86,7 +94,15 @@ protected BatchEmitter(final Builder builder) { Preconditions.checkArgument(builder.bufferSize > 0, "bufferSize must be greater than 0"); this.bufferSize = builder.bufferSize; - this.eventStore = builder.eventStore; + this.bufferCapacity = builder.bufferCapacity; +// this.eventStore = builder.eventStore; + + if (builder.eventStore == null) { + this.eventStore = new InMemoryEventStore(this.bufferCapacity); + } else { + this.eventStore = builder.eventStore; + } + this.retryDelay = new AtomicLong(0); } @@ -106,7 +122,7 @@ public void add(final TrackerPayload payload) { } if (!result) { - LOGGER.error("Unable to addEvent payload to emitter, emitter buffer is full"); + LOGGER.error("Unable to add payload to emitter, emitter buffer is full"); } } @@ -182,6 +198,7 @@ private Runnable getPostRequestRunnable(int numberOfEvents) { LOGGER.error("BatchEmitter failed to send {} events: code: {}", eventsInRequest.size(), code); eventStore.cleanupAfterSendingAttempt(false, batchedEvents.getBatchId()); + // exponentially increase retry backoff time after the first failure long currentDelay = this.retryDelay.get(); if (currentDelay == 0) { this.retryDelay.compareAndSet(0, 50L); diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java index 8a3e5774..54c92d47 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java @@ -1,6 +1,8 @@ package com.snowplowanalytics.snowplow.tracker.emitter; import com.snowplowanalytics.snowplow.tracker.payload.TrackerPayload; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.*; import java.util.concurrent.ConcurrentHashMap; @@ -8,11 +10,19 @@ import java.util.concurrent.atomic.AtomicLong; public class InMemoryEventStore implements EventStore { + private static final Logger LOGGER = LoggerFactory.getLogger(InMemoryEventStore.class); private final AtomicLong batchId = new AtomicLong(1); - public final LinkedBlockingDeque eventBuffer = new LinkedBlockingDeque<>(); + public final LinkedBlockingDeque eventBuffer; public final ConcurrentHashMap> eventsBeingSent = new ConcurrentHashMap<>(); + public InMemoryEventStore() { + eventBuffer = new LinkedBlockingDeque<>(); + } + + public InMemoryEventStore(int bufferCapacity) { + eventBuffer = new LinkedBlockingDeque<>(bufferCapacity); + } @Override public boolean addEvent(TrackerPayload trackerPayload) { @@ -40,8 +50,14 @@ public void cleanupAfterSendingAttempt(boolean successfullySent, long batchId) { // Events that didn't send are inserted at the head of the eventBuffer // for immediate resending. if (!successfullySent) { - for (TrackerPayload event : events) { - eventBuffer.addFirst(event); + while (events.size() > 0) { + TrackerPayload payloadToReinsert = events.remove(0); + boolean result = eventBuffer.offerFirst(payloadToReinsert); + if (!result) { + LOGGER.error("Event buffer is full. Dropping newer payload to reinsert older payload"); + eventBuffer.removeLast(); + eventBuffer.offerFirst(payloadToReinsert); + } } } } diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java index f55d3d4d..eebf5b0f 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java @@ -144,6 +144,21 @@ public void addToBuffer_withMore10Payloads_shouldEmptyBuffer() throws Interrupte Assert.assertEquals(1, mockHttpClientAdapter.postCounter); } + @Test + public void addToBuffer_doesNotAddEventIfBufferFull() { + emitter = BatchEmitter.builder() + .httpClientAdapter(mockHttpClientAdapter) + .bufferCapacity(1) + .build(); + + emitter.add(createPayload()); + + TrackerPayload differentPayload = createPayload(); + emitter.add(differentPayload); + + Assert.assertFalse(emitter.getBuffer().contains(differentPayload)); + } + @Test public void flushBuffer_shouldEmptyBuffer() throws InterruptedException { List payloads = createPayloads(2); diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java index 415c3b5b..0a15b8c7 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java @@ -63,7 +63,6 @@ public void getAllEventsIfAskedForMoreEventsThanAreStored() { eventStore.addEvent(trackerPayload); List events = eventStore.getEventBatch(3).getPayloads(); - System.out.println(events); Assert.assertEquals(2, events.size()); } @@ -94,6 +93,25 @@ public void doNotPutEventsBackInBufferIfSent() { Assert.assertEquals(0, eventStore.getSize()); } + @Test + public void dropEventsOnFailureWhenBufferFull() { + eventStore = new InMemoryEventStore(3); + + TrackerPayload differentPayload = createTrackerPayload(); + + eventStore.addEvent(differentPayload); + eventStore.getEventBatch(1); + + eventStore.addEvent(trackerPayload); + eventStore.addEvent(trackerPayload); + eventStore.addEvent(trackerPayload); + + eventStore.cleanupAfterSendingAttempt(false, 1L); + Assert.assertEquals(3, eventStore.getSize()); + Assert.assertTrue(eventStore.getAllEvents().contains(differentPayload)); + + } + private TrackerPayload createTrackerPayload() { PageView pv = PageView.builder() .pageUrl("https://www.snowplowanalytics.com/") From 93903459d9b54f54529fa0cbbae84fc5ea8647f2 Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Wed, 23 Feb 2022 11:10:16 +0000 Subject: [PATCH 11/20] Fix simple-console demo --- build.gradle | 2 +- examples/simple-console/build.gradle | 4 +- .../main/java/com/snowplowanalytics/Main.java | 37 ++++++------------- .../snowplow/tracker/TrackerTest.java | 2 +- 4 files changed, 16 insertions(+), 29 deletions(-) diff --git a/build.gradle b/build.gradle index 52310b54..639091a3 100644 --- a/build.gradle +++ b/build.gradle @@ -25,7 +25,7 @@ wrapper.gradleVersion = '6.5.0' group = 'com.snowplowanalytics' archivesBaseName = 'snowplow-java-tracker' -version = '0.12.0-alpha.0' +version = '0.12.0-alpha.1' sourceCompatibility = '1.8' targetCompatibility = '1.8' diff --git a/examples/simple-console/build.gradle b/examples/simple-console/build.gradle index 162b9659..97217503 100644 --- a/examples/simple-console/build.gradle +++ b/examples/simple-console/build.gradle @@ -16,9 +16,9 @@ test { } dependencies { - implementation 'com.snowplowanalytics:snowplow-java-tracker:0.10.1' + implementation 'com.snowplowanalytics:snowplow-java-tracker:0.+' - implementation ('com.snowplowanalytics:snowplow-java-tracker:0.10.1') { + implementation ('com.snowplowanalytics:snowplow-java-tracker:0.+') { capabilities { requireCapability 'com.snowplowanalytics:snowplow-java-tracker-okhttp-support' } diff --git a/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java b/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java index 6e139547..67aa343a 100644 --- a/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java +++ b/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java @@ -58,6 +58,12 @@ public static void main(String[] args) throws InterruptedException { .bufferSize(4) // send batches of 4 events. In production this number should be higher, depending on the size/event volume .build(); + // for versions before 0.12.0, the property batchSize was called bufferSize +// BatchEmitter emitter = BatchEmitter.builder() +// .url(collectorEndpoint) +// .bufferSize(4) +// .build(); + // now we have the emitter, we need a tracker to turn our events into something a Snowplow collector can understand final Tracker tracker = new Tracker.TrackerBuilder(emitter, namespace, appId) .base64(true) @@ -156,31 +162,12 @@ public static void main(String[] args) throws InterruptedException { .customContext(context) .build(); -// Thread.sleep(30000); - - System.out.println("About to track events"); - - - for (int i = 0; i < 150; i++) { -// tracker.track(pageViewEvent); // the .track method schedules the event for delivery to Snowplow -// tracker.track(ecommerceTransaction); // This will track two events -// tracker.track(unstructured); -// tracker.track(screenView); -// tracker.track(timing); -// tracker.track(structured); - - tracker.track(getPageView()); - tracker.track(getPageView()); - tracker.track(getPageView()); - tracker.track(getPageView()); - tracker.track(getPageView()); - tracker.track(getPageView()); - tracker.track(getPageView()); - Thread.sleep(10); - } - -// Thread.sleep(5000); - + tracker.track(pageViewEvent); // the .track method schedules the event for delivery to Snowplow + tracker.track(ecommerceTransaction); // This will track two events + tracker.track(unstructured); + tracker.track(screenView); + tracker.track(timing); + tracker.track(structured); // Will close all threads and force send remaining events emitter.close(); diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java index 72b53aa7..4d05356c 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java @@ -525,7 +525,7 @@ public void testTrackTimingWithSubject() throws InterruptedException { @Test public void testGetTrackerVersion() { Tracker tracker = new Tracker.TrackerBuilder(mockEmitter, "namespace", "an-app-id").build(); - assertEquals("java-0.12.0-alpha.0", tracker.getTrackerVersion()); + assertEquals("java-0.12.0-alpha.1", tracker.getTrackerVersion()); } @Test From 4ae3d40993b7a6dfc5119bdfae03dd8c68a8c21b Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Wed, 23 Feb 2022 14:56:26 +0000 Subject: [PATCH 12/20] Remove unused benchmark class --- examples/benchmarking/build.gradle | 4 - .../snowplowanalytics/BenchmarkHelpers.java | 48 ------ .../InMemoryEventStoreBenchmark.java | 142 ------------------ .../snowplowanalytics/TrackerBenchmark.java | 3 - .../snowplow/tracker/Subject.java | 2 +- .../snowplow/tracker/Tracker.java | 2 +- .../snowplow/tracker/Utils.java | 2 +- .../tracker/emitter/BatchEmitter.java | 2 +- .../tracker/events/AbstractEvent.java | 2 +- .../snowplow/tracker/payload/Payload.java | 2 +- .../tracker/payload/SelfDescribingJson.java | 6 +- .../tracker/payload/TrackerPayload.java | 2 +- 12 files changed, 10 insertions(+), 207 deletions(-) delete mode 100644 examples/benchmarking/src/jmh/java/com/snowplowanalytics/BenchmarkHelpers.java delete mode 100644 examples/benchmarking/src/jmh/java/com/snowplowanalytics/InMemoryEventStoreBenchmark.java diff --git a/examples/benchmarking/build.gradle b/examples/benchmarking/build.gradle index 9c9dd4b9..e00d4ad9 100644 --- a/examples/benchmarking/build.gradle +++ b/examples/benchmarking/build.gradle @@ -34,7 +34,3 @@ repositories { dependencies { jmh 'com.snowplowanalytics:snowplow-java-tracker:0.10.1' } - -jmh { - includes = ['InMemoryEventStore*'] -} diff --git a/examples/benchmarking/src/jmh/java/com/snowplowanalytics/BenchmarkHelpers.java b/examples/benchmarking/src/jmh/java/com/snowplowanalytics/BenchmarkHelpers.java deleted file mode 100644 index 2d7a6579..00000000 --- a/examples/benchmarking/src/jmh/java/com/snowplowanalytics/BenchmarkHelpers.java +++ /dev/null @@ -1,48 +0,0 @@ -package com.snowplowanalytics; - -import com.snowplowanalytics.snowplow.tracker.Tracker; -import com.snowplowanalytics.snowplow.tracker.emitter.BatchEmitter; -import com.snowplowanalytics.snowplow.tracker.events.PageView; -import com.snowplowanalytics.snowplow.tracker.http.HttpClientAdapter; -import com.snowplowanalytics.snowplow.tracker.payload.SelfDescribingJson; -import com.snowplowanalytics.snowplow.tracker.payload.TrackerPayload; - -public class BenchmarkHelpers { - static PageView event = PageView.builder() - .pageUrl("https://www.snowplowanalytics.com/") - .pageTitle("Snowplow") - .build(); - - static TrackerPayload payload = event.getPayload(); - - public static class MockHttpClientAdapter implements HttpClientAdapter { - @Override - public int post(SelfDescribingJson payload) { - return 200; - } - - @Override - public int get(TrackerPayload payload) { - return 0; - } - - @Override - public String getUrl() { - return null; - } - - @Override - public Object getHttpClient() { - return null; - } - } - - static MockHttpClientAdapter mockHttpClientAdapter = new MockHttpClientAdapter(); - - static BatchEmitter emitter = BatchEmitter.builder() - .httpClientAdapter(mockHttpClientAdapter) - .bufferSize(5) - .build(); - - static Tracker tracker = new Tracker.TrackerBuilder(emitter, "namespace", "appId").build(); -} diff --git a/examples/benchmarking/src/jmh/java/com/snowplowanalytics/InMemoryEventStoreBenchmark.java b/examples/benchmarking/src/jmh/java/com/snowplowanalytics/InMemoryEventStoreBenchmark.java deleted file mode 100644 index a492eb72..00000000 --- a/examples/benchmarking/src/jmh/java/com/snowplowanalytics/InMemoryEventStoreBenchmark.java +++ /dev/null @@ -1,142 +0,0 @@ -/* - * Copyright (c) 2014-2021 Snowplow Analytics Ltd. All rights reserved. - * - * This program is licensed to you under the Apache License Version 2.0, - * and you may not use this file except in compliance with the Apache License Version 2.0. - * You may obtain a copy of the Apache License Version 2.0 at http://www.apache.org/licenses/LICENSE-2.0. - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the Apache License Version 2.0 is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the Apache License Version 2.0 for the specific language governing permissions and limitations there under. - */ -package com.snowplowanalytics; - -import com.snowplowanalytics.snowplow.tracker.events.PageView; -import com.snowplowanalytics.snowplow.tracker.payload.TrackerPayload; -import org.openjdk.jmh.annotations.*; -import org.openjdk.jmh.infra.Blackhole; - -import java.util.ArrayList; -import java.util.List; -import java.util.concurrent.*; - -@State(Scope.Thread) -@BenchmarkMode({Mode.AverageTime, Mode.Throughput}) -@OutputTimeUnit(TimeUnit.NANOSECONDS) -@Warmup(iterations = 10, time = 100, timeUnit = TimeUnit.MILLISECONDS) -@Measurement(iterations = 20, time = 100, timeUnit = TimeUnit.MILLISECONDS) -@Fork(3) -public class InMemoryEventStoreBenchmark { - - static TrackerPayload payload = BenchmarkHelpers.event.getPayload(); - - static List payloads = getTenPayloads(); - - public static List getTenPayloads() { - ArrayList payloads = new ArrayList<>(); - payloads.add(payload); - return payloads; - } - - - @State(Scope.Benchmark) - public static class BufferComponents { - ConcurrentLinkedDeque ConcurrentLinkedDeque; - ConcurrentLinkedQueue ConcurrentLinkedQueue; - LinkedBlockingDeque LinkedBlockingDeque; - LinkedBlockingQueue LinkedBlockingQueue; - List events; - - @Setup(Level.Iteration) - public void doSetUp() { - ConcurrentLinkedDeque = new ConcurrentLinkedDeque<>(); - ConcurrentLinkedQueue = new ConcurrentLinkedQueue<>(); - LinkedBlockingDeque = new LinkedBlockingDeque<>(); - LinkedBlockingQueue = new LinkedBlockingQueue<>(); - - ConcurrentLinkedDeque.addAll(payloads); - ConcurrentLinkedQueue.addAll(payloads); - LinkedBlockingDeque.addAll(payloads); - LinkedBlockingQueue.addAll(payloads); - - events = new ArrayList<>(); - } - } - -// @Benchmark -// public List testGetFromConcurrentLinkedDeque(BufferComponents bufferComponents) { -// for (int i = 0; i < 5; i++) { -// TrackerPayload payload = bufferComponents.ConcurrentLinkedDeque.poll(); -// if (payload == null) { -// break; -// } -// bufferComponents.events.add(payload); -// } -// return bufferComponents.events; -// } -// -// @Benchmark -// public List testGetFromConcurrentLinkedQueue(BufferComponents bufferComponents) { -// for (int i = 0; i < 5; i++) { -// TrackerPayload payload = bufferComponents.ConcurrentLinkedQueue.poll(); -// if (payload == null) { -// break; -// } -// bufferComponents.events.add(payload); -// } -// return bufferComponents.events; -// } -// -// @Benchmark -// public List testGetFromLinkedBlockingQueue(BufferComponents bufferComponents) { -// bufferComponents.LinkedBlockingQueue.drainTo(bufferComponents.events, 5); -// return bufferComponents.events; -// } -// -// @Benchmark -// public List testGetFromLinkedBlockingDeque(BufferComponents bufferComponents) { -// bufferComponents.LinkedBlockingDeque.drainTo(bufferComponents.events, 5); -// return bufferComponents.events; -// } - - @Benchmark - public void testInsertLinkedBlockingDequeTail(BufferComponents bufferComponents, Blackhole blackhole) { - bufferComponents.LinkedBlockingDeque.addAll(payloads); - blackhole.consume(bufferComponents); - } - - @Benchmark - public void testInsertLinkedBlockingQueueTail(BufferComponents bufferComponents, Blackhole blackhole) { - bufferComponents.LinkedBlockingQueue.addAll(payloads); - blackhole.consume(bufferComponents); - } - - @Benchmark - public void testInsertConcurrentLinkedQueueTail(BufferComponents bufferComponents, Blackhole blackhole) { - bufferComponents.ConcurrentLinkedQueue.addAll(payloads); - blackhole.consume(bufferComponents); - } - - @Benchmark - public void testInsertConcurrentLinkedDequeTail(BufferComponents bufferComponents, Blackhole blackhole) { - bufferComponents.ConcurrentLinkedDeque.addAll(payloads); - blackhole.consume(bufferComponents); - } - - @Benchmark - public void testInsertConcurrentLinkedDequeHead(BufferComponents bufferComponents, Blackhole blackhole) { - for (TrackerPayload payload : payloads) { - bufferComponents.ConcurrentLinkedDeque.addFirst(payload); - } - blackhole.consume(bufferComponents); - } - - @Benchmark - public void testInsertLinkedBlockingDequeHead(BufferComponents bufferComponents, Blackhole blackhole) { - for (TrackerPayload payload : payloads) { - bufferComponents.LinkedBlockingDeque.addFirst(payload); - } - blackhole.consume(bufferComponents); - } -} diff --git a/examples/benchmarking/src/jmh/java/com/snowplowanalytics/TrackerBenchmark.java b/examples/benchmarking/src/jmh/java/com/snowplowanalytics/TrackerBenchmark.java index 0683c9af..c51b900f 100644 --- a/examples/benchmarking/src/jmh/java/com/snowplowanalytics/TrackerBenchmark.java +++ b/examples/benchmarking/src/jmh/java/com/snowplowanalytics/TrackerBenchmark.java @@ -66,9 +66,6 @@ public static Tracker getTracker(Emitter emitter) { } public static void closeThreads(Tracker tracker) { - // Use this line for versions 0.12.0 onwards -// tracker.close(); - // Use these lines for previous versions BatchEmitter emitter = (BatchEmitter) tracker.getEmitter(); emitter.close(); } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/Subject.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/Subject.java index 3cd7b315..c61958e8 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/Subject.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/Subject.java @@ -27,7 +27,7 @@ public class Subject { private HashMap standardPairs = new HashMap<>(); /** - * Creates a Subject which will addEvent extra data to each event. + * Creates a Subject which will add extra data to each event. * * @param builder The builder that constructs a subject */ diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java index 39970b8f..8079ab9b 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java @@ -266,7 +266,7 @@ private void addTrackerParameters(TrackerPayload payload) { private void addContext(Event event, TrackerPayload payload) { List entities = event.getContext(); - // Build the final context and addEvent it to the payload + // Build the final context and add it to the payload if (entities != null && entities.size() > 0) { SelfDescribingJson envelope = getFinalContext(entities); payload.addMap(envelope.getMap(), this.parameters.getBase64Encoded(), Parameter.CONTEXT_ENCODED, Parameter.CONTEXT); diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/Utils.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/Utils.java index e79cc2ef..a031e0d3 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/Utils.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/Utils.java @@ -143,7 +143,7 @@ public static String mapToQueryString(Map map) { String encodedKey = urlEncodeUTF8(key); String encodedVal = urlEncodeUTF8(map.get(key)); - // Do not addEvent empty Keys + // Do not add empty Keys if (!encodedKey.isEmpty()) { sb.append(String.format("%s=%s", encodedKey, encodedVal)); } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java index 3d936970..1af13dc6 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java @@ -172,7 +172,7 @@ public long getRetryDelay() { /** * Returns a Runnable POST Request operation * - * @param numberOfEvents the event buffer to be sent + * @param numberOfEvents the number of events to be sent in the request * @return the new Runnable object */ private Runnable getPostRequestRunnable(int numberOfEvents) { diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java index bc3822a8..92bb8988 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java @@ -214,7 +214,7 @@ public Subject getSubject() { /** * Adds the default parameters to a TrackerPayload object. * - * @param payload the payload to addEvent too. + * @param payload the payload to add to. * @return the TrackerPayload with appended values. */ protected TrackerPayload putDefaultParams(TrackerPayload payload) { diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/Payload.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/Payload.java index 50a57483..e6febee3 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/Payload.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/Payload.java @@ -32,7 +32,7 @@ public interface Payload { /** * Add all the mappings from the specified map. The effect is the equivalent to that of calling: - * - addEvent(String key, String value) for each key value pair. + * - add(String key, String value) for each key value pair. * * @param map Key-Value pairs to be stored in this payload */ diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/SelfDescribingJson.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/SelfDescribingJson.java index abe7ba0e..a41285ea 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/SelfDescribingJson.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/SelfDescribingJson.java @@ -125,10 +125,10 @@ public SelfDescribingJson setData(Object data) { } /** - * Allows us to addEvent data from one SelfDescribingJson into another + * Allows us to add data from one SelfDescribingJson into another * without copying over the Schema. * - * @param data the payload to addEvent to the SelfDescribingJson + * @param data the payload to add to the SelfDescribingJson * @return this SelfDescribingJson */ public SelfDescribingJson setData(SelfDescribingJson data) { @@ -142,7 +142,7 @@ public SelfDescribingJson setData(SelfDescribingJson data) { @Deprecated @Override public void add(String key, String value) { - LOGGER.info("Payload: addEvent(String, String) method called - Doing nothing."); + LOGGER.info("Payload: add(String, String) method called - Doing nothing."); } @Deprecated diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java index 82f38a4f..93bb9303 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java @@ -53,7 +53,7 @@ public void add(final String key, final String value) { /** * Add all the mappings from the specified map. The effect is the equivalent to - * that of calling: - addEvent(String key, String value) for each key value pair. + * that of calling: - add(String key, String value) for each key value pair. * * @param map Key-Value pairs to be stored in this payload */ From 3b02062ce38b40533c6214c3160e2b6bef5508b9 Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Thu, 24 Feb 2022 10:34:38 +0000 Subject: [PATCH 13/20] Restore SimpleEmitter functionality --- .../src/main/java/com/snowplowanalytics/Main.java | 14 -------------- .../snowplow/tracker/emitter/SimpleEmitter.java | 7 ++++++- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java b/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java index 67aa343a..a1d96ef1 100644 --- a/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java +++ b/examples/simple-console/src/main/java/com/snowplowanalytics/Main.java @@ -36,14 +36,6 @@ public static String getUrlFromArgs(String[] args) { return args[0]; } - public static PageView getPageView() { - return PageView.builder() - .pageTitle("Snowplow Analytics") - .pageUrl("https://www.snowplowanalytics.com") - .referrer("https://www.google.com") - .build(); - } - public static void main(String[] args) throws InterruptedException { String collectorEndpoint = getUrlFromArgs(args); @@ -58,12 +50,6 @@ public static void main(String[] args) throws InterruptedException { .bufferSize(4) // send batches of 4 events. In production this number should be higher, depending on the size/event volume .build(); - // for versions before 0.12.0, the property batchSize was called bufferSize -// BatchEmitter emitter = BatchEmitter.builder() -// .url(collectorEndpoint) -// .bufferSize(4) -// .build(); - // now we have the emitter, we need a tracker to turn our events into something a Snowplow collector can understand final Tracker tracker = new Tracker.TrackerBuilder(emitter, namespace, appId) .base64(true) diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/SimpleEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/SimpleEmitter.java index e3849efb..067b9e06 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/SimpleEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/SimpleEmitter.java @@ -50,9 +50,14 @@ protected SimpleEmitter(final Builder builder) { super(builder); } + /** + * Adds an event to the buffer and instantly sends it + * + * @param payload a payload + */ @Override public void add(TrackerPayload payload) { - // nothing happens + executor.execute(getGetRequestRunnable(payload)); } /** From d68018dff0ee7490e887b9a20355bccbbad08556 Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Thu, 24 Feb 2022 14:15:23 +0000 Subject: [PATCH 14/20] Use atomicLong more effectively --- .../snowplow/tracker/emitter/BatchEmitter.java | 13 ++++--------- .../snowplow/tracker/emitter/EventStore.java | 2 +- .../tracker/emitter/InMemoryEventStore.java | 2 +- .../snowplow/tracker/emitter/BatchEmitterTest.java | 1 - .../tracker/emitter/InMemoryEventStoreTest.java | 14 +++++++------- 5 files changed, 13 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java index 1af13dc6..e5ae6c7b 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java @@ -16,7 +16,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; @@ -95,7 +94,6 @@ protected BatchEmitter(final Builder builder) { this.bufferSize = builder.bufferSize; this.bufferCapacity = builder.bufferCapacity; -// this.eventStore = builder.eventStore; if (builder.eventStore == null) { this.eventStore = new InMemoryEventStore(this.bufferCapacity); @@ -116,7 +114,7 @@ public void add(final TrackerPayload payload) { boolean result = this.eventStore.addEvent(payload); if (!isClosing) { - if (this.eventStore.getSize() >= this.bufferSize) { + if (this.eventStore.size() >= this.bufferSize) { executor.schedule(getPostRequestRunnable(this.bufferSize), this.retryDelay.get(), TimeUnit.MILLISECONDS); } } @@ -131,7 +129,7 @@ public void add(final TrackerPayload payload) { */ @Override public void flushBuffer() { - executor.schedule(getPostRequestRunnable(this.eventStore.getSize()), 0, TimeUnit.MILLISECONDS); + executor.schedule(getPostRequestRunnable(this.eventStore.size()), 0, TimeUnit.MILLISECONDS); } /** @@ -199,11 +197,8 @@ private Runnable getPostRequestRunnable(int numberOfEvents) { eventStore.cleanupAfterSendingAttempt(false, batchedEvents.getBatchId()); // exponentially increase retry backoff time after the first failure - long currentDelay = this.retryDelay.get(); - if (currentDelay == 0) { - this.retryDelay.compareAndSet(0, 50L); - } else { - this.retryDelay.set(currentDelay * 2); + if (!this.retryDelay.compareAndSet(0, 50L)) { + this.retryDelay.updateAndGet(currentDelay -> currentDelay * 2); } } } catch (Exception e) { diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java index 41cf70d5..fad8bc4a 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java @@ -14,5 +14,5 @@ public interface EventStore { void cleanupAfterSendingAttempt(boolean successfullySent, long batchId); - int getSize(); + int size(); } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java index 54c92d47..dfbaa78a 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java @@ -69,7 +69,7 @@ public List getAllEvents() { } @Override - public int getSize() { + public int size() { return eventBuffer.size(); } } diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java index eebf5b0f..8d89c533 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java @@ -247,7 +247,6 @@ public void close_sendsEventsAndStopsThreads() throws InterruptedException { for (TrackerPayload payload : payloads) { emitter.add(payload); } - System.out.println(emitter.getBuffer().size()); Thread.sleep(500); emitter.close(); diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java index 0a15b8c7..72a0e5f1 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java @@ -43,7 +43,7 @@ public void getSize_returnsCorrectNumberOfStoredEvents() { eventStore.addEvent(trackerPayload); eventStore.addEvent(trackerPayload); - Assert.assertEquals(2, eventStore.getSize()); + Assert.assertEquals(2, eventStore.size()); } @Test @@ -54,7 +54,7 @@ public void getEventsFromStorage() { eventStore.addEvent(trackerPayload); Assert.assertEquals(2, eventStore.getEventBatch(2).getPayloads().size()); - Assert.assertEquals(2, eventStore.getSize()); + Assert.assertEquals(2, eventStore.size()); } @Test @@ -73,11 +73,11 @@ public void putEventsBackInBufferIfFailedToSend() { eventStore.addEvent(trackerPayload); eventStore.getEventBatch(2); - Assert.assertEquals(0, eventStore.getSize()); + Assert.assertEquals(0, eventStore.size()); eventStore.cleanupAfterSendingAttempt(false, 1L); - Assert.assertEquals(2, eventStore.getSize()); + Assert.assertEquals(2, eventStore.size()); } @Test @@ -86,11 +86,11 @@ public void doNotPutEventsBackInBufferIfSent() { eventStore.addEvent(trackerPayload); eventStore.getEventBatch(2); - Assert.assertEquals(0, eventStore.getSize()); + Assert.assertEquals(0, eventStore.size()); eventStore.cleanupAfterSendingAttempt(true, 1L); - Assert.assertEquals(0, eventStore.getSize()); + Assert.assertEquals(0, eventStore.size()); } @Test @@ -107,7 +107,7 @@ public void dropEventsOnFailureWhenBufferFull() { eventStore.addEvent(trackerPayload); eventStore.cleanupAfterSendingAttempt(false, 1L); - Assert.assertEquals(3, eventStore.getSize()); + Assert.assertEquals(3, eventStore.size()); Assert.assertTrue(eventStore.getAllEvents().contains(differentPayload)); } From 3adc24ee4916631a05a57b84a43aa7ae6b777f01 Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Fri, 25 Feb 2022 10:32:20 +0000 Subject: [PATCH 15/20] Remove unnecessary 'this'es --- .../tracker/emitter/BatchEmitter.java | 39 ++++++++----------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java index e5ae6c7b..1d8a4703 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java @@ -35,16 +35,11 @@ public class BatchEmitter extends AbstractEmitter implements Closeable { private static final Logger LOGGER = LoggerFactory.getLogger(BatchEmitter.class); - private boolean isClosing = false; - private int bufferSize; - private int bufferCapacity; private final EventStore eventStore; private final AtomicLong retryDelay; - private final long closeTimeout = 5; - public static abstract class Builder> extends AbstractEmitter.Builder { private int bufferSize = 50; // Optional @@ -91,17 +86,14 @@ protected BatchEmitter(final Builder builder) { // Precondition checks Preconditions.checkArgument(builder.bufferSize > 0, "bufferSize must be greater than 0"); - - this.bufferSize = builder.bufferSize; - this.bufferCapacity = builder.bufferCapacity; + bufferSize = builder.bufferSize; if (builder.eventStore == null) { - this.eventStore = new InMemoryEventStore(this.bufferCapacity); + eventStore = new InMemoryEventStore(builder.bufferCapacity); } else { - this.eventStore = builder.eventStore; + eventStore = builder.eventStore; } - - this.retryDelay = new AtomicLong(0); + retryDelay = new AtomicLong(0L); } /** @@ -111,11 +103,11 @@ protected BatchEmitter(final Builder builder) { */ @Override public void add(final TrackerPayload payload) { - boolean result = this.eventStore.addEvent(payload); + boolean result = eventStore.addEvent(payload); if (!isClosing) { - if (this.eventStore.size() >= this.bufferSize) { - executor.schedule(getPostRequestRunnable(this.bufferSize), this.retryDelay.get(), TimeUnit.MILLISECONDS); + if (eventStore.size() >= bufferSize) { + executor.schedule(getPostRequestRunnable(bufferSize), retryDelay.get(), TimeUnit.MILLISECONDS); } } @@ -129,7 +121,7 @@ public void add(final TrackerPayload payload) { */ @Override public void flushBuffer() { - executor.schedule(getPostRequestRunnable(this.eventStore.size()), 0, TimeUnit.MILLISECONDS); + executor.schedule(getPostRequestRunnable(eventStore.size()), 0, TimeUnit.MILLISECONDS); } /** @@ -139,7 +131,7 @@ public void flushBuffer() { */ @Override public List getBuffer() { - return this.eventStore.getAllEvents(); + return eventStore.getAllEvents(); } /** @@ -160,11 +152,11 @@ public void setBufferSize(final int bufferSize) { */ @Override public int getBufferSize() { - return this.bufferSize; + return bufferSize; } public long getRetryDelay() { - return this.retryDelay.get(); + return retryDelay.get(); } /** @@ -177,7 +169,7 @@ private Runnable getPostRequestRunnable(int numberOfEvents) { return () -> { BatchPayload batchedEvents = null; try { - batchedEvents = this.eventStore.getEventBatch(numberOfEvents); + batchedEvents = eventStore.getEventBatch(numberOfEvents); List eventsInRequest = batchedEvents.getPayloads(); if (eventsInRequest.size() == 0) { @@ -190,15 +182,15 @@ private Runnable getPostRequestRunnable(int numberOfEvents) { // Process results if (isSuccessfulSend(code)) { LOGGER.debug("BatchEmitter successfully sent {} events: code: {}", eventsInRequest.size(), code); - this.retryDelay.set(0L); + retryDelay.set(0L); eventStore.cleanupAfterSendingAttempt(true, batchedEvents.getBatchId()); } else { LOGGER.error("BatchEmitter failed to send {} events: code: {}", eventsInRequest.size(), code); eventStore.cleanupAfterSendingAttempt(false, batchedEvents.getBatchId()); // exponentially increase retry backoff time after the first failure - if (!this.retryDelay.compareAndSet(0, 50L)) { - this.retryDelay.updateAndGet(currentDelay -> currentDelay * 2); + if (!retryDelay.compareAndSet(0, 50L)) { + retryDelay.updateAndGet(currentDelay -> currentDelay * 2); } } } catch (Exception e) { @@ -233,6 +225,7 @@ private SelfDescribingJson getFinalPost(final List events) { */ @Override public void close() { + final long closeTimeout = 5; isClosing = true; flushBuffer(); // Attempt to send all remaining events From af8c6256b9b026ca3d449a841b0069fc5933ac78 Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Wed, 2 Mar 2022 17:16:21 +0000 Subject: [PATCH 16/20] Tidy up Emitter code --- .../snowplow/tracker/emitter/AbstractEmitter.java | 3 +-- .../snowplow/tracker/emitter/BatchEmitter.java | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java index 155323f4..a942c6e0 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java @@ -13,7 +13,6 @@ package com.snowplowanalytics.snowplow.tracker.emitter; import java.util.List; -import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ThreadFactory; @@ -47,7 +46,7 @@ public static abstract class Builder> { /** * Set a custom ScheduledExecutorService to send http request. * - * /!\ Be aware that calling `close()` on a BatchEmitter instance has a side-effect and will shutdown that ExecutorService. + * @implNote Be aware that calling `close()` on a BatchEmitter instance has a side-effect and will shutdown that ExecutorService. * @param executorService the ScheduledExecutorService to use * @return itself */ diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java index 1d8a4703..9cd564bb 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java @@ -100,6 +100,7 @@ protected BatchEmitter(final Builder builder) { * Adds a TrackerPayload to the concurrent queue buffer * * @param payload a payload + * @implNote As a side effect it triggers an Emitter thread to emit a batch of events. */ @Override public void add(final TrackerPayload payload) { @@ -155,7 +156,7 @@ public int getBufferSize() { return bufferSize; } - public long getRetryDelay() { + long getRetryDelay() { return retryDelay.get(); } From dadf07fc2932735686da81d514e507438008a909 Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Wed, 2 Mar 2022 17:29:11 +0000 Subject: [PATCH 17/20] Update BatchEmitter javadoc comments --- .../snowplow/tracker/emitter/BatchEmitter.java | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java index 9cd564bb..14bb9485 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java @@ -55,11 +55,19 @@ public T bufferSize(final int bufferSize) { return self(); } + /** + * @param eventStore The EventStore to use + * @return itself + */ public T eventStore(final EventStore eventStore) { this.eventStore = eventStore; return self(); } + /** + * @param bufferCapacity The maximum capacity of the default InMemoryEventStore event buffer + * @return itself + */ public T bufferCapacity(final int bufferCapacity) { this.bufferCapacity = bufferCapacity; return self(); @@ -117,8 +125,8 @@ public void add(final TrackerPayload payload) { } } - /* - * Forces all the payloads currently in the buffer to be sent + /** + * Forces all the payloads currently in the buffer to be sent immediately */ @Override public void flushBuffer() { @@ -222,7 +230,8 @@ private SelfDescribingJson getFinalPost(final List events) { } /** - * On close attempt to send all remaining events. + * On close, attempt to send all remaining events. + * @implNote Be aware that calling `close()` has a side-effect of shutting down the Emitter ScheduledExecutorService. */ @Override public void close() { From 73ceabbea91e6b1393f0c14d6775d71267e669df Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Wed, 2 Mar 2022 18:08:19 +0000 Subject: [PATCH 18/20] Remove implNote javadoc tags --- .../snowplow/tracker/emitter/AbstractEmitter.java | 4 +++- .../snowplow/tracker/emitter/BatchEmitter.java | 8 ++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java index a942c6e0..de14c06e 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java @@ -45,8 +45,10 @@ public static abstract class Builder> { /** * Set a custom ScheduledExecutorService to send http request. + *

+ * Implementation note: Be aware that calling `close()` on a BatchEmitter instance + * has a side-effect and will shutdown that ExecutorService. * - * @implNote Be aware that calling `close()` on a BatchEmitter instance has a side-effect and will shutdown that ExecutorService. * @param executorService the ScheduledExecutorService to use * @return itself */ diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java index 14bb9485..449d3953 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java @@ -106,9 +106,11 @@ protected BatchEmitter(final Builder builder) { /** * Adds a TrackerPayload to the concurrent queue buffer + *

+ * Implementation note: Be aware that calling `close()` on a BatchEmitter instance + * has a side-effect and will shutdown that ExecutorService. * * @param payload a payload - * @implNote As a side effect it triggers an Emitter thread to emit a batch of events. */ @Override public void add(final TrackerPayload payload) { @@ -231,7 +233,9 @@ private SelfDescribingJson getFinalPost(final List events) { /** * On close, attempt to send all remaining events. - * @implNote Be aware that calling `close()` has a side-effect of shutting down the Emitter ScheduledExecutorService. + *

+ * Implementation note: Be aware that calling `close()` + * has a side-effect of shutting down the Emitter ScheduledExecutorService. */ @Override public void close() { From e02468fc75e7a5faf325925295dc3105d671ca0d Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Wed, 2 Mar 2022 19:17:00 +0000 Subject: [PATCH 19/20] Tidy up tests --- .../tracker/emitter/BatchEmitterTest.java | 27 ++++++------------- .../emitter/InMemoryEventStoreTest.java | 2 +- 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java index 8d89c533..cb86d1db 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java @@ -70,9 +70,11 @@ public Object getHttpClient() { } } - public static class FailingHttpClientAdapter implements HttpClientAdapter { - public int failedPostCounter = 0; - public int successfulPostCounter = 0; + // this class fails to "send" the first 4 requests + // but returns a successful result (200) subsequently + static class FailingHttpClientAdapter implements HttpClientAdapter { + int failedPostCounter = 0; + int successfulPostCounter = 0; @Override public int post(SelfDescribingJson payload) { if (failedPostCounter >= 4) { @@ -119,11 +121,9 @@ public void addToBuffer_withLess10Payloads_shouldNotEmptyBuffer() throws Interru Thread.sleep(500); - List storedPayloads = emitter.getBuffer(); - Assert.assertFalse(mockHttpClientAdapter.isPostCalled); Assert.assertEquals(2, emitter.getBuffer().size()); - Assert.assertEquals(payloads, storedPayloads); + Assert.assertEquals(payloads, emitter.getBuffer()); } @Test @@ -139,7 +139,6 @@ public void addToBuffer_withMore10Payloads_shouldEmptyBuffer() throws Interrupte @SuppressWarnings("unchecked") List> capturedPayload = (List>) mockHttpClientAdapter.capturedPayload.getMap().get("data"); -// assertPayload(payloads, capturedPayload); Assert.assertEquals(0, emitter.getBuffer().size()); Assert.assertEquals(1, mockHttpClientAdapter.postCounter); } @@ -276,10 +275,6 @@ public void eventsThatFailToSendAreReturnedToEventBuffer() throws InterruptedExc for (TrackerPayload payload : payloads) { emitter.add(payload); } - - Thread.sleep(500); - - Assert.assertEquals(payloads, emitter.getBuffer()); emitter.flushBuffer(); Thread.sleep(500); @@ -314,11 +309,9 @@ public void successfulSendAfterFailureResetsBackoffTime() throws InterruptedExce emitter = BatchEmitter.builder() .httpClientAdapter(failingHttpClientAdapter) .bufferSize(1) + .threadCount(1) .build(); - // these requests will pass the failingHttpClientAdapter threshold for failing - // but because they are being processed asynchronously, the successful requests - // may be processed by the Emitter before the failed ones List payloads = createPayloads(6); for (TrackerPayload payload : payloads) { emitter.add(payload); @@ -326,11 +319,7 @@ public void successfulSendAfterFailureResetsBackoffTime() throws InterruptedExce Thread.sleep(500); - // this request will definitely succeed - emitter.add(createPayload()); - Thread.sleep(500); - - Assert.assertEquals(3, failingHttpClientAdapter.successfulPostCounter); + Assert.assertEquals(2, failingHttpClientAdapter.successfulPostCounter); Assert.assertEquals(0, emitter.getRetryDelay()); } diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java index 72a0e5f1..610c09b6 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java @@ -94,7 +94,7 @@ public void doNotPutEventsBackInBufferIfSent() { } @Test - public void dropEventsOnFailureWhenBufferFull() { + public void dropNewerEventsOnFailureWhenBufferFull() { eventStore = new InMemoryEventStore(3); TrackerPayload differentPayload = createTrackerPayload(); From 436a4ffdf5bcc2f6763e84ab2c21ecda4098bd42 Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Wed, 2 Mar 2022 19:18:21 +0000 Subject: [PATCH 20/20] Rename EventStore getEventBatch to getEventsBatch --- .../snowplow/tracker/emitter/BatchEmitter.java | 2 +- .../snowplow/tracker/emitter/EventStore.java | 2 +- .../snowplow/tracker/emitter/InMemoryEventStore.java | 2 +- .../tracker/emitter/InMemoryEventStoreTest.java | 10 +++++----- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java index 449d3953..12c1f706 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java @@ -180,7 +180,7 @@ private Runnable getPostRequestRunnable(int numberOfEvents) { return () -> { BatchPayload batchedEvents = null; try { - batchedEvents = eventStore.getEventBatch(numberOfEvents); + batchedEvents = eventStore.getEventsBatch(numberOfEvents); List eventsInRequest = batchedEvents.getPayloads(); if (eventsInRequest.size() == 0) { diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java index fad8bc4a..9a2eaa6a 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/EventStore.java @@ -8,7 +8,7 @@ public interface EventStore { boolean addEvent(TrackerPayload trackerPayload); - BatchPayload getEventBatch(int numberToRemove); + BatchPayload getEventsBatch(int numberToRemove); List getAllEvents(); diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java index dfbaa78a..d5366da5 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStore.java @@ -30,7 +30,7 @@ public boolean addEvent(TrackerPayload trackerPayload) { } @Override - public BatchPayload getEventBatch(int numberToGet) { + public BatchPayload getEventsBatch(int numberToGet) { List eventsToSend = new ArrayList<>(); eventBuffer.drainTo(eventsToSend, numberToGet); diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java index 610c09b6..33a7d843 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/InMemoryEventStoreTest.java @@ -53,7 +53,7 @@ public void getEventsFromStorage() { eventStore.addEvent(trackerPayload); eventStore.addEvent(trackerPayload); - Assert.assertEquals(2, eventStore.getEventBatch(2).getPayloads().size()); + Assert.assertEquals(2, eventStore.getEventsBatch(2).getPayloads().size()); Assert.assertEquals(2, eventStore.size()); } @@ -62,7 +62,7 @@ public void getAllEventsIfAskedForMoreEventsThanAreStored() { eventStore.addEvent(trackerPayload); eventStore.addEvent(trackerPayload); - List events = eventStore.getEventBatch(3).getPayloads(); + List events = eventStore.getEventsBatch(3).getPayloads(); Assert.assertEquals(2, events.size()); } @@ -71,7 +71,7 @@ public void getAllEventsIfAskedForMoreEventsThanAreStored() { public void putEventsBackInBufferIfFailedToSend() { eventStore.addEvent(trackerPayload); eventStore.addEvent(trackerPayload); - eventStore.getEventBatch(2); + eventStore.getEventsBatch(2); Assert.assertEquals(0, eventStore.size()); @@ -84,7 +84,7 @@ public void putEventsBackInBufferIfFailedToSend() { public void doNotPutEventsBackInBufferIfSent() { eventStore.addEvent(trackerPayload); eventStore.addEvent(trackerPayload); - eventStore.getEventBatch(2); + eventStore.getEventsBatch(2); Assert.assertEquals(0, eventStore.size()); @@ -100,7 +100,7 @@ public void dropNewerEventsOnFailureWhenBufferFull() { TrackerPayload differentPayload = createTrackerPayload(); eventStore.addEvent(differentPayload); - eventStore.getEventBatch(1); + eventStore.getEventsBatch(1); eventStore.addEvent(trackerPayload); eventStore.addEvent(trackerPayload);