From d14b07727a1c210177a1387089567c3f2764d83c Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 27 Nov 2023 15:41:44 -0500 Subject: [PATCH] fix(triggers): improve whitespace handling (#253) (#271) (cherry picked from commit 1abcb0193e1e8abe68e72a4656bc887dc90c9481) Co-authored-by: Andrew Azores --- .../cryostat/agent/triggers/SmartTrigger.java | 10 +- .../agent/triggers/TriggerParser.java | 3 +- .../agent/triggers/TriggerParserTest.java | 215 ++++++++++++++++++ 3 files changed, 222 insertions(+), 6 deletions(-) create mode 100644 src/test/java/io/cryostat/agent/triggers/TriggerParserTest.java diff --git a/src/main/java/io/cryostat/agent/triggers/SmartTrigger.java b/src/main/java/io/cryostat/agent/triggers/SmartTrigger.java index 5f2cef27..54f21623 100644 --- a/src/main/java/io/cryostat/agent/triggers/SmartTrigger.java +++ b/src/main/java/io/cryostat/agent/triggers/SmartTrigger.java @@ -24,8 +24,8 @@ public class SmartTrigger { private static final String DURATION_PATTERN_STR = - "(TargetDuration[<>=]+duration\\(['\"](\\d+[sSmMhH]+)['\"]\\))"; - private static final String DEFINITION_PATTERN_STR = "(.+)(?:;)" + DURATION_PATTERN_STR; + "(TargetDuration\\s*[<>=]+\\s*duration\\(['\"](\\d+[sSmMhH]+)['\"]\\))"; + private static final String DEFINITION_PATTERN_STR = "(.+)\\s*(?:;)\\s*" + DURATION_PATTERN_STR; private static final Pattern DEFINITION_PATTERN = Pattern.compile(DEFINITION_PATTERN_STR); public enum TriggerState { @@ -56,10 +56,10 @@ public SmartTrigger(String expression, String templateName) { this.state = TriggerState.NEW; Matcher m = DEFINITION_PATTERN.matcher(expression); if (m.matches()) { - triggerCondition = m.group(1); - durationConstraint = m.group(2).replaceAll("'", "\""); + triggerCondition = m.group(1).replaceAll("\\s", ""); + durationConstraint = m.group(2).replaceAll("'", "\"").replaceAll("\\s", ""); /* Duration.parse requires timestamps in ISO8601 Duration format */ - targetDuration = Duration.parse("PT" + m.group(3)); + targetDuration = Duration.parse("PT" + m.group(3).replaceAll("\\s", "")); } else { triggerCondition = expression; durationConstraint = ""; diff --git a/src/main/java/io/cryostat/agent/triggers/TriggerParser.java b/src/main/java/io/cryostat/agent/triggers/TriggerParser.java index 391a45e6..3024c5c3 100644 --- a/src/main/java/io/cryostat/agent/triggers/TriggerParser.java +++ b/src/main/java/io/cryostat/agent/triggers/TriggerParser.java @@ -41,7 +41,7 @@ public TriggerParser(FlightRecorderHelper flightRecorderHelper) { public List parse(String[] args) { List triggers = new ArrayList<>(); - if (args.length < 1) { + if (args == null || args.length < 1) { log.trace("Agent args were empty, no Triggers were defined"); return triggers; } @@ -53,6 +53,7 @@ public List parse(String[] args) { String[] expressions = triggerDefinitions.split(","); for (String s : expressions) { + s = s.replaceAll("\\s", ""); Matcher m = EXPRESSION_PATTERN.matcher(s); if (m.matches()) { String constraintString = m.group(1); diff --git a/src/test/java/io/cryostat/agent/triggers/TriggerParserTest.java b/src/test/java/io/cryostat/agent/triggers/TriggerParserTest.java new file mode 100644 index 00000000..fa977d76 --- /dev/null +++ b/src/test/java/io/cryostat/agent/triggers/TriggerParserTest.java @@ -0,0 +1,215 @@ +/* + * Copyright The Cryostat Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.cryostat.agent.triggers; + +import java.time.Duration; +import java.util.ArrayList; +import java.util.List; + +import io.cryostat.agent.FlightRecorderHelper; +import io.cryostat.agent.triggers.SmartTrigger.TriggerState; + +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.NullSource; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class TriggerParserTest { + + @Mock FlightRecorderHelper helper; + + TriggerParser parser; + + @BeforeEach + void setup() { + this.parser = new TriggerParser(helper); + } + + @ParameterizedTest + @MethodSource("emptyCases") + @NullSource + void testEmptyCases(List args) { + MatcherAssert.assertThat( + parser.parse(args == null ? null : args.toArray(new String[0])), + Matchers.equalTo(List.of())); + } + + @Test + void testSingleSimpleTrigger() { + Mockito.when(helper.isValidTemplate(Mockito.anyString())).thenReturn(true); + String[] in = new String[] {"[ProcessCpuLoad>0.2]~profile"}; + List out = parser.parse(in); + + MatcherAssert.assertThat(out, Matchers.hasSize(1)); + SmartTrigger trigger = out.get(0); + + MatcherAssert.assertThat(trigger.getExpression(), Matchers.equalTo("ProcessCpuLoad>0.2")); + MatcherAssert.assertThat(trigger.getRecordingTemplateName(), Matchers.equalTo("profile")); + MatcherAssert.assertThat(trigger.getDurationConstraint(), Matchers.emptyString()); + MatcherAssert.assertThat( + trigger.getTriggerCondition(), Matchers.equalTo("ProcessCpuLoad>0.2")); + MatcherAssert.assertThat(trigger.getState(), Matchers.equalTo(TriggerState.NEW)); + MatcherAssert.assertThat( + trigger.getTargetDuration(), Matchers.equalTo(Duration.ofSeconds(0))); + MatcherAssert.assertThat(trigger.getTimeConditionFirstMet(), Matchers.nullValue()); + } + + @Test + void testSingleComplexTrigger() { + Mockito.when(helper.isValidTemplate(Mockito.anyString())).thenReturn(true); + String[] in = + new String[] {"[ProcessCpuLoad>0.2;TargetDuration>duration(\"30s\")]~profile"}; + List out = parser.parse(in); + + MatcherAssert.assertThat(out, Matchers.hasSize(1)); + SmartTrigger trigger = out.get(0); + + MatcherAssert.assertThat( + trigger.getExpression(), + Matchers.equalTo("ProcessCpuLoad>0.2;TargetDuration>duration(\"30s\")")); + MatcherAssert.assertThat(trigger.getRecordingTemplateName(), Matchers.equalTo("profile")); + MatcherAssert.assertThat( + trigger.getDurationConstraint(), + Matchers.equalTo("TargetDuration>duration(\"30s\")")); + MatcherAssert.assertThat( + trigger.getTriggerCondition(), Matchers.equalTo("ProcessCpuLoad>0.2")); + MatcherAssert.assertThat(trigger.getState(), Matchers.equalTo(TriggerState.NEW)); + MatcherAssert.assertThat( + trigger.getTargetDuration(), Matchers.equalTo(Duration.ofSeconds(30))); + MatcherAssert.assertThat(trigger.getTimeConditionFirstMet(), Matchers.nullValue()); + } + + @Test + void testSingleComplexTriggerWithWhitespace() { + Mockito.when(helper.isValidTemplate(Mockito.anyString())).thenReturn(true); + String[] in = + new String[] { + "[ProcessCpuLoad > 0.2 ; TargetDuration > duration(\"30s\")]~profile" + }; + List out = parser.parse(in); + + MatcherAssert.assertThat(out, Matchers.hasSize(1)); + SmartTrigger trigger = out.get(0); + + MatcherAssert.assertThat( + trigger.getExpression(), + Matchers.equalTo("ProcessCpuLoad>0.2;TargetDuration>duration(\"30s\")")); + MatcherAssert.assertThat(trigger.getRecordingTemplateName(), Matchers.equalTo("profile")); + MatcherAssert.assertThat( + trigger.getDurationConstraint(), + Matchers.equalTo("TargetDuration>duration(\"30s\")")); + MatcherAssert.assertThat( + trigger.getTriggerCondition(), Matchers.equalTo("ProcessCpuLoad>0.2")); + MatcherAssert.assertThat(trigger.getState(), Matchers.equalTo(TriggerState.NEW)); + MatcherAssert.assertThat( + trigger.getTargetDuration(), Matchers.equalTo(Duration.ofSeconds(30))); + MatcherAssert.assertThat(trigger.getTimeConditionFirstMet(), Matchers.nullValue()); + } + + @Test + void testMultipleComplexTriggerWithWhitespace() { + Mockito.when(helper.isValidTemplate(Mockito.anyString())).thenReturn(true); + String[] in = + new String[] { + "[ProcessCpuLoad>0.2 ; TargetDuration>duration(\"30s\")]~profile," + + " [(HeapMemoryUsagePercent > 50 && NonHeapMemoryUsage > 1) ||" + + " SystemCpuLoad > 4 ; TargetDuration > duration(\"2m\")]~default.jfc" + }; + List out = parser.parse(in); + + MatcherAssert.assertThat(out, Matchers.hasSize(2)); + + SmartTrigger trigger1 = out.get(0); + MatcherAssert.assertThat( + trigger1.getExpression(), + Matchers.equalTo("ProcessCpuLoad>0.2;TargetDuration>duration(\"30s\")")); + MatcherAssert.assertThat(trigger1.getRecordingTemplateName(), Matchers.equalTo("profile")); + MatcherAssert.assertThat( + trigger1.getDurationConstraint(), + Matchers.equalTo("TargetDuration>duration(\"30s\")")); + MatcherAssert.assertThat( + trigger1.getTriggerCondition(), Matchers.equalTo("ProcessCpuLoad>0.2")); + MatcherAssert.assertThat(trigger1.getState(), Matchers.equalTo(TriggerState.NEW)); + MatcherAssert.assertThat( + trigger1.getTargetDuration(), Matchers.equalTo(Duration.ofSeconds(30))); + MatcherAssert.assertThat(trigger1.getTimeConditionFirstMet(), Matchers.nullValue()); + + SmartTrigger trigger2 = out.get(1); + MatcherAssert.assertThat( + trigger2.getExpression(), + Matchers.equalTo( + "(HeapMemoryUsagePercent>50&&NonHeapMemoryUsage>1)||SystemCpuLoad>4;TargetDuration>duration(\"2m\")")); + MatcherAssert.assertThat(trigger2.getRecordingTemplateName(), Matchers.equalTo("default")); + MatcherAssert.assertThat( + trigger2.getDurationConstraint(), + Matchers.equalTo("TargetDuration>duration(\"2m\")")); + MatcherAssert.assertThat( + trigger2.getTriggerCondition(), + Matchers.equalTo( + "(HeapMemoryUsagePercent>50&&NonHeapMemoryUsage>1)||SystemCpuLoad>4")); + MatcherAssert.assertThat(trigger2.getState(), Matchers.equalTo(TriggerState.NEW)); + MatcherAssert.assertThat( + trigger2.getTargetDuration(), Matchers.equalTo(Duration.ofMinutes(2))); + MatcherAssert.assertThat(trigger2.getTimeConditionFirstMet(), Matchers.nullValue()); + } + + @Test + void testMultipleComplexTriggerWithWhitespaceWhenOnlyOneTemplateValid() { + Mockito.when(helper.isValidTemplate(Mockito.anyString())) + .thenReturn(true) + .thenReturn(false); + String[] in = + new String[] { + "[ProcessCpuLoad>0.2 ; TargetDuration>duration(\"30s\")]~profile," + + " [(HeapMemoryUsagePercent > 50 && NonHeapMemoryUsage > 1) ||" + + " SystemCpuLoad > 4 ; TargetDuration > duration(\"2m\")]~default.jfc" + }; + List out = parser.parse(in); + + MatcherAssert.assertThat(out, Matchers.hasSize(1)); + + SmartTrigger trigger1 = out.get(0); + MatcherAssert.assertThat( + trigger1.getExpression(), + Matchers.equalTo("ProcessCpuLoad>0.2;TargetDuration>duration(\"30s\")")); + MatcherAssert.assertThat(trigger1.getRecordingTemplateName(), Matchers.equalTo("profile")); + MatcherAssert.assertThat( + trigger1.getDurationConstraint(), + Matchers.equalTo("TargetDuration>duration(\"30s\")")); + MatcherAssert.assertThat( + trigger1.getTriggerCondition(), Matchers.equalTo("ProcessCpuLoad>0.2")); + MatcherAssert.assertThat(trigger1.getState(), Matchers.equalTo(TriggerState.NEW)); + MatcherAssert.assertThat( + trigger1.getTargetDuration(), Matchers.equalTo(Duration.ofSeconds(30))); + MatcherAssert.assertThat(trigger1.getTimeConditionFirstMet(), Matchers.nullValue()); + } + + static List> emptyCases() { + List> l = new ArrayList<>(); + l.add(List.of()); + l.add(List.of("")); + l.add(List.of(" ")); + return l; + } +}