diff --git a/algo/src/main/java/org/neo4j/gds/algorithms/community/BasicAlgorithmRunner.java b/algo/src/main/java/org/neo4j/gds/algorithms/community/BasicAlgorithmRunner.java index ff14781ff8..85f1ea5878 100644 --- a/algo/src/main/java/org/neo4j/gds/algorithms/community/BasicAlgorithmRunner.java +++ b/algo/src/main/java/org/neo4j/gds/algorithms/community/BasicAlgorithmRunner.java @@ -115,15 +115,16 @@ public , R, C extends AlgoBaseConfig> AlgorithmComputatio ); // run the algorithm - try { - algorithmMetricsService.started(algorithmFactory.taskName()); + var algorithmMetric = algorithmMetricsService.create(algorithmFactory.taskName()); + try(algorithmMetric) { + algorithmMetric.start(); var algorithmResult = algorithm.compute(); return AlgorithmComputationResult.of(algorithmResult, graph, graphStore, algorithm.getTerminationFlag()); } catch (Exception e) { log.warn("Computation failed", e); algorithm.getProgressTracker().endSubTaskWithFailure(); - algorithmMetricsService.failed(algorithmFactory.taskName()); + algorithmMetric.failed(); throw e; } } diff --git a/algo/src/test/java/org/neo4j/gds/algorithms/community/BasicAlgorithmRunnerTest.java b/algo/src/test/java/org/neo4j/gds/algorithms/community/BasicAlgorithmRunnerTest.java index 9edf4c83fe..1c4206391f 100644 --- a/algo/src/test/java/org/neo4j/gds/algorithms/community/BasicAlgorithmRunnerTest.java +++ b/algo/src/test/java/org/neo4j/gds/algorithms/community/BasicAlgorithmRunnerTest.java @@ -24,6 +24,7 @@ import org.neo4j.gds.Algorithm; import org.neo4j.gds.GraphAlgorithmFactory; import org.neo4j.gds.algorithms.AlgorithmMemoryValidationService; +import org.neo4j.gds.algorithms.metrics.AlgorithmMetric; import org.neo4j.gds.algorithms.metrics.AlgorithmMetricsService; import org.neo4j.gds.api.DatabaseId; import org.neo4j.gds.api.Graph; @@ -40,6 +41,7 @@ import static org.assertj.core.api.Assertions.assertThatException; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.RETURNS_DEEP_STUBS; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -58,7 +60,9 @@ void shouldRegisterAlgorithmMetricCountForSuccess() { when(graphStoreCatalogServiceMock.getGraphWithGraphStore(any(), any(), any(), any(), any())) .thenReturn(Pair.of(graphMock, mock(GraphStore.class))); + var algorithmMetricMock = mock(AlgorithmMetric.class); var algorithmMetricsServiceMock = mock(AlgorithmMetricsService.class); + when(algorithmMetricsServiceMock.create(anyString())).thenReturn(algorithmMetricMock); var logMock = mock(Log.class); when(logMock.getNeo4jLog()).thenReturn(Neo4jProxy.testLog()); @@ -87,9 +91,14 @@ void shouldRegisterAlgorithmMetricCountForSuccess() { DatabaseId.EMPTY ); - verify(algorithmMetricsServiceMock, times(1)).started("TestingMetrics"); - verify(algorithmMetricsServiceMock, times(0)).failed("TestingMetrics"); - verifyNoMoreInteractions(algorithmMetricsServiceMock); + verify(algorithmMetricsServiceMock, times(1)).create("TestingMetrics"); + verify(algorithmMetricMock, times(1)).start(); + verify(algorithmMetricMock, times(1)).close(); + verify(algorithmMetricMock, times(0)).failed(); + verifyNoMoreInteractions( + algorithmMetricsServiceMock, + algorithmMetricMock + ); } @@ -102,7 +111,9 @@ void shouldRegisterAlgorithmMetricCountForFailure() { when(graphStoreCatalogServiceMock.getGraphWithGraphStore(any(), any(), any(), any(), any())) .thenReturn(Pair.of(graphMock, mock(GraphStore.class))); + var algorithmMetricMock = mock(AlgorithmMetric.class); var algorithmMetricsServiceMock = mock(AlgorithmMetricsService.class); + when(algorithmMetricsServiceMock.create(anyString())).thenReturn(algorithmMetricMock); var logMock = mock(Log.class); when(logMock.getNeo4jLog()).thenReturn(Neo4jProxy.testLog()); @@ -134,9 +145,14 @@ void shouldRegisterAlgorithmMetricCountForFailure() { ) ).withMessage("Ooops"); - verify(algorithmMetricsServiceMock, times(1)).started("TestingMetrics"); - verify(algorithmMetricsServiceMock, times(1)).failed("TestingMetrics"); - verifyNoMoreInteractions(algorithmMetricsServiceMock); + verify(algorithmMetricsServiceMock, times(1)).create("TestingMetrics"); + verify(algorithmMetricMock, times(1)).start(); + verify(algorithmMetricMock, times(1)).close(); + verify(algorithmMetricMock, times(1)).failed(); + verifyNoMoreInteractions( + algorithmMetricsServiceMock, + algorithmMetricMock + ); } } diff --git a/algorithm-metrics-api/src/main/java/org/neo4j/gds/algorithms/metrics/AlgorithmMetric.java b/algorithm-metrics-api/src/main/java/org/neo4j/gds/algorithms/metrics/AlgorithmMetric.java new file mode 100644 index 0000000000..abe76bba13 --- /dev/null +++ b/algorithm-metrics-api/src/main/java/org/neo4j/gds/algorithms/metrics/AlgorithmMetric.java @@ -0,0 +1,36 @@ +/* + * Copyright (c) "Neo4j" + * Neo4j Sweden AB [http://neo4j.com] + * + * This file is part of Neo4j. + * + * Neo4j is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.neo4j.gds.algorithms.metrics; + +public abstract class AlgorithmMetric implements AutoCloseable { + + protected final String algorithm; + + protected AlgorithmMetric(String algorithm) { + this.algorithm = algorithm; + } + + public abstract void start(); + + public abstract void failed(); + + @Override + public abstract void close(); +} diff --git a/algorithm-metrics-api/src/main/java/org/neo4j/gds/algorithms/metrics/AlgorithmMetricRegistrar.java b/algorithm-metrics-api/src/main/java/org/neo4j/gds/algorithms/metrics/AlgorithmMetricRegistrar.java index 9f98c0ed99..0f793c4655 100644 --- a/algorithm-metrics-api/src/main/java/org/neo4j/gds/algorithms/metrics/AlgorithmMetricRegistrar.java +++ b/algorithm-metrics-api/src/main/java/org/neo4j/gds/algorithms/metrics/AlgorithmMetricRegistrar.java @@ -21,6 +21,6 @@ public interface AlgorithmMetricRegistrar { - void started(String algorithm); - void failed(String algorithm); + AlgorithmMetric create(String algorithm); + } diff --git a/algorithm-metrics-api/src/main/java/org/neo4j/gds/algorithms/metrics/AlgorithmMetricsService.java b/algorithm-metrics-api/src/main/java/org/neo4j/gds/algorithms/metrics/AlgorithmMetricsService.java index 2dc6c24c0a..fc6350364c 100644 --- a/algorithm-metrics-api/src/main/java/org/neo4j/gds/algorithms/metrics/AlgorithmMetricsService.java +++ b/algorithm-metrics-api/src/main/java/org/neo4j/gds/algorithms/metrics/AlgorithmMetricsService.java @@ -27,12 +27,7 @@ public AlgorithmMetricsService(AlgorithmMetricRegistrar metricRegistrar) { this.metricRegistrar = metricRegistrar; } - public void started(String algorithm) { - metricRegistrar.started(algorithm); + public AlgorithmMetric create(String algorithm) { + return metricRegistrar.create(algorithm); } - - public void failed(String algorithm) { - metricRegistrar.failed(algorithm); - } - } diff --git a/algorithm-metrics-api/src/main/java/org/neo4j/gds/algorithms/metrics/PassthroughAlgorithmMetric.java b/algorithm-metrics-api/src/main/java/org/neo4j/gds/algorithms/metrics/PassthroughAlgorithmMetric.java new file mode 100644 index 0000000000..597d809a08 --- /dev/null +++ b/algorithm-metrics-api/src/main/java/org/neo4j/gds/algorithms/metrics/PassthroughAlgorithmMetric.java @@ -0,0 +1,36 @@ +/* + * Copyright (c) "Neo4j" + * Neo4j Sweden AB [http://neo4j.com] + * + * This file is part of Neo4j. + * + * Neo4j is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.neo4j.gds.algorithms.metrics; + +public final class PassthroughAlgorithmMetric extends AlgorithmMetric { + + PassthroughAlgorithmMetric(String algorithm) { + super(algorithm); + } + + @Override + public void start() {} + + @Override + public void failed() {} + + @Override + public void close() {} +} diff --git a/algorithm-metrics-api/src/main/java/org/neo4j/gds/algorithms/metrics/PassthroughAlgorithmMetricRegistrar.java b/algorithm-metrics-api/src/main/java/org/neo4j/gds/algorithms/metrics/PassthroughAlgorithmMetricRegistrar.java index bb4fbad2d4..4cbdf232d0 100644 --- a/algorithm-metrics-api/src/main/java/org/neo4j/gds/algorithms/metrics/PassthroughAlgorithmMetricRegistrar.java +++ b/algorithm-metrics-api/src/main/java/org/neo4j/gds/algorithms/metrics/PassthroughAlgorithmMetricRegistrar.java @@ -25,12 +25,7 @@ public class PassthroughAlgorithmMetricRegistrar implements AlgorithmMetricRegistrar { @Override - public void started(String algorithm) { - - } - - @Override - public void failed(String algorithm) { - + public AlgorithmMetric create(String algorithm) { + return new PassthroughAlgorithmMetric(algorithm); } } diff --git a/algorithm-metrics-api/src/test/java/org/neo4j/gds/algorithms/metrics/AlgorithmMetricsServiceTest.java b/algorithm-metrics-api/src/test/java/org/neo4j/gds/algorithms/metrics/AlgorithmMetricsServiceTest.java index a869605f09..47bab58d5e 100644 --- a/algorithm-metrics-api/src/test/java/org/neo4j/gds/algorithms/metrics/AlgorithmMetricsServiceTest.java +++ b/algorithm-metrics-api/src/test/java/org/neo4j/gds/algorithms/metrics/AlgorithmMetricsServiceTest.java @@ -29,30 +29,17 @@ class AlgorithmMetricsServiceTest { @Test - void shouldRegisterStarted() { + void shouldCreateAlgorithmMetric() { // given var registrarMock = mock(AlgorithmMetricRegistrar.class); var metricsService = new AlgorithmMetricsService(registrarMock); // when - metricsService.started("foo"); + metricsService.create("foo"); // then - verify(registrarMock, times(1)).started("foo"); + verify(registrarMock, times(1)).create("foo"); verifyNoMoreInteractions(registrarMock); } - @Test - void shouldRegisterFailed() { - // given - var registrarMock = mock(AlgorithmMetricRegistrar.class); - var metricsService = new AlgorithmMetricsService(registrarMock); - - // when - metricsService.failed("foo"); - - // then - verify(registrarMock, times(1)).failed("foo"); - verifyNoMoreInteractions(registrarMock); - } }