From e3ea275294e0bba6512be6ebec6e9cbea77efd02 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 2 Dec 2022 15:14:43 +0100 Subject: [PATCH] feat: metrics contains the whole resource --- .../micrometer/MicrometerMetrics.java | 14 +++-- .../operator/api/monitoring/Metrics.java | 58 ++++++++----------- .../processing/event/EventProcessor.java | 54 ++++++++--------- .../processing/event/EventProcessorTest.java | 2 +- 4 files changed, 59 insertions(+), 69 deletions(-) diff --git a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java index fff9cf427f..fb7d064c26 100644 --- a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java +++ b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java @@ -6,6 +6,7 @@ import java.util.Map; import java.util.Optional; +import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.api.reconciler.Constants; @@ -84,10 +85,10 @@ public void cleanupDoneFor(ResourceID resourceID, Map metadata) } @Override - public void reconcileCustomResource(ResourceID resourceID, RetryInfo retryInfoNullable, + public void reconcileCustomResource(HasMetadata resource, RetryInfo retryInfoNullable, Map metadata) { Optional retryInfo = Optional.ofNullable(retryInfoNullable); - incrementCounter(resourceID, RECONCILIATIONS + "started", + incrementCounter(ResourceID.fromResource(resource), RECONCILIATIONS + "started", metadata, RECONCILIATIONS + "retries.number", "" + retryInfo.map(RetryInfo::getAttemptCount).orElse(0), @@ -96,11 +97,11 @@ public void reconcileCustomResource(ResourceID resourceID, RetryInfo retryInfoNu } @Override - public void finishedReconciliation(ResourceID resourceID, Map metadata) { - incrementCounter(resourceID, RECONCILIATIONS + "success", metadata); + public void finishedReconciliation(HasMetadata resource, Map metadata) { + incrementCounter(ResourceID.fromResource(resource), RECONCILIATIONS + "success", metadata); } - public void failedReconciliation(ResourceID resourceID, Exception exception, + public void failedReconciliation(HasMetadata resource, Exception exception, Map metadata) { var cause = exception.getCause(); if (cause == null) { @@ -108,7 +109,8 @@ public void failedReconciliation(ResourceID resourceID, Exception exception, } else if (cause instanceof RuntimeException) { cause = cause.getCause() != null ? cause.getCause() : cause; } - incrementCounter(resourceID, RECONCILIATIONS + "failed", metadata, "exception", + incrementCounter(ResourceID.fromResource(resource), RECONCILIATIONS + "failed", metadata, + "exception", cause.getClass().getSimpleName()); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/Metrics.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/Metrics.java index be608b098e..31ffda96f8 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/Metrics.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/Metrics.java @@ -29,56 +29,39 @@ public interface Metrics { */ default void receivedEvent(Event event, Map metadata) {} - /** - * - * @deprecated Use (and implement) {@link #receivedEvent(Event, Map)} instead - */ - @Deprecated - default void receivedEvent(Event event) { - receivedEvent(event, Collections.emptyMap()); - } - - /** - * - * @deprecated Use (and implement) {@link #reconcileCustomResource(ResourceID, RetryInfo, Map)} - * instead - */ - @Deprecated - default void reconcileCustomResource(ResourceID resourceID, RetryInfo retryInfo) { - reconcileCustomResource(resourceID, retryInfo, Collections.emptyMap()); - } + @Deprecated(forRemoval = true) + default void reconcileCustomResource(ResourceID resourceID, RetryInfo retryInfo, + Map metadata) {} /** * Called right before a resource is dispatched to the ExecutorService for reconciliation. * - * @param resourceID the {@link ResourceID} associated with the resource + * @param resource the associated with the resource * @param retryInfo the current retry state information for the reconciliation request * @param metadata metadata associated with the resource being processed */ - default void reconcileCustomResource(ResourceID resourceID, RetryInfo retryInfo, - Map metadata) {} - - /** - * - * @deprecated Use (and implement) {@link #failedReconciliation(ResourceID, Exception, Map)} - * instead - */ - @Deprecated - default void failedReconciliation(ResourceID resourceID, Exception exception) { - failedReconciliation(resourceID, exception, Collections.emptyMap()); + default void reconcileCustomResource(HasMetadata resource, RetryInfo retryInfo, + Map metadata) { + reconcileCustomResource(ResourceID.fromResource(resource), retryInfo, metadata); } + @Deprecated(forRemoval = true) + default void failedReconciliation(ResourceID resourceID, Exception exception, + Map metadata) {} + /** * Called when a precedent reconciliation for the resource associated with the specified * {@link ResourceID} resulted in the provided exception, resulting in a retry of the * reconciliation. * - * @param resourceID the {@link ResourceID} associated with the resource being processed + * @param resource the {@link ResourceID} associated with the resource being processed * @param exception the exception that caused the failed reconciliation resulting in a retry * @param metadata metadata associated with the resource being processed */ - default void failedReconciliation(ResourceID resourceID, Exception exception, - Map metadata) {} + default void failedReconciliation(HasMetadata resource, Exception exception, + Map metadata) { + failedReconciliation(ResourceID.fromResource(resource), exception, metadata); + } /** * @@ -107,16 +90,21 @@ default void finishedReconciliation(ResourceID resourceID) { finishedReconciliation(resourceID, Collections.emptyMap()); } + @Deprecated(forRemoval = true) + default void finishedReconciliation(ResourceID resourceID, Map metadata) {} + /** * Called when the * {@link io.javaoperatorsdk.operator.api.reconciler.Reconciler#reconcile(HasMetadata, Context)} * method of the Reconciler associated with the resource associated with the specified * {@link ResourceID} has sucessfully finished. * - * @param resourceID the {@link ResourceID} associated with the resource being processed + * @param resource the {@link ResourceID} associated with the resource being processed * @param metadata metadata associated with the resource being processed */ - default void finishedReconciliation(ResourceID resourceID, Map metadata) {} + default void finishedReconciliation(HasMetadata resource, Map metadata) { + finishedReconciliation(ResourceID.fromResource(resource), metadata); + } /** * Encapsulates the information about a controller execution i.e. a call to either diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java index e7a600cf2c..57679f48ca 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java @@ -32,25 +32,25 @@ import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getName; -public class EventProcessor implements EventHandler, LifecycleAware { +public class EventProcessor

implements EventHandler, LifecycleAware { private static final Logger log = LoggerFactory.getLogger(EventProcessor.class); private static final long MINIMAL_RATE_LIMIT_RESCHEDULE_DURATION = 50; private volatile boolean running; private final ControllerConfiguration controllerConfiguration; - private final ReconciliationDispatcher reconciliationDispatcher; + private final ReconciliationDispatcher

reconciliationDispatcher; private final Retry retry; private final ExecutorService executor; private final Metrics metrics; - private final Cache cache; - private final EventSourceManager eventSourceManager; + private final Cache

cache; + private final EventSourceManager

eventSourceManager; private final RateLimiter rateLimiter; private final ResourceStateManager resourceStateManager = new ResourceStateManager(); private final Map metricsMetadata; - public EventProcessor(EventSourceManager eventSourceManager) { + public EventProcessor(EventSourceManager

eventSourceManager) { this( eventSourceManager.getController().getConfiguration(), eventSourceManager.getControllerResourceEventSource(), @@ -63,8 +63,8 @@ public EventProcessor(EventSourceManager eventSourceManager) { @SuppressWarnings("rawtypes") EventProcessor( ControllerConfiguration controllerConfiguration, - ReconciliationDispatcher reconciliationDispatcher, - EventSourceManager eventSourceManager, + ReconciliationDispatcher

reconciliationDispatcher, + EventSourceManager

eventSourceManager, Metrics metrics) { this( controllerConfiguration, @@ -78,11 +78,11 @@ public EventProcessor(EventSourceManager eventSourceManager) { @SuppressWarnings({"rawtypes", "unchecked"}) private EventProcessor( ControllerConfiguration controllerConfiguration, - Cache cache, + Cache

cache, ExecutorService executor, - ReconciliationDispatcher reconciliationDispatcher, + ReconciliationDispatcher

reconciliationDispatcher, Metrics metrics, - EventSourceManager eventSourceManager) { + EventSourceManager

eventSourceManager) { this.controllerConfiguration = controllerConfiguration; this.running = false; this.executor = @@ -136,7 +136,7 @@ private void submitReconciliationExecution(ResourceState state) { try { boolean controllerUnderExecution = isControllerUnderExecution(state); final var resourceID = state.getId(); - Optional maybeLatest = cache.get(resourceID); + Optional

maybeLatest = cache.get(resourceID); maybeLatest.ifPresent(MDCUtils::addResourceInfo); if (!controllerUnderExecution && maybeLatest.isPresent()) { var rateLimit = state.getRateLimit(); @@ -151,9 +151,9 @@ private void submitReconciliationExecution(ResourceState state) { } state.setUnderProcessing(true); final var latest = maybeLatest.get(); - ExecutionScope executionScope = new ExecutionScope<>(latest, state.getRetry()); + ExecutionScope

executionScope = new ExecutionScope<>(latest, state.getRetry()); state.unMarkEventReceived(); - metrics.reconcileCustomResource(resourceID, state.getRetry(), metricsMetadata); + metrics.reconcileCustomResource(latest, state.getRetry(), metricsMetadata); log.debug("Executing events for custom resource. Scope: {}", executionScope); executor.execute(new ReconcilerExecutor(executionScope)); } else { @@ -221,7 +221,7 @@ private void handleRateLimitedSubmission(ResourceID resourceID, Duration minimal } synchronized void eventProcessingFinished( - ExecutionScope executionScope, PostExecutionControl postExecutionControl) { + ExecutionScope

executionScope, PostExecutionControl

postExecutionControl) { if (!running) { return; } @@ -244,7 +244,7 @@ synchronized void eventProcessingFinished( return; } cleanupOnSuccessfulExecution(executionScope); - metrics.finishedReconciliation(resourceID, metricsMetadata); + metrics.finishedReconciliation(executionScope.getResource(), metricsMetadata); if (state.deleteEventPresent()) { cleanupForDeletedEvent(executionScope.getResourceID()); } else if (postExecutionControl.isFinalizerRemoved()) { @@ -253,12 +253,12 @@ synchronized void eventProcessingFinished( postExecutionControl .getUpdatedCustomResource() .ifPresent( - r -> { + p -> { if (!postExecutionControl.updateIsStatusPatch()) { eventSourceManager .getControllerResourceEventSource() .handleRecentResourceUpdate( - ResourceID.fromResource(r), r, executionScope.getResource()); + ResourceID.fromResource(p), p, executionScope.getResource()); } }); if (state.eventPresent()) { @@ -270,7 +270,7 @@ synchronized void eventProcessingFinished( } private void reScheduleExecutionIfInstructed( - PostExecutionControl postExecutionControl, R customResource) { + PostExecutionControl

postExecutionControl, P customResource) { postExecutionControl .getReScheduleDelay() @@ -281,7 +281,7 @@ private void reScheduleExecutionIfInstructed( }, () -> scheduleExecutionForMaxReconciliationInterval(customResource)); } - private void scheduleExecutionForMaxReconciliationInterval(R customResource) { + private void scheduleExecutionForMaxReconciliationInterval(P customResource) { this.controllerConfiguration .maxReconciliationInterval() .ifPresent(m -> { @@ -294,7 +294,7 @@ private void scheduleExecutionForMaxReconciliationInterval(R customResource) { }); } - TimerEventSource retryEventSource() { + TimerEventSource

retryEventSource() { return eventSourceManager.retryEventSource(); } @@ -304,7 +304,7 @@ TimerEventSource retryEventSource() { * according to the retry timing if there was an exception. */ private void handleRetryOnException( - ExecutionScope executionScope, Exception exception) { + ExecutionScope

executionScope, Exception exception) { final var state = getOrInitRetryExecution(executionScope); var resourceID = state.getId(); boolean eventPresent = state.eventPresent(); @@ -323,7 +323,7 @@ private void handleRetryOnException( "Scheduling timer event for retry with delay:{} for resource: {}", delay, resourceID); - metrics.failedReconciliation(resourceID, exception, metricsMetadata); + metrics.failedReconciliation(executionScope.getResource(), exception, metricsMetadata); retryEventSource().scheduleOnce(resourceID, delay); }, () -> { @@ -332,7 +332,7 @@ private void handleRetryOnException( }); } - private void cleanupOnSuccessfulExecution(ExecutionScope executionScope) { + private void cleanupOnSuccessfulExecution(ExecutionScope

executionScope) { log.debug( "Cleanup for successful execution for resource: {}", getName(executionScope.getResource())); if (isRetryConfigured()) { @@ -341,7 +341,7 @@ private void cleanupOnSuccessfulExecution(ExecutionScope executionScope) { retryEventSource().cancelOnceSchedule(executionScope.getResourceID()); } - private ResourceState getOrInitRetryExecution(ExecutionScope executionScope) { + private ResourceState getOrInitRetryExecution(ExecutionScope

executionScope) { final var state = resourceStateManager.getOrCreate(executionScope.getResourceID()); RetryExecution retryExecution = state.getRetry(); if (retryExecution == null) { @@ -387,9 +387,9 @@ private void handleAlreadyMarkedEvents() { } private class ReconcilerExecutor implements Runnable { - private final ExecutionScope executionScope; + private final ExecutionScope

executionScope; - private ReconcilerExecutor(ExecutionScope executionScope) { + private ReconcilerExecutor(ExecutionScope

executionScope) { this.executionScope = executionScope; } @@ -401,7 +401,7 @@ public void run() { try { MDCUtils.addResourceInfo(executionScope.getResource()); thread.setName("ReconcilerExecutor-" + controllerName() + "-" + thread.getId()); - PostExecutionControl postExecutionControl = + PostExecutionControl

postExecutionControl = reconciliationDispatcher.handleExecution(executionScope); eventProcessingFinished(executionScope, postExecutionControl); } finally { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java index 23d5961015..23b3f5c51c 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java @@ -277,7 +277,7 @@ void startProcessedMarkedEventReceivedBefore() { eventProcessor.start(); verify(reconciliationDispatcherMock, timeout(100).times(1)).handleExecution(any()); - verify(metricsMock, times(1)).reconcileCustomResource(any(), isNull(), any()); + verify(metricsMock, times(1)).reconcileCustomResource(any(HasMetadata.class), isNull(), any()); } @Test