Skip to content

Monitoring improvements #616

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 13 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package io.javaoperatorsdk.operator.micrometer;
package io.javaoperatorsdk.operator.monitoring.micrometer;

import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;

import io.javaoperatorsdk.operator.Metrics;
import io.javaoperatorsdk.operator.processing.DefaultEventHandler.EventMonitor;
import io.javaoperatorsdk.operator.processing.event.CustomResourceID;
import io.javaoperatorsdk.operator.api.monitoring.Metrics;
import io.javaoperatorsdk.operator.processing.event.Event;
import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.Timer;
Expand All @@ -14,17 +14,6 @@ public class MicrometerMetrics implements Metrics {

public static final String PREFIX = "operator.sdk.";
private final MeterRegistry registry;
private final EventMonitor monitor = new EventMonitor() {
@Override
public void processedEvent(CustomResourceID uid, Event event) {
incrementProcessedEventsNumber();
}

@Override
public void failedEvent(CustomResourceID uid, Event event) {
incrementControllerRetriesNumber();
}
};

public MicrometerMetrics(MeterRegistry registry) {
this.registry = registry;
Expand Down Expand Up @@ -64,21 +53,37 @@ public void incrementControllerRetriesNumber() {

}

public void incrementProcessedEventsNumber() {
registry
.counter(
PREFIX + "total.events.received", "events", "totalEvents", "type",
"eventsReceived")
.increment();
public void processingEvent(Event event) {
incrementCounter(event, "events.received");
}

public void processedEvent(Event event) {
incrementCounter(event, "events.processed");
}

public void failedEvent(Event event, RuntimeException exception) {
var cause = exception.getCause();
if (cause == null) {
cause = exception;
} else if (cause instanceof RuntimeException) {
cause = cause.getCause() != null ? cause.getCause() : cause;
}
incrementCounter(event, "events.failed", "exception", cause.getClass().getSimpleName());
}

public <T extends Map<?, ?>> T monitorSizeOf(T map, String name) {
return registry.gaugeMapSize(PREFIX + name + ".size", Collections.emptyList(), map);
}

@Override
public EventMonitor getEventMonitor() {
return monitor;
private void incrementCounter(Event event, String counterName, String... additionalTags) {
final var id = event.getRelatedCustomResourceID();
var tags = List.of("namespace", id.getNamespace().orElse(""),
"scope", id.getNamespace().isPresent() ? "namespace" : "cluster",
"type", event.getType().name());
if (additionalTags != null && additionalTags.length > 0) {
tags = new LinkedList<>(tags);
tags.addAll(List.of(additionalTags));
}
registry.counter(PREFIX + counterName, tags.toArray(new String[0])).increment();
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@

import io.fabric8.kubernetes.client.Config;
import io.fabric8.kubernetes.client.CustomResource;
import io.javaoperatorsdk.operator.Metrics;
import io.javaoperatorsdk.operator.api.ResourceController;
import io.javaoperatorsdk.operator.api.monitoring.Metrics;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

import io.fabric8.kubernetes.client.Config;
import io.fabric8.kubernetes.client.CustomResource;
import io.javaoperatorsdk.operator.Metrics;
import io.javaoperatorsdk.operator.api.ResourceController;
import io.javaoperatorsdk.operator.api.monitoring.Metrics;

public class ConfigurationServiceOverrider {
private final ConfigurationService original;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
package io.javaoperatorsdk.operator;
package io.javaoperatorsdk.operator.api.monitoring;

import java.util.Map;

import io.javaoperatorsdk.operator.processing.DefaultEventHandler;
import io.javaoperatorsdk.operator.processing.DefaultEventHandler.EventMonitor;
import io.javaoperatorsdk.operator.processing.event.Event;

public interface Metrics {
Metrics NOOP = new Metrics() {};

default void processingEvent(Event event) {}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really what we should measure? Isn't that the number of reconciliations in this case actually. Since we tried to move away from events.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it measures the number of reconciliation requests. The thinking was that it might be interesting to see how many reconciliation requests happened vs. the number of reconciliations that actually went through (processedEvent).

Copy link
Collaborator

@csviri csviri Oct 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we rename the method accordingly? Like reconcilingStarted, reconcilingFinished, reconcilingFailed ?


default void processedEvent(Event event) {}

default void failedEvent(Event event, RuntimeException exception) {}


interface ControllerExecution<T> {
String name();
Expand All @@ -23,15 +28,7 @@ default <T> T timeControllerExecution(ControllerExecution<T> execution) {
return execution.execute();
}

default void incrementControllerRetriesNumber() {}

default void incrementProcessedEventsNumber() {}

default <T extends Map<?, ?>> T monitorSizeOf(T map, String name) {
return map;
}

default DefaultEventHandler.EventMonitor getEventMonitor() {
return EventMonitor.NOOP;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@
import io.fabric8.kubernetes.client.dsl.MixedOperation;
import io.fabric8.kubernetes.client.dsl.Resource;
import io.javaoperatorsdk.operator.CustomResourceUtils;
import io.javaoperatorsdk.operator.Metrics.ControllerExecution;
import io.javaoperatorsdk.operator.MissingCRDException;
import io.javaoperatorsdk.operator.OperatorException;
import io.javaoperatorsdk.operator.api.Context;
import io.javaoperatorsdk.operator.api.DeleteControl;
import io.javaoperatorsdk.operator.api.ResourceController;
import io.javaoperatorsdk.operator.api.UpdateControl;
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
import io.javaoperatorsdk.operator.api.monitoring.Metrics.ControllerExecution;
import io.javaoperatorsdk.operator.processing.event.DefaultEventSourceManager;
import io.javaoperatorsdk.operator.processing.event.EventSourceManager;

Expand Down
Loading