Reworking MetricServices

smallrye · Aug 2, 2023 · 8018e4c · 8018e4c
1 parent 60f667a
commit 8018e4c
Show file tree

Hide file tree

Showing 21 changed files with 173 additions and 308 deletions.
diff --git a/...r/implementation-cdi/src/main/java/io/smallrye/graphql/cdi/config/MicroProfileConfig.java b/...r/implementation-cdi/src/main/java/io/smallrye/graphql/cdi/config/MicroProfileConfig.java
@@ -6,6 +6,7 @@
 
 import org.eclipse.microprofile.config.ConfigProvider;
 
+import io.smallrye.graphql.config.ConfigKey;
 import io.smallrye.graphql.spi.config.Config;
 import io.smallrye.graphql.spi.config.LogPayloadOption;
 

diff --git a/server/implementation-cdi/src/main/java/io/smallrye/graphql/cdi/event/EventsService.java b/server/implementation-cdi/src/main/java/io/smallrye/graphql/cdi/event/EventsService.java
@@ -9,7 +9,7 @@
 import graphql.GraphQL;
 import graphql.schema.GraphQLSchema;
 import io.smallrye.graphql.api.Context;
-import io.smallrye.graphql.cdi.config.ConfigKey;
+import io.smallrye.graphql.config.ConfigKey;
 import io.smallrye.graphql.execution.event.InvokeInfo;
 import io.smallrye.graphql.execution.event.Priorities;
 import io.smallrye.graphql.schema.model.Operation;

diff --git a/...er/implementation-cdi/src/main/java/io/smallrye/graphql/cdi/metrics/MPMetricsService.java b/...er/implementation-cdi/src/main/java/io/smallrye/graphql/cdi/metrics/MPMetricsService.java
@@ -1,68 +1,37 @@
 package io.smallrye.graphql.cdi.metrics;
 
 import java.time.Duration;
-import java.util.Collections;
-import java.util.IdentityHashMap;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 import jakarta.enterprise.inject.spi.CDI;
 import jakarta.enterprise.util.AnnotationLiteral;
 
-import org.eclipse.microprofile.metrics.Metadata;
 import org.eclipse.microprofile.metrics.MetricRegistry;
-import org.eclipse.microprofile.metrics.MetricType;
 import org.eclipse.microprofile.metrics.Tag;
 import org.eclipse.microprofile.metrics.annotation.RegistryType;
+import org.jboss.logging.Logger;
 
 import io.smallrye.graphql.api.Context;
-import io.smallrye.graphql.cdi.config.ConfigKey;
-import io.smallrye.graphql.schema.model.Operation;
-import io.smallrye.graphql.spi.EventingService;
+import io.smallrye.graphql.spi.MetricsService;
 
 /**
  * Listening for event and create metrics from it. Uses MP Metrics 3.x API.
  *
  * @author Jan Martiska (jmartisk@redhat.com)
  * @author Phillip Kruger (phillip.kruger@redhat.com)
  */
-public class MPMetricsService implements EventingService {
+public class MPMetricsService implements MetricsService {
 
     private MetricRegistry metricRegistry;
-    private final Map<Context, Long> startTimes = Collections.synchronizedMap(new IdentityHashMap<>());
+    private final Map<Long, MetricMeasurement> metricsMemory = new ConcurrentHashMap<>();
     private static final String METRIC_NAME = "mp_graphql";
-    private final String DESCRIPTION = "Call statistics for the operation denoted by the 'name' tag";
+    private Logger LOG = Logger.getLogger(MPMetricsService.class);
 
-    @Override
-    public Operation createOperation(Operation operation) {
-        final Tag[] tags = getTags(operation);
-
-        Metadata metadata = Metadata.builder()
-                .withName(METRIC_NAME)
-                .withType(MetricType.SIMPLE_TIMER)
-                .withDescription(DESCRIPTION)
-                .build();
-        getMetricRegistry().simpleTimer(metadata, tags);
-        return operation;
-    }
-
-    @Override
-    public void beforeDataFetch(Context context) {
-        startTimes.put(context, System.nanoTime());
-    }
-
-    @Override
-    public void afterDataFetch(Context context) {
-        Long startTime = startTimes.remove(context);
-        if (startTime != null) {
-            long duration = System.nanoTime() - startTime;
-            getMetricRegistry().simpleTimer(METRIC_NAME, getTags(context))
-                    .update(Duration.ofNanos(duration));
-        }
-    }
-
-    @Override
-    public String getConfigKey() {
-        return ConfigKey.ENABLE_METRICS;
+    public MPMetricsService() {
+        // If MP Metrics are not available, this will throw an exception
+        // and make sure that this service doesn't get registered
+        getMetricRegistry();
     }
 
     private MetricRegistry getMetricRegistry() {
@@ -72,20 +41,30 @@ private MetricRegistry getMetricRegistry() {
         return metricRegistry;
     }
 
-    private Tag[] getTags(Context context) {
+    private Tag[] getTags(MetricMeasurement metricMeasurement) {
         return new Tag[] {
-                new Tag("name", context.getFieldName()),
-                new Tag("type", context.getOperationType()),
-                new Tag("source", String.valueOf(context.getSource() != null))
+                new Tag("name", metricMeasurement.getName()),
+                new Tag("type", metricMeasurement.getOperationType()),
+                new Tag("source", String.valueOf(metricMeasurement.isSource()))
         };
     }
 
-    private Tag[] getTags(Operation operation) {
-        return new Tag[] {
-                new Tag("name", operation.getName()),
-                new Tag("type", operation.getOperationType().toString()),
-                new Tag("source", String.valueOf(operation.isSourceField()))
-        };
+    @Override
+    public void start(Long measurementId, Context context) {
+        metricsMemory.put(measurementId, new MetricMeasurement(context.getFieldName(),
+                context.hasSource(),
+                context.getOperationType(),
+                System.nanoTime()));
+        LOG.tracef("(" + measurementId + ") Started recording metrics for: %s", context.getFieldName());
+    }
+
+    @Override
+    public void end(Long measurementId) {
+        MetricMeasurement metricMeasurement = metricsMemory.remove(measurementId);
+        long duration = System.nanoTime() - metricMeasurement.getTimeStarted();
+        getMetricRegistry().simpleTimer(METRIC_NAME, getTags(metricMeasurement))
+                .update(Duration.ofNanos(duration));
+        LOG.tracef("(" + measurementId + ") Finished recording metrics for: %s", metricMeasurement.getName());
     }
 
     class VendorType extends AnnotationLiteral<RegistryType> implements RegistryType {

diff --git a/...r/implementation-cdi/src/main/java/io/smallrye/graphql/cdi/metrics/MetricMeasurement.java b/...r/implementation-cdi/src/main/java/io/smallrye/graphql/cdi/metrics/MetricMeasurement.java
@@ -0,0 +1,31 @@
+package io.smallrye.graphql.cdi.metrics;
+
+public class MetricMeasurement {
+    private String name;
+    private boolean source;
+    private String operationType;
+    private long timeStarted;
+
+    public MetricMeasurement(String name, boolean source, String operationType, long timeStarted) {
+        this.name = name;
+        this.source = source;
+        this.operationType = operationType;
+        this.timeStarted = timeStarted;
+    }
+
+    public String getName() {
+        return name;
+    }
+
+    public boolean isSource() {
+        return source;
+    }
+
+    public String getOperationType() {
+        return operationType;
+    }
+
+    public long getTimeStarted() {
+        return timeStarted;
+    }
+}
diff --git a/server/implementation-cdi/src/main/java/io/smallrye/graphql/cdi/metrics/MetricsService.java b/server/implementation-cdi/src/main/java/io/smallrye/graphql/cdi/metrics/MetricsService.java
diff --git a/...mentation-cdi/src/main/java/io/smallrye/graphql/cdi/metrics/MicrometerMetricsService.java b/...mentation-cdi/src/main/java/io/smallrye/graphql/cdi/metrics/MicrometerMetricsService.java
@@ -1,66 +1,52 @@
 package io.smallrye.graphql.cdi.metrics;
 
 import java.time.Duration;
-import java.util.Collections;
-import java.util.IdentityHashMap;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.jboss.logging.Logger;
 
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.Metrics;
 import io.micrometer.core.instrument.Tags;
-import io.micrometer.core.instrument.Timer;
 import io.smallrye.graphql.api.Context;
-import io.smallrye.graphql.cdi.config.ConfigKey;
-import io.smallrye.graphql.schema.model.Operation;
-import io.smallrye.graphql.spi.EventingService;
-
-public class MicrometerMetricsService implements EventingService {
+import io.smallrye.graphql.spi.MetricsService;
 
+public class MicrometerMetricsService implements MetricsService {
     private final MeterRegistry meterRegistry = Metrics.globalRegistry;
-    private final Map<Context, Long> startTimes = Collections.synchronizedMap(new IdentityHashMap<>());
+    private final Map<Long, MetricMeasurement> metricsMemory = new ConcurrentHashMap<>();
     private static final String METRIC_NAME = "mp_graphql";
-    private final String DESCRIPTION = "Call statistics for the operation denoted by the 'name' tag";
+    private Logger LOG = Logger.getLogger(MicrometerMetricsService.class);
 
-    @Override
-    public Operation createOperation(Operation operation) {
-        final Tags tags = getTags(operation);
-        Timer.builder(METRIC_NAME)
-                .tags(tags)
-                .description(DESCRIPTION)
-                .register(meterRegistry);
-        return operation;
+    public MicrometerMetricsService() {
+        // If Micrometer is not available, this will throw an exception
+        // and make sure that this service doesn't get registered
+        meterRegistry.getMeters();
     }
 
-    @Override
-    public void beforeDataFetch(Context context) {
-        startTimes.put(context, System.nanoTime());
+    private Tags getTags(MetricMeasurement metricMeasurement) {
+        return Tags.of("name", metricMeasurement.getName())
+                .and("type", metricMeasurement.getOperationType())
+                .and("source", String.valueOf(metricMeasurement.isSource()));
     }
 
     @Override
-    public void afterDataFetch(Context context) {
-        Long startTime = startTimes.remove(context);
-        if (startTime != null) {
-            long duration = System.nanoTime() - startTime;
-            meterRegistry.timer(METRIC_NAME, getTags(context))
-                    .record(Duration.ofNanos(duration));
-        }
+    public void start(Long measurementId, Context context) {
+        metricsMemory.put(measurementId,
+                new MetricMeasurement(
+                        context.getFieldName(),
+                        context.hasSource(),
+                        context.getOperationType(),
+                        System.nanoTime()));
+        LOG.tracef("(" + measurementId + ") Started recording metrics for: %s", context.getFieldName());
     }
 
     @Override
-    public String getConfigKey() {
-        return ConfigKey.ENABLE_METRICS;
+    public void end(Long measurementId) {
+        MetricMeasurement metricMeasurement = metricsMemory.remove(measurementId);
+        long duration = System.nanoTime() - metricMeasurement.getTimeStarted();
+        meterRegistry.timer(METRIC_NAME, getTags(metricMeasurement))
+                .record(Duration.ofNanos(duration));
+        LOG.tracef("(" + measurementId + ") Finished recording metrics for: %s", metricMeasurement.getName());
     }
-
-    private Tags getTags(Context context) {
-        return Tags.of("name", context.getFieldName())
-                .and("type", context.getOperationType())
-                .and("source", String.valueOf(context.getSource() != null));
-    }
-
-    private Tags getTags(Operation operation) {
-        return Tags.of("name", operation.getName())
-                .and("type", operation.getOperationType().toString())
-                .and("source", String.valueOf(operation.isSourceField()));
-    }
-
 }
diff --git a/server/implementation-cdi/src/main/java/io/smallrye/graphql/cdi/tracing/TracingService.java b/server/implementation-cdi/src/main/java/io/smallrye/graphql/cdi/tracing/TracingService.java
@@ -14,7 +14,7 @@
 import io.opentelemetry.api.trace.Tracer;
 import io.opentelemetry.context.Scope;
 import io.smallrye.graphql.api.Context;
-import io.smallrye.graphql.cdi.config.ConfigKey;
+import io.smallrye.graphql.config.ConfigKey;
 import io.smallrye.graphql.execution.event.Priorities;
 import io.smallrye.graphql.spi.EventingService;
 

diff --git a/...entation-cdi/src/main/resources/META-INF/services/io.smallrye.graphql.spi.EventingService b/...entation-cdi/src/main/resources/META-INF/services/io.smallrye.graphql.spi.EventingService
@@ -1,3 +1,2 @@
 io.smallrye.graphql.cdi.event.EventsService
-io.smallrye.graphql.cdi.metrics.MetricsService
 io.smallrye.graphql.cdi.tracing.TracingService
diff --git a/...mentation-cdi/src/main/resources/META-INF/services/io.smallrye.graphql.spi.MetricsService b/...mentation-cdi/src/main/resources/META-INF/services/io.smallrye.graphql.spi.MetricsService
@@ -0,0 +1,2 @@
+io.smallrye.graphql.cdi.metrics.MicrometerMetricsService
+io.smallrye.graphql.cdi.metrics.MPMetricsService
diff --git a/...mallrye/graphql/cdi/config/ConfigKey.java → ...io/smallrye/graphql/config/ConfigKey.java b/...mallrye/graphql/cdi/config/ConfigKey.java → ...io/smallrye/graphql/config/ConfigKey.java
@@ -1,4 +1,4 @@
-package io.smallrye.graphql.cdi.config;
+package io.smallrye.graphql.config;
 
 /**
  * All the config options available

diff --git a/server/implementation/src/main/java/io/smallrye/graphql/execution/ExecutionService.java b/server/implementation/src/main/java/io/smallrye/graphql/execution/ExecutionService.java
@@ -35,6 +35,7 @@
 import io.smallrye.graphql.execution.error.ExceptionHandler;
 import io.smallrye.graphql.execution.error.UnparseableDocumentException;
 import io.smallrye.graphql.execution.event.EventEmitter;
+import io.smallrye.graphql.execution.metrics.MetricsEmitter;
 import io.smallrye.graphql.schema.model.Operation;
 import io.smallrye.graphql.schema.model.Schema;
 import io.smallrye.graphql.schema.model.Type;
@@ -59,6 +60,7 @@ public class ExecutionService {
     private final Schema schema;
 
     private final EventEmitter eventEmitter = EventEmitter.getInstance();
+    private final MetricsEmitter metricsEmitter = MetricsEmitter.getInstance();
 
     private GraphQL graphQL;
 

diff --git a/...r/implementation/src/main/java/io/smallrye/graphql/execution/context/SmallRyeContext.java b/...r/implementation/src/main/java/io/smallrye/graphql/execution/context/SmallRyeContext.java
@@ -25,6 +25,7 @@
  */
 public class SmallRyeContext implements Context {
     private final String createdBy;
+    private String fetchId;
     private JsonObject request;
     private String executionId;
     private Field field;

diff --git a/...ion/src/main/java/io/smallrye/graphql/execution/datafetcher/AbstractAsyncDataFetcher.java b/...ion/src/main/java/io/smallrye/graphql/execution/datafetcher/AbstractAsyncDataFetcher.java
@@ -59,6 +59,7 @@ protected <O> O invokeAndTransform(
                             te.appendDataFetcherResult(resultBuilder, dfe);
                         } finally {
                             eventEmitter.fireAfterDataFetch(context);
+                            //                            metricsEmitter.end(measurementIds.remove());
                         }
                     }