Skip to content

Commit b54e610

Browse files
committed
refactor: do not rely on DefaultEventHandler so much for EventMonitor
The goal is to not rely on DefaultEventHandler eventually. EventMonitor was kept on DefaultEventHandler for backwards compatibility reason but this should be moved to its own package along with the Metrics class for v2
1 parent c92540e commit b54e610

File tree

4 files changed

+67
-31
lines changed

4 files changed

+67
-31
lines changed

micrometer-support/src/main/java/io/javaoperatorsdk/operator/micrometer/MicrometerMetrics.java

+18-1
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,26 @@
44
import java.util.Map;
55

66
import io.javaoperatorsdk.operator.Metrics;
7-
import io.javaoperatorsdk.operator.Metrics.ControllerExecution;
7+
import io.javaoperatorsdk.operator.processing.DefaultEventHandler.EventMonitor;
8+
import io.javaoperatorsdk.operator.processing.event.Event;
89
import io.micrometer.core.instrument.MeterRegistry;
910
import io.micrometer.core.instrument.Timer;
1011

1112
public class MicrometerMetrics implements Metrics {
1213

1314
public static final String PREFIX = "operator.sdk.";
1415
private final MeterRegistry registry;
16+
private final EventMonitor monitor = new EventMonitor() {
17+
@Override
18+
public void processedEvent(String uid, Event event) {
19+
incrementProcessedEventsNumber();
20+
}
21+
22+
@Override
23+
public void failedEvent(String uid, Event event) {
24+
incrementControllerRetriesNumber();
25+
}
26+
};
1527

1628
public MicrometerMetrics(MeterRegistry registry) {
1729
this.registry = registry;
@@ -63,4 +75,9 @@ public void incrementProcessedEventsNumber() {
6375
public <T extends Map<?, ?>> T monitorSizeOf(T map, String name) {
6476
return registry.gaugeMapSize(PREFIX + name + ".size", Collections.emptyList(), map);
6577
}
78+
79+
@Override
80+
public EventMonitor getEventMonitor() {
81+
return monitor;
82+
}
6683
}

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Metrics.java

+7
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
import java.util.Map;
44

5+
import io.javaoperatorsdk.operator.processing.DefaultEventHandler;
6+
import io.javaoperatorsdk.operator.processing.DefaultEventHandler.EventMonitor;
7+
58
public interface Metrics {
69
Metrics NOOP = new Metrics() {};
710

@@ -27,4 +30,8 @@ default void incrementProcessedEventsNumber() {}
2730
default <T extends Map<?, ?>> T monitorSizeOf(T map, String name) {
2831
return map;
2932
}
33+
34+
default DefaultEventHandler.EventMonitor getEventMonitor() {
35+
return EventMonitor.NOOP;
36+
}
3037
}

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java

-14
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,6 @@
1818
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
1919
import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager;
2020
import io.javaoperatorsdk.operator.processing.ConfiguredController;
21-
import io.javaoperatorsdk.operator.processing.DefaultEventHandler;
22-
import io.javaoperatorsdk.operator.processing.DefaultEventHandler.EventMonitor;
23-
import io.javaoperatorsdk.operator.processing.event.Event;
2421

2522
@SuppressWarnings("rawtypes")
2623
public class Operator implements AutoCloseable {
@@ -32,17 +29,6 @@ public class Operator implements AutoCloseable {
3229
public Operator(KubernetesClient k8sClient, ConfigurationService configurationService) {
3330
this.k8sClient = k8sClient;
3431
this.configurationService = configurationService;
35-
DefaultEventHandler.setEventMonitor(new EventMonitor() {
36-
@Override
37-
public void processedEvent(String uid, Event event) {
38-
configurationService.getMetrics().incrementProcessedEventsNumber();
39-
}
40-
41-
@Override
42-
public void failedEvent(String uid, Event event) {
43-
configurationService.getMetrics().incrementControllerRetriesNumber();
44-
}
45-
});
4632
}
4733

4834
/** Adds a shutdown hook that automatically calls {@link #close()} when the app shuts down. */

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java

+42-16
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.slf4j.LoggerFactory;
1515

1616
import io.fabric8.kubernetes.client.CustomResource;
17+
import io.javaoperatorsdk.operator.Metrics;
1718
import io.javaoperatorsdk.operator.api.RetryInfo;
1819
import io.javaoperatorsdk.operator.api.config.ConfigurationService;
1920
import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager;
@@ -35,13 +36,9 @@
3536
public class DefaultEventHandler<R extends CustomResource<?, ?>> implements EventHandler {
3637

3738
private static final Logger log = LoggerFactory.getLogger(DefaultEventHandler.class);
38-
private static EventMonitor monitor = new EventMonitor() {
39-
@Override
40-
public void processedEvent(String uid, Event event) {}
4139

42-
@Override
43-
public void failedEvent(String uid, Event event) {}
44-
};
40+
@Deprecated
41+
private static EventMonitor monitor = EventMonitor.NOOP;
4542

4643
private final EventBuffer eventBuffer;
4744
private final Set<String> underProcessing = new HashSet<>();
@@ -51,23 +48,25 @@ public void failedEvent(String uid, Event event) {}
5148
private final ExecutorService executor;
5249
private final String controllerName;
5350
private final ReentrantLock lock = new ReentrantLock();
51+
private final EventMonitor eventMonitor;
5452
private volatile boolean running;
5553
private DefaultEventSourceManager<R> eventSourceManager;
5654

5755
public DefaultEventHandler(ConfiguredController<R> controller) {
5856
this(ExecutorServiceManager.instance().executorService(),
5957
controller.getConfiguration().getName(),
6058
new EventDispatcher<>(controller),
61-
GenericRetry.fromConfiguration(controller.getConfiguration().getRetryConfiguration()));
59+
GenericRetry.fromConfiguration(controller.getConfiguration().getRetryConfiguration()),
60+
controller.getConfiguration().getConfigurationService().getMetrics().getEventMonitor());
6261
}
6362

6463
DefaultEventHandler(EventDispatcher<R> eventDispatcher, String relatedControllerName,
6564
Retry retry) {
66-
this(null, relatedControllerName, eventDispatcher, retry);
65+
this(null, relatedControllerName, eventDispatcher, retry, null);
6766
}
6867

6968
private DefaultEventHandler(ExecutorService executor, String relatedControllerName,
70-
EventDispatcher<R> eventDispatcher, Retry retry) {
69+
EventDispatcher<R> eventDispatcher, Retry retry, EventMonitor monitor) {
7170
this.running = true;
7271
this.executor =
7372
executor == null
@@ -78,14 +77,44 @@ private DefaultEventHandler(ExecutorService executor, String relatedControllerNa
7877
this.eventDispatcher = eventDispatcher;
7978
this.retry = retry;
8079
this.eventBuffer = new EventBuffer();
80+
this.eventMonitor = monitor != null ? monitor : EventMonitor.NOOP;
81+
}
82+
83+
public void setEventSourceManager(DefaultEventSourceManager<R> eventSourceManager) {
84+
this.eventSourceManager = eventSourceManager;
8185
}
8286

87+
/**
88+
* @deprecated the EventMonitor to be used should now be retrieved from
89+
* {@link Metrics#getEventMonitor()}
90+
* @param monitor
91+
*/
92+
@Deprecated
8393
public static void setEventMonitor(EventMonitor monitor) {
8494
DefaultEventHandler.monitor = monitor;
8595
}
8696

87-
public void setEventSourceManager(DefaultEventSourceManager<R> eventSourceManager) {
88-
this.eventSourceManager = eventSourceManager;
97+
/*
98+
* TODO: promote this interface to top-level, probably create a `monitoring` package?
99+
*/
100+
public interface EventMonitor {
101+
EventMonitor NOOP = new EventMonitor() {
102+
@Override
103+
public void processedEvent(String uid, Event event) {}
104+
105+
@Override
106+
public void failedEvent(String uid, Event event) {}
107+
};
108+
109+
void processedEvent(String uid, Event event);
110+
111+
void failedEvent(String uid, Event event);
112+
}
113+
114+
private EventMonitor monitor() {
115+
// todo: remove us of static monitor, only here for backwards compatibility
116+
return DefaultEventHandler.monitor != EventMonitor.NOOP ? DefaultEventHandler.monitor
117+
: eventMonitor;
89118
}
90119

91120
@Override
@@ -102,6 +131,7 @@ public void handleEvent(Event event) {
102131
log.debug("Received event: {}", event);
103132

104133
final Predicate<CustomResource> selector = event.getCustomResourcesSelector();
134+
final var monitor = monitor();
105135
for (String uid : eventSourceManager.getLatestResourceUids(selector)) {
106136
eventBuffer.addEvent(uid, event);
107137
monitor.processedEvent(uid, event);
@@ -168,6 +198,7 @@ void eventProcessingFinished(
168198

169199
if (retry != null && postExecutionControl.exceptionDuringExecution()) {
170200
handleRetryOnException(executionScope);
201+
final var monitor = monitor();
171202
executionScope.getEvents()
172203
.forEach(e -> monitor.failedEvent(executionScope.getCustomResourceUid(), e));
173204
return;
@@ -296,11 +327,6 @@ private void unsetUnderExecution(String customResourceUid) {
296327
underProcessing.remove(customResourceUid);
297328
}
298329

299-
public interface EventMonitor {
300-
void processedEvent(String uid, Event event);
301-
302-
void failedEvent(String uid, Event event);
303-
}
304330

305331
private class ControllerExecution implements Runnable {
306332
private final ExecutionScope<R> executionScope;

0 commit comments

Comments
 (0)