From d2be27372c1e3efb76e2edd648bcfab8945ea51f Mon Sep 17 00:00:00 2001 From: Rashmi Pujar Date: Thu, 31 Mar 2022 11:52:53 -0400 Subject: Bug fixes for APEX PDP metrics 1. APEX engine metrics were being initialized to 0 in the constructor which caused them to reset to 0. Hence, initialization is removed, and updates are done to metrics only upon: engineStart/Stop, executionEnter/Exit stages. 2. APEX engine Uptime was only updated when engine stopped, hence always 0. Also, uptime is redundant since it can be derived from lastStart timestamp and hence is removed as a metric. 3. Fixed a corner case bug which occurs when all policies are undeployed the counters are not correct. 4. Fix Unit tests Issue-ID: POLICY-4044 Signed-off-by: Rashmi Pujar Change-Id: I4d875ff288cf8c242a1851eb105e56dc69b97883 --- .../model/enginemodel/concepts/AxEngineStats.java | 39 +++------------------- .../enginemodel/concepts/EngineStatsTest.java | 38 ++++----------------- .../onappf/handler/PdpUpdateMessageHandler.java | 19 ++++++----- 3 files changed, 22 insertions(+), 74 deletions(-) diff --git a/model/src/main/java/org/onap/policy/apex/model/enginemodel/concepts/AxEngineStats.java b/model/src/main/java/org/onap/policy/apex/model/enginemodel/concepts/AxEngineStats.java index 1420d1e0d..42a5ef785 100644 --- a/model/src/main/java/org/onap/policy/apex/model/enginemodel/concepts/AxEngineStats.java +++ b/model/src/main/java/org/onap/policy/apex/model/enginemodel/concepts/AxEngineStats.java @@ -50,10 +50,7 @@ public class AxEngineStats extends AxConcept { static final Gauge ENGINE_EVENT_EXECUTIONS = Gauge.build().name("engine_event_executions") .namespace(PrometheusUtils.PdpType.PDPA.getNamespace()).labelNames(ENGINE_INSTANCE_ID) .help("Total number of APEX events processed by the engine.").register(); - static final Gauge ENGINE_UPTIME = Gauge.build().name("engine_uptime") - .namespace(PrometheusUtils.PdpType.PDPA.getNamespace()).labelNames(ENGINE_INSTANCE_ID) - .help("Time elapsed since the engine was started.").register(); - static final Gauge ENGINE_START_TIMESTAMP = Gauge.build().name("engine_last_start_timestamp_epoch") + static final Gauge ENGINE_LAST_START_TIMESTAMP = Gauge.build().name("engine_last_start_timestamp_epoch") .namespace(PrometheusUtils.PdpType.PDPA.getNamespace()).labelNames(ENGINE_INSTANCE_ID) .help("Epoch timestamp of the instance when engine was last started.").register(); static final Gauge ENGINE_AVG_EXECUTION_TIME = Gauge.build().name("engine_average_execution_time_seconds") @@ -87,22 +84,6 @@ public class AxEngineStats extends AxConcept { upTime = 0; lastEnterTime = 0; lastStart = 0; - initEngineMetricsWithPrometheus(); - } - - /** - * Register the APEX engine metrics with Prometheus. - */ - private void initEngineMetricsWithPrometheus() { - var engineId = getKey().getParentArtifactKey().getId(); - if (engineId.startsWith(AxKey.NULL_KEY_NAME)) { - return; - } - ENGINE_UPTIME.labels(engineId).set(upTime / 1000d); - ENGINE_EVENT_EXECUTIONS.labels(engineId).set(this.eventCount); - ENGINE_START_TIMESTAMP.labels(engineId).set(this.lastStart); - ENGINE_AVG_EXECUTION_TIME.labels(engineId).set(this.averageExecutionTime / 1000d); - ENGINE_LAST_EXECUTION_TIME.labels(engineId).observe(this.lastExecutionTime / 1000d); } /** @@ -146,7 +127,6 @@ public class AxEngineStats extends AxConcept { this.averageExecutionTime = averageExecutionTime; this.upTime = upTime; this.lastStart = lastStart; - initEngineMetricsWithPrometheus(); } /** @@ -218,8 +198,6 @@ public class AxEngineStats extends AxConcept { */ public void setEventCount(final long eventCount) { this.eventCount = eventCount; - ENGINE_EVENT_EXECUTIONS.labels(getKey().getParentArtifactKey().getId()) - .set(this.eventCount); } /** @@ -238,8 +216,6 @@ public class AxEngineStats extends AxConcept { */ public void setLastExecutionTime(final long lastExecutionTime) { this.lastExecutionTime = lastExecutionTime; - ENGINE_LAST_EXECUTION_TIME.labels(getKey().getParentArtifactKey().getId()) - .observe(this.lastExecutionTime / 1000d); } /** @@ -258,8 +234,6 @@ public class AxEngineStats extends AxConcept { */ public void setAverageExecutionTime(final double averageExecutionTime) { this.averageExecutionTime = averageExecutionTime; - ENGINE_AVG_EXECUTION_TIME.labels(getKey().getParentArtifactKey().getId()) - .set(this.averageExecutionTime / 1000d); } /** @@ -281,7 +255,6 @@ public class AxEngineStats extends AxConcept { */ public void setUpTime(final long upTime) { this.upTime = upTime; - ENGINE_UPTIME.labels(getKey().getParentArtifactKey().getId()).set(this.upTime / 1000d); } /** @@ -291,7 +264,6 @@ public class AxEngineStats extends AxConcept { */ private void setLastStart(final long lastStart) { this.lastStart = lastStart; - ENGINE_START_TIMESTAMP.labels(getKey().getParentArtifactKey().getId()).set(this.lastStart); } /** @@ -314,7 +286,6 @@ public class AxEngineStats extends AxConcept { upTime = 0; lastEnterTime = 0; lastStart = 0; - initEngineMetricsWithPrometheus(); } /** @@ -325,12 +296,13 @@ public class AxEngineStats extends AxConcept { public synchronized void executionEnter(final AxArtifactKey eventkey) { final long now = System.currentTimeMillis(); eventCount++; + ENGINE_EVENT_EXECUTIONS.labels(getKey().getParentArtifactKey().getId()).inc(); if (eventCount < 0) { eventCount = 2; + ENGINE_EVENT_EXECUTIONS.labels(getKey().getParentArtifactKey().getId()).set(this.eventCount); } lastEnterTime = now; timeStamp = now; - ENGINE_EVENT_EXECUTIONS.labels(getKey().getParentArtifactKey().getId()).set(this.eventCount); } /** @@ -356,6 +328,7 @@ public class AxEngineStats extends AxConcept { final long now = System.currentTimeMillis(); timeStamp = now; this.setLastStart(now); + ENGINE_LAST_START_TIMESTAMP.labels(getKey().getParentArtifactKey().getId()).set(this.lastStart); } /** @@ -366,7 +339,7 @@ public class AxEngineStats extends AxConcept { timeStamp = now; upTime += (timeStamp - this.getLastStart()); this.setLastStart(0); - ENGINE_UPTIME.labels(getKey().getParentArtifactKey().getId()).set(this.upTime / 1000d); + ENGINE_LAST_START_TIMESTAMP.labels(getKey().getParentArtifactKey().getId()).set(this.lastStart); } /** @@ -432,8 +405,6 @@ public class AxEngineStats extends AxConcept { copy.setAverageExecutionTime(averageExecutionTime); copy.setUpTime(upTime); copy.setLastStart(lastStart); - initEngineMetricsWithPrometheus(); - return copy; } diff --git a/model/src/test/java/org/onap/policy/apex/model/enginemodel/concepts/EngineStatsTest.java b/model/src/test/java/org/onap/policy/apex/model/enginemodel/concepts/EngineStatsTest.java index 3eca27c08..542c6e4c5 100644 --- a/model/src/test/java/org/onap/policy/apex/model/enginemodel/concepts/EngineStatsTest.java +++ b/model/src/test/java/org/onap/policy/apex/model/enginemodel/concepts/EngineStatsTest.java @@ -62,16 +62,13 @@ public class EngineStatsTest { stats.setAverageExecutionTime(123.45); assertEquals(Double.valueOf(123.45), Double.valueOf(stats.getAverageExecutionTime())); - checkAvgExecTimeMetric(stats); stats.setEventCount(987); assertEquals(987, stats.getEventCount()); - checkEventsCountMetric(stats); final long lastExecutionTime = System.currentTimeMillis(); stats.setLastExecutionTime(lastExecutionTime); assertEquals(lastExecutionTime, stats.getLastExecutionTime()); - checkLastExecTimeMetric(stats); final long timestamp = System.currentTimeMillis(); stats.setTimeStamp(timestamp); @@ -81,17 +78,14 @@ public class EngineStatsTest { final long upTime = System.currentTimeMillis() - timestamp; stats.setUpTime(upTime); assertEquals(upTime, stats.getUpTime()); - checkUpTimeMetric(stats); stats.engineStart(); assertTrue(stats.getUpTime() > -1); checkEngineStartTimestampMetric(stats); - checkLastExecTimeMetric(stats); stats.engineStop(); assertTrue(stats.getUpTime() >= 0); stats.engineStop(); - checkUpTimeMetric(stats); checkEngineStartTimestampMetric(stats); stats.reset(); @@ -104,15 +98,12 @@ public class EngineStatsTest { stats.setEventCount(10); stats.executionEnter(new AxArtifactKey()); assertEquals(11, stats.getEventCount()); - checkEventsCountMetric(stats); stats.reset(); stats.engineStart(); - stats.setEventCount(4); - checkUpTimeMetric(stats); + stats.setEventCount(3); stats.executionEnter(new AxArtifactKey()); checkEventsCountMetric(stats); - checkAvgExecTimeMetric(stats); checkEngineStartTimestampMetric(stats); synchronized (WAIT_LOCK) { @@ -124,10 +115,13 @@ public class EngineStatsTest { } stats.executionExit(); + checkAvgExecTimeMetric(stats); + checkEventsCountMetric(stats); + checkLastExecTimeMetric(stats); final double avExecutionTime = stats.getAverageExecutionTime(); assertTrue(avExecutionTime >= 2.0 && avExecutionTime < 10.0); stats.engineStop(); - checkUpTimeMetric(stats); + checkEngineStartTimestampMetric(stats); AxValidationResult result = new AxValidationResult(); result = stats.validate(result); @@ -145,7 +139,6 @@ public class EngineStatsTest { stats.clean(); stats.reset(); - checkAllPrometheusMetrics(stats); final AxEngineStats clonedStats = new AxEngineStats(stats); assertEquals("AxEngineStats:(engineKey=AxReferenceKey:(parentKey", clonedStats.toString().substring(0, 50)); @@ -158,7 +151,6 @@ public class EngineStatsTest { assertEquals(stats, stats); // NOSONAR assertEquals(stats, clonedStats); assertNotNull(stats); - checkAllPrometheusMetrics(clonedStats); Object helloObject = "Hello"; assertNotEquals(stats, helloObject); @@ -176,7 +168,6 @@ public class EngineStatsTest { stats.setTimeStamp(0); assertEquals(stats, new AxEngineStats(statsKey)); assertEquals(0, stats.compareTo(new AxEngineStats(statsKey))); - checkAllPrometheusMetrics(clonedStats); stats.setEventCount(1); assertNotEquals(stats, new AxEngineStats(statsKey)); @@ -210,23 +201,16 @@ public class EngineStatsTest { stats.engineStart(); assertNotEquals(stats, new AxEngineStats(statsKey)); + checkEngineStartTimestampMetric(stats); final AxEngineStats newStats = new AxEngineStats(statsKey); newStats.setTimeStamp(stats.getTimeStamp()); assertNotEquals(stats, newStats); assertNotEquals(0, stats.compareTo(newStats)); stats.engineStop(); - checkUpTimeMetric(stats); checkEngineStartTimestampMetric(stats); stats.reset(); assertEquals(stats, new AxEngineStats(statsKey)); assertEquals(0, stats.compareTo(new AxEngineStats(statsKey))); - checkAllPrometheusMetrics(stats); - } - - private void checkUpTimeMetric(AxEngineStats stats) { - Double upTimeMetric = CollectorRegistry.defaultRegistry.getSampleValue("pdpa_engine_uptime", - new String[]{AxEngineStats.ENGINE_INSTANCE_ID}, new String[]{ENGINE_KEY + ":" + ENGINE_VERSION}) * 1000d; - assertEquals(upTimeMetric.longValue(), stats.getUpTime()); } private void checkEventsCountMetric(AxEngineStats stats) { @@ -256,12 +240,4 @@ public class EngineStatsTest { new String[]{ENGINE_KEY + ":" + ENGINE_VERSION}) * 1000d; assertEquals(avgExecTimeMetric, Double.valueOf(stats.getAverageExecutionTime())); } - - private void checkAllPrometheusMetrics(AxEngineStats stats) { - checkEventsCountMetric(stats); - checkUpTimeMetric(stats); - checkAvgExecTimeMetric(stats); - checkEngineStartTimestampMetric(stats); - checkEngineStartTimestampMetric(stats); - } -} \ No newline at end of file +} diff --git a/services/services-onappf/src/main/java/org/onap/policy/apex/services/onappf/handler/PdpUpdateMessageHandler.java b/services/services-onappf/src/main/java/org/onap/policy/apex/services/onappf/handler/PdpUpdateMessageHandler.java index f10a0b064..1bcb8970b 100644 --- a/services/services-onappf/src/main/java/org/onap/policy/apex/services/onappf/handler/PdpUpdateMessageHandler.java +++ b/services/services-onappf/src/main/java/org/onap/policy/apex/services/onappf/handler/PdpUpdateMessageHandler.java @@ -162,9 +162,10 @@ public class PdpUpdateMessageHandler { final PdpMessageHandler pdpMessageHandler, ApexEngineHandler apexEngineHandler) { PdpResponseDetails pdpResponseDetails = null; if (null != apexEngineHandler && apexEngineHandler.isApexEngineRunning()) { - final var runningPolicies = apexEngineHandler.getRunningPolicies(); + List runningPolicies = apexEngineHandler.getRunningPolicies(); try { apexEngineHandler.shutdown(); + runningPolicies = apexEngineHandler.getRunningPolicies(); pdpResponseDetails = pdpMessageHandler.createPdpResonseDetails(pdpUpdateMsg.getRequestId(), PdpResponseStatus.SUCCESS, "Pdp update successful. No policies are running."); } catch (final ApexStarterException e) { @@ -180,7 +181,7 @@ public class PdpUpdateMessageHandler { private PdpResponseDetails startApexEngineBasedOnPolicies(final PdpUpdate pdpUpdateMsg, final PdpMessageHandler pdpMessageHandler, ApexEngineHandler apexEngineHandler) { PdpResponseDetails pdpResponseDetails = null; - List runningPolicies = null; + List runningPolicies = new ArrayList<>(); try { if (null != apexEngineHandler && apexEngineHandler.isApexEngineRunning()) { apexEngineHandler.updateApexEngine(pdpUpdateMsg.getPoliciesToBeDeployed(), @@ -296,7 +297,7 @@ public class PdpUpdateMessageHandler { private void updateDeploymentCounts(final List runningPolicies, final PdpUpdate pdpUpdateMsg) { final var statisticsManager = ApexPolicyStatisticsManager.getInstanceFromRegistry(); - if (statisticsManager != null) { + if (statisticsManager != null && runningPolicies != null) { if (pdpUpdateMsg.getPoliciesToBeDeployed() != null && !pdpUpdateMsg.getPoliciesToBeDeployed().isEmpty()) { var policiesToDeploy = pdpUpdateMsg.getPoliciesToBeDeployed().stream() .map(ToscaWithTypeAndObjectProperties::getIdentifier).collect(Collectors.toList()); @@ -312,13 +313,13 @@ public class PdpUpdateMessageHandler { var policiesToUndeploy = pdpUpdateMsg.getPoliciesToBeUndeployed(); if (policiesToUndeploy != null && !policiesToUndeploy.isEmpty()) { - var policiesSuccessfullyUndeployed = new ArrayList<>(policiesToUndeploy); - policiesSuccessfullyUndeployed.retainAll(runningPolicies); - policiesSuccessfullyUndeployed.forEach(policy -> statisticsManager.updatePolicyUndeployCounter(true)); - - var policiesFailedToUndeploy = new ArrayList<>(policiesToUndeploy); - policiesFailedToUndeploy.removeIf(runningPolicies::contains); + var policiesFailedToUndeploy = new ArrayList<>(policiesToUndeploy); + policiesFailedToUndeploy.retainAll(runningPolicies); policiesFailedToUndeploy.forEach(policy -> statisticsManager.updatePolicyUndeployCounter(false)); + + var policiesSuccessfullyUndeployed = new ArrayList<>(policiesToUndeploy); + policiesSuccessfullyUndeployed.removeIf(runningPolicies::contains); + policiesSuccessfullyUndeployed.forEach(policy -> statisticsManager.updatePolicyUndeployCounter(true)); } } } -- cgit 1.2.3-korg