Skip to content

Commit dfe8474

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 3fc60c3 commit dfe8474

File tree

4 files changed

+59
-24
lines changed

4 files changed

+59
-24
lines changed

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

Lines changed: 18 additions & 1 deletion
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

Lines changed: 7 additions & 0 deletions
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

Lines changed: 0 additions & 14 deletions
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

Lines changed: 34 additions & 9 deletions
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,22 +48,24 @@ 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 DefaultEventSourceManager<R> eventSourceManager;
5553

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

6362
DefaultEventHandler(EventDispatcher<R> eventDispatcher, String relatedControllerName,
6463
Retry retry) {
65-
this(null, relatedControllerName, eventDispatcher, retry);
64+
this(null, relatedControllerName, eventDispatcher, retry, null);
6665
}
6766

6867
private DefaultEventHandler(ExecutorService executor, String relatedControllerName,
69-
EventDispatcher<R> eventDispatcher, Retry retry) {
68+
EventDispatcher<R> eventDispatcher, Retry retry, EventMonitor monitor) {
7069
this.executor =
7170
executor == null
7271
? new ScheduledThreadPoolExecutor(
@@ -76,29 +75,54 @@ private DefaultEventHandler(ExecutorService executor, String relatedControllerNa
7675
this.eventDispatcher = eventDispatcher;
7776
this.retry = retry;
7877
eventBuffer = new EventBuffer();
78+
this.eventMonitor = monitor != null ? monitor : EventMonitor.NOOP;
7979
}
8080

8181
public void setEventSourceManager(DefaultEventSourceManager<R> eventSourceManager) {
8282
this.eventSourceManager = eventSourceManager;
8383
}
8484

85+
/**
86+
* @deprecated the EventMonitor to be used should now be retrieved from
87+
* {@link Metrics#getEventMonitor()}
88+
* @param monitor
89+
*/
90+
@Deprecated
8591
public static void setEventMonitor(EventMonitor monitor) {
8692
DefaultEventHandler.monitor = monitor;
8793
}
8894

95+
/*
96+
* TODO: promote this interface to top-level, probably create a `monitoring` package?
97+
*/
8998
public interface EventMonitor {
99+
EventMonitor NOOP = new EventMonitor() {
100+
@Override
101+
public void processedEvent(String uid, Event event) {}
102+
103+
@Override
104+
public void failedEvent(String uid, Event event) {}
105+
};
106+
90107
void processedEvent(String uid, Event event);
91108

92109
void failedEvent(String uid, Event event);
93110
}
94111

112+
private EventMonitor monitor() {
113+
// todo: remove us of static monitor, only here for backwards compatibility
114+
return DefaultEventHandler.monitor != EventMonitor.NOOP ? DefaultEventHandler.monitor
115+
: eventMonitor;
116+
}
117+
95118
@Override
96119
public void handleEvent(Event event) {
97120
try {
98121
lock.lock();
99122
log.debug("Received event: {}", event);
100123

101124
final Predicate<CustomResource> selector = event.getCustomResourcesSelector();
125+
final var monitor = monitor();
102126
for (String uid : eventSourceManager.getLatestResourceUids(selector)) {
103127
eventBuffer.addEvent(uid, event);
104128
monitor.processedEvent(uid, event);
@@ -151,6 +175,7 @@ void eventProcessingFinished(
151175

152176
if (retry != null && postExecutionControl.exceptionDuringExecution()) {
153177
handleRetryOnException(executionScope);
178+
final var monitor = monitor();
154179
executionScope.getEvents()
155180
.forEach(e -> monitor.failedEvent(executionScope.getCustomResourceUid(), e));
156181
return;

0 commit comments

Comments
 (0)