Skip to content

Commit 0558d3d

Browse files
AlexandreCarltonAlexandreCarlton
committed
Add context objects to StatisticsCollector methods
The `StatisticsCollector` interface is useful for gathering basic metrics about data loaders, but is limited in that we are unable to pass in any sort of context (key or call). This would allow us to obtain more insightful metrics (for example, we would be able to break down metrics based on the GraphQL operation name). To this end, we add `*StatisticsContext` objects to each `StatisticsCollector` method; we provide distinct implementations for each one rather than a generic catch-all so that we may evolve them independently as needed.
1 parent d42e37e commit 0558d3d

13 files changed

+438
-89
lines changed

‎src/main/java/org/dataloader/DataLoaderHelper.java

+15-8
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@
44
import org.dataloader.annotations.Internal;
55
import org.dataloader.impl.CompletableFutureKit;
66
import org.dataloader.stats.StatisticsCollector;
7+
import org.dataloader.stats.context.IncrementBatchLoadCountByStatisticsContext;
8+
import org.dataloader.stats.context.IncrementBatchLoadExceptionCountStatisticsContext;
9+
import org.dataloader.stats.context.IncrementCacheHitCountStatisticsContext;
10+
import org.dataloader.stats.context.IncrementLoadCountStatisticsContext;
11+
import org.dataloader.stats.context.IncrementLoadErrorCountStatisticsContext;
712

813
import java.time.Clock;
914
import java.time.Instant;
@@ -105,7 +110,7 @@ Optional<CompletableFuture<V>> getIfPresent(K key) {
105110
if (cachingEnabled) {
106111
Object cacheKey = getCacheKey(nonNull(key));
107112
if (futureCache.containsKey(cacheKey)) {
108-
stats.incrementCacheHitCount();
113+
stats.incrementCacheHitCount(new IncrementCacheHitCountStatisticsContext<>(key));
109114
return Optional.of(futureCache.get(cacheKey));
110115
}
111116
}
@@ -132,7 +137,7 @@ CompletableFuture<V> load(K key, Object loadContext) {
132137
boolean batchingEnabled = loaderOptions.batchingEnabled();
133138
boolean cachingEnabled = loaderOptions.cachingEnabled();
134139

135-
stats.incrementLoadCount();
140+
stats.incrementLoadCount(new IncrementLoadCountStatisticsContext<>(key, loadContext));
136141

137142
if (cachingEnabled) {
138143
return loadFromCache(key, loadContext, batchingEnabled);
@@ -223,18 +228,20 @@ private CompletableFuture<List<V>> sliceIntoBatchesOfBatches(List<K> keys, List<
223228

224229
@SuppressWarnings("unchecked")
225230
private CompletableFuture<List<V>> dispatchQueueBatch(List<K> keys, List<Object> callContexts, List<CompletableFuture<V>> queuedFutures) {
226-
stats.incrementBatchLoadCountBy(keys.size());
231+
stats.incrementBatchLoadCountBy(keys.size(), new IncrementBatchLoadCountByStatisticsContext<>(keys, callContexts));
227232
CompletableFuture<List<V>> batchLoad = invokeLoader(keys, callContexts, loaderOptions.cachingEnabled());
228233
return batchLoad
229234
.thenApply(values -> {
230235
assertResultSize(keys, values);
231236

232237
List<K> clearCacheKeys = new ArrayList<>();
233238
for (int idx = 0; idx < queuedFutures.size(); idx++) {
239+
K key = keys.get(idx);
234240
V value = values.get(idx);
241+
Object callContext = callContexts.get(idx);
235242
CompletableFuture<V> future = queuedFutures.get(idx);
236243
if (value instanceof Throwable) {
237-
stats.incrementLoadErrorCount();
244+
stats.incrementLoadErrorCount(new IncrementLoadErrorCountStatisticsContext<>(key, callContext));
238245
future.completeExceptionally((Throwable) value);
239246
clearCacheKeys.add(keys.get(idx));
240247
} else if (value instanceof Try) {
@@ -244,7 +251,7 @@ private CompletableFuture<List<V>> dispatchQueueBatch(List<K> keys, List<Object>
244251
if (tryValue.isSuccess()) {
245252
future.complete(tryValue.get());
246253
} else {
247-
stats.incrementLoadErrorCount();
254+
stats.incrementLoadErrorCount(new IncrementLoadErrorCountStatisticsContext<>(key, callContext));
248255
future.completeExceptionally(tryValue.getThrowable());
249256
clearCacheKeys.add(keys.get(idx));
250257
}
@@ -255,7 +262,7 @@ private CompletableFuture<List<V>> dispatchQueueBatch(List<K> keys, List<Object>
255262
possiblyClearCacheEntriesOnExceptions(clearCacheKeys);
256263
return values;
257264
}).exceptionally(ex -> {
258-
stats.incrementBatchLoadExceptionCount();
265+
stats.incrementBatchLoadExceptionCount(new IncrementBatchLoadExceptionCountStatisticsContext<>(keys, callContexts));
259266
if (ex instanceof CompletionException) {
260267
ex = ex.getCause();
261268
}
@@ -294,7 +301,7 @@ private CompletableFuture<V> loadFromCache(K key, Object loadContext, boolean ba
294301

295302
if (futureCache.containsKey(cacheKey)) {
296303
// We already have a promise for this key, no need to check value cache or queue up load
297-
stats.incrementCacheHitCount();
304+
stats.incrementCacheHitCount(new IncrementCacheHitCountStatisticsContext<>(key, loadContext));
298305
return futureCache.get(cacheKey);
299306
}
300307

@@ -310,7 +317,7 @@ private CompletableFuture<V> queueOrInvokeLoader(K key, Object loadContext, bool
310317
loaderQueue.add(new LoaderQueueEntry<>(key, loadCallFuture, loadContext));
311318
return loadCallFuture;
312319
} else {
313-
stats.incrementBatchLoadCountBy(1);
320+
stats.incrementBatchLoadCountBy(1, new IncrementBatchLoadCountByStatisticsContext<>(key, loadContext));
314321
// immediate execution of batch function
315322
return invokeLoaderImmediately(key, loadContext, cachingEnabled);
316323
}

‎src/main/java/org/dataloader/stats/DelegatingStatisticsCollector.java

+49-13
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
package org.dataloader.stats;
22

3+
import org.dataloader.stats.context.IncrementBatchLoadCountByStatisticsContext;
4+
import org.dataloader.stats.context.IncrementBatchLoadExceptionCountStatisticsContext;
5+
import org.dataloader.stats.context.IncrementCacheHitCountStatisticsContext;
6+
import org.dataloader.stats.context.IncrementLoadCountStatisticsContext;
7+
import org.dataloader.stats.context.IncrementLoadErrorCountStatisticsContext;
8+
39
import static org.dataloader.impl.Assertions.nonNull;
410

511
/**
@@ -20,33 +26,63 @@ public DelegatingStatisticsCollector(StatisticsCollector delegateCollector) {
2026
}
2127

2228
@Override
23-
public long incrementLoadCount() {
24-
delegateCollector.incrementLoadCount();
25-
return collector.incrementLoadCount();
29+
public <K> long incrementLoadCount(IncrementLoadCountStatisticsContext<K> context) {
30+
delegateCollector.incrementLoadCount(context);
31+
return collector.incrementLoadCount(context);
2632
}
2733

34+
@Deprecated
2835
@Override
29-
public long incrementBatchLoadCountBy(long delta) {
30-
delegateCollector.incrementBatchLoadCountBy(delta);
31-
return collector.incrementBatchLoadCountBy(delta);
36+
public long incrementLoadCount() {
37+
return incrementLoadCount(null);
3238
}
3339

3440
@Override
35-
public long incrementCacheHitCount() {
36-
delegateCollector.incrementCacheHitCount();
37-
return collector.incrementCacheHitCount();
41+
public <K> long incrementLoadErrorCount(IncrementLoadErrorCountStatisticsContext<K> context) {
42+
delegateCollector.incrementLoadErrorCount(context);
43+
return collector.incrementLoadErrorCount(context);
3844
}
3945

46+
@Deprecated
4047
@Override
4148
public long incrementLoadErrorCount() {
42-
delegateCollector.incrementLoadErrorCount();
43-
return collector.incrementLoadErrorCount();
49+
return incrementLoadErrorCount(null);
50+
}
51+
52+
@Override
53+
public <K> long incrementBatchLoadCountBy(long delta, IncrementBatchLoadCountByStatisticsContext<K> context) {
54+
delegateCollector.incrementBatchLoadCountBy(delta, context);
55+
return collector.incrementBatchLoadCountBy(delta, context);
56+
}
57+
58+
@Deprecated
59+
@Override
60+
public long incrementBatchLoadCountBy(long delta) {
61+
return incrementBatchLoadCountBy(delta, null);
62+
}
63+
64+
@Override
65+
public <K> long incrementBatchLoadExceptionCount(IncrementBatchLoadExceptionCountStatisticsContext<K> context) {
66+
delegateCollector.incrementBatchLoadExceptionCount(context);
67+
return collector.incrementBatchLoadExceptionCount(context);
4468
}
4569

70+
@Deprecated
4671
@Override
4772
public long incrementBatchLoadExceptionCount() {
48-
delegateCollector.incrementBatchLoadExceptionCount();
49-
return collector.incrementBatchLoadExceptionCount();
73+
return incrementBatchLoadExceptionCount(null);
74+
}
75+
76+
@Override
77+
public <K> long incrementCacheHitCount(IncrementCacheHitCountStatisticsContext<K> context) {
78+
delegateCollector.incrementCacheHitCount(context);
79+
return collector.incrementCacheHitCount(context);
80+
}
81+
82+
@Deprecated
83+
@Override
84+
public long incrementCacheHitCount() {
85+
return incrementCacheHitCount(null);
5086
}
5187

5288
/**

‎src/main/java/org/dataloader/stats/NoOpStatisticsCollector.java

+37-1
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,71 @@
11
package org.dataloader.stats;
22

3+
import org.dataloader.stats.context.IncrementBatchLoadCountByStatisticsContext;
4+
import org.dataloader.stats.context.IncrementBatchLoadExceptionCountStatisticsContext;
5+
import org.dataloader.stats.context.IncrementCacheHitCountStatisticsContext;
6+
import org.dataloader.stats.context.IncrementLoadCountStatisticsContext;
7+
import org.dataloader.stats.context.IncrementLoadErrorCountStatisticsContext;
8+
39
/**
410
* A statistics collector that does nothing
511
*/
612
public class NoOpStatisticsCollector implements StatisticsCollector {
713

814
private static final Statistics ZERO_STATS = new Statistics();
915

16+
@Override
17+
public <K> long incrementLoadCount(IncrementLoadCountStatisticsContext<K> context) {
18+
return 0;
19+
}
20+
21+
@Deprecated
1022
@Override
1123
public long incrementLoadCount() {
24+
return incrementLoadCount(null);
25+
}
26+
27+
@Override
28+
public <K> long incrementLoadErrorCount(IncrementLoadErrorCountStatisticsContext<K> context) {
1229
return 0;
1330
}
1431

32+
@Deprecated
1533
@Override
1634
public long incrementLoadErrorCount() {
35+
return incrementLoadErrorCount(null);
36+
}
37+
38+
@Override
39+
public <K> long incrementBatchLoadCountBy(long delta, IncrementBatchLoadCountByStatisticsContext<K> context) {
1740
return 0;
1841
}
1942

43+
@Deprecated
2044
@Override
2145
public long incrementBatchLoadCountBy(long delta) {
46+
return incrementBatchLoadCountBy(delta, null);
47+
}
48+
49+
@Override
50+
public <K> long incrementBatchLoadExceptionCount(IncrementBatchLoadExceptionCountStatisticsContext<K> context) {
2251
return 0;
2352
}
2453

54+
@Deprecated
2555
@Override
2656
public long incrementBatchLoadExceptionCount() {
57+
return incrementBatchLoadExceptionCount(null);
58+
}
59+
60+
@Override
61+
public <K> long incrementCacheHitCount(IncrementCacheHitCountStatisticsContext<K> context) {
2762
return 0;
2863
}
2964

65+
@Deprecated
3066
@Override
3167
public long incrementCacheHitCount() {
32-
return 0;
68+
return incrementCacheHitCount(null);
3369
}
3470

3571
@Override

‎src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java

+42-7
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
package org.dataloader.stats;
22

3+
import org.dataloader.stats.context.IncrementBatchLoadCountByStatisticsContext;
4+
import org.dataloader.stats.context.IncrementBatchLoadExceptionCountStatisticsContext;
5+
import org.dataloader.stats.context.IncrementCacheHitCountStatisticsContext;
6+
import org.dataloader.stats.context.IncrementLoadCountStatisticsContext;
7+
import org.dataloader.stats.context.IncrementLoadErrorCountStatisticsContext;
8+
39
import java.util.concurrent.atomic.AtomicLong;
410

511
/**
@@ -17,30 +23,59 @@ public class SimpleStatisticsCollector implements StatisticsCollector {
1723
private final AtomicLong loadErrorCount = new AtomicLong();
1824

1925
@Override
20-
public long incrementLoadCount() {
26+
public <K> long incrementLoadCount(IncrementLoadCountStatisticsContext<K> context) {
2127
return loadCount.incrementAndGet();
2228
}
2329

30+
@Deprecated
31+
@Override
32+
public long incrementLoadCount() {
33+
return incrementLoadCount(null);
34+
}
2435

2536
@Override
26-
public long incrementBatchLoadCountBy(long delta) {
37+
public <K> long incrementLoadErrorCount(IncrementLoadErrorCountStatisticsContext<K> context) {
38+
return loadErrorCount.incrementAndGet();
39+
}
40+
41+
@Deprecated
42+
@Override
43+
public long incrementLoadErrorCount() {
44+
return incrementLoadErrorCount(null);
45+
}
46+
47+
@Override
48+
public <K> long incrementBatchLoadCountBy(long delta, IncrementBatchLoadCountByStatisticsContext<K> context) {
2749
batchInvokeCount.incrementAndGet();
2850
return batchLoadCount.addAndGet(delta);
2951
}
3052

53+
@Deprecated
3154
@Override
32-
public long incrementCacheHitCount() {
33-
return cacheHitCount.incrementAndGet();
55+
public long incrementBatchLoadCountBy(long delta) {
56+
return incrementBatchLoadCountBy(delta, null);
3457
}
3558

3659
@Override
37-
public long incrementLoadErrorCount() {
38-
return loadErrorCount.incrementAndGet();
60+
public <K> long incrementBatchLoadExceptionCount(IncrementBatchLoadExceptionCountStatisticsContext<K> context) {
61+
return batchLoadExceptionCount.incrementAndGet();
3962
}
4063

64+
@Deprecated
4165
@Override
4266
public long incrementBatchLoadExceptionCount() {
43-
return batchLoadExceptionCount.incrementAndGet();
67+
return incrementBatchLoadExceptionCount(null);
68+
}
69+
70+
@Override
71+
public <K> long incrementCacheHitCount(IncrementCacheHitCountStatisticsContext<K> context) {
72+
return cacheHitCount.incrementAndGet();
73+
}
74+
75+
@Deprecated
76+
@Override
77+
public long incrementCacheHitCount() {
78+
return incrementCacheHitCount(null);
4479
}
4580

4681
@Override

0 commit comments

Comments
 (0)