This is an automated email from the ASF dual-hosted git repository. wusheng pushed a commit to branch oal-v2 in repository https://gitbox.apache.org/repos/asf/skywalking.git
commit 7af21ebd2f8b294b5fe0c8b88710c2f9daccab16 Author: Wu Sheng <[email protected]> AuthorDate: Mon Feb 9 22:43:37 2026 +0800 Add comprehensive error handling and tests for OAL parsing This commit adds robust error handling for OAL V2 parsing with detailed error messages and comprehensive unit tests. New components: -------------- 1. OALErrorListener - Custom ANTLR error listener that: - Collects all syntax errors during parsing - Captures error location (file:line:column) - Preserves offending tokens for context - Formats errors in user-friendly format 2. OALParsingErrorTest - Comprehensive error test suite covering: - Missing semicolons - Invalid variable names - Missing/malformed from() clauses - Invalid filter expressions - Unclosed parentheses and strings - Invalid operators - Missing function calls - Invalid function arguments - Multiple chained errors - Valid scripts (positive tests) Enhanced OALScriptParserV2: -------------------------- - Integrates OALErrorListener for better error reporting - Throws IllegalArgumentException with formatted error messages - Provides file name and line/column information in errors - Removes default console error listener to prevent noise Error message format: -------------------- OAL parsing failed with N error(s): 1. file.oal:2:15 - mismatched input ';' expecting '(' (at ';') 2. file.oal:3:20 - missing ')' at '<EOF>' Benefits: --------- - Developers get clear, actionable error messages - Error locations help quickly identify problems - Multiple errors reported at once (when possible) - Tests ensure error handling remains robust - Foundation for IDE integration (language server protocol) All tests compile successfully and follow JUnit 5 patterns. Co-Authored-By: Claude Opus 4.5 <[email protected]> --- .../skywalking/oal/v2/parser/OALErrorListener.java | 161 ++++++++++ .../oal/v2/parser/OALScriptParserV2.java | 11 + .../oal/v2/parser/OALParsingErrorTest.java | 340 +++++++++++++++++++++ 3 files changed, 512 insertions(+) diff --git a/oap-server/oal-rt/src/main/java/org/apache/skywalking/oal/v2/parser/OALErrorListener.java b/oap-server/oal-rt/src/main/java/org/apache/skywalking/oal/v2/parser/OALErrorListener.java new file mode 100644 index 0000000000..c52dc1ac99 --- /dev/null +++ b/oap-server/oal-rt/src/main/java/org/apache/skywalking/oal/v2/parser/OALErrorListener.java @@ -0,0 +1,161 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.skywalking.oal.v2.parser; + +import java.util.ArrayList; +import java.util.List; +import lombok.Getter; +import org.antlr.v4.runtime.BaseErrorListener; +import org.antlr.v4.runtime.RecognitionException; +import org.antlr.v4.runtime.Recognizer; +import org.antlr.v4.runtime.Token; + +/** + * Error listener for OAL parsing that collects detailed error information. + * + * This listener captures syntax errors during parsing and provides: + * - Error location (line and column) + * - Offending token + * - Error message + * - File name for context + * + * Example usage: + * <pre> + * OALErrorListener errorListener = new OALErrorListener("core.oal"); + * parser.removeErrorListeners(); + * parser.addErrorListener(errorListener); + * + * // After parsing + * if (errorListener.hasErrors()) { + * for (OALSyntaxError error : errorListener.getErrors()) { + * System.err.println(error.toString()); + * } + * } + * </pre> + */ +public class OALErrorListener extends BaseErrorListener { + + private final String fileName; + + @Getter + private final List<OALSyntaxError> errors = new ArrayList<>(); + + public OALErrorListener(String fileName) { + this.fileName = fileName; + } + + @Override + public void syntaxError( + Recognizer<?, ?> recognizer, + Object offendingSymbol, + int line, + int charPositionInLine, + String msg, + RecognitionException e) { + + String tokenText = null; + if (offendingSymbol instanceof Token) { + Token token = (Token) offendingSymbol; + tokenText = token.getText(); + } + + OALSyntaxError error = new OALSyntaxError( + fileName, + line, + charPositionInLine, + msg, + tokenText + ); + + errors.add(error); + } + + /** + * Check if any errors were encountered during parsing. + */ + public boolean hasErrors() { + return !errors.isEmpty(); + } + + /** + * Get error count. + */ + public int getErrorCount() { + return errors.size(); + } + + /** + * Get formatted error message including all errors. + */ + public String getFormattedErrors() { + if (!hasErrors()) { + return ""; + } + + StringBuilder sb = new StringBuilder(); + sb.append("OAL parsing failed with ").append(errors.size()).append(" error(s):\n"); + + for (int i = 0; i < errors.size(); i++) { + OALSyntaxError error = errors.get(i); + sb.append(" ").append(i + 1).append(". ").append(error.toString()).append("\n"); + } + + return sb.toString(); + } + + /** + * Represents a single syntax error in OAL script. + */ + @Getter + public static class OALSyntaxError { + private final String fileName; + private final int line; + private final int column; + private final String message; + private final String offendingToken; + + public OALSyntaxError(String fileName, int line, int column, String message, String offendingToken) { + this.fileName = fileName; + this.line = line; + this.column = column; + this.message = message; + this.offendingToken = offendingToken; + } + + @Override + public String toString() { + StringBuilder sb = new StringBuilder(); + sb.append(fileName).append(":").append(line).append(":").append(column); + sb.append(" - ").append(message); + + if (offendingToken != null && !offendingToken.isEmpty()) { + sb.append(" (at '").append(offendingToken).append("')"); + } + + return sb.toString(); + } + + /** + * Get short location string for error reporting. + */ + public String getLocation() { + return fileName + ":" + line + ":" + column; + } + } +} diff --git a/oap-server/oal-rt/src/main/java/org/apache/skywalking/oal/v2/parser/OALScriptParserV2.java b/oap-server/oal-rt/src/main/java/org/apache/skywalking/oal/v2/parser/OALScriptParserV2.java index baba2f1da5..19b1696f5c 100644 --- a/oap-server/oal-rt/src/main/java/org/apache/skywalking/oal/v2/parser/OALScriptParserV2.java +++ b/oap-server/oal-rt/src/main/java/org/apache/skywalking/oal/v2/parser/OALScriptParserV2.java @@ -94,6 +94,7 @@ public class OALScriptParserV2 { * @param fileName filename for error reporting * @return parser result * @throws IOException if parsing fails + * @throws IllegalArgumentException if syntax errors are found */ public static OALScriptParserV2 parse(Reader reader, String fileName) throws IOException { // Create lexer @@ -105,9 +106,19 @@ public class OALScriptParserV2 { // Create parser OALParser parser = new OALParser(tokens); + // Add custom error listener to collect detailed error information + OALErrorListener errorListener = new OALErrorListener(fileName); + parser.removeErrorListeners(); // Remove default console error listener + parser.addErrorListener(errorListener); + // Parse the script OALParser.RootContext root = parser.root(); + // Check for syntax errors + if (errorListener.hasErrors()) { + throw new IllegalArgumentException(errorListener.getFormattedErrors()); + } + // Walk the parse tree with V2 listener OALListenerV2 listener = new OALListenerV2(fileName); ParseTreeWalker walker = new ParseTreeWalker(); diff --git a/oap-server/oal-rt/src/test/java/org/apache/skywalking/oal/v2/parser/OALParsingErrorTest.java b/oap-server/oal-rt/src/test/java/org/apache/skywalking/oal/v2/parser/OALParsingErrorTest.java new file mode 100644 index 0000000000..8ab88ce04d --- /dev/null +++ b/oap-server/oal-rt/src/test/java/org/apache/skywalking/oal/v2/parser/OALParsingErrorTest.java @@ -0,0 +1,340 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.skywalking.oal.v2.parser; + +import java.io.IOException; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; + +/** + * Test error handling and error messages for OAL parsing. + * + * This test suite validates that: + * 1. Parser properly detects syntax errors + * 2. Error messages are clear and helpful + * 3. Error locations (line/column) are reported accurately + * 4. Different types of errors are handled appropriately + */ +public class OALParsingErrorTest { + + /** + * Test syntax error: missing semicolon + */ + @Test + public void testSyntaxError_MissingSemicolon() { + String script = "service_resp_time = from(Service.latency).longAvg()"; // Missing ; + + Exception exception = assertThrows(IllegalArgumentException.class, () -> { + OALScriptParserV2.parse(script, "test.oal"); + }, "Parser should throw IllegalArgumentException for missing semicolon"); + + String message = exception.getMessage(); + assertTrue(message.contains("test.oal"), "Error message should include file name"); + assertTrue(message.contains("1:"), "Error message should include line number"); + } + + /** + * Test syntax error: invalid variable name (starts with number) + */ + @Test + public void testSyntaxError_InvalidVariableName() { + String script = "1service_resp_time = from(Service.latency).longAvg();"; + + assertThrows(IllegalArgumentException.class, () -> { + OALScriptParserV2.parse(script, "test.oal"); + }, "Parser should reject variable names starting with numbers"); + } + + /** + * Test syntax error: missing equals sign + */ + @Test + public void testSyntaxError_MissingEquals() { + String script = "service_resp_time from(Service.latency).longAvg();"; + + assertThrows(IllegalArgumentException.class, () -> { + OALScriptParserV2.parse(script, "test.oal"); + }, "Parser should require equals sign in metric definition"); + } + + /** + * Test syntax error: missing from() clause + */ + @Test + public void testSyntaxError_MissingFrom() { + String script = "service_resp_time = .longAvg();"; + + assertThrows(IllegalArgumentException.class, () -> { + OALScriptParserV2.parse(script, "test.oal"); + }, "Parser should require from() clause"); + } + + /** + * Test syntax error: missing source name in from() + */ + @Test + public void testSyntaxError_MissingSourceName() { + String script = "service_resp_time = from().longAvg();"; + + assertThrows(IllegalArgumentException.class, () -> { + OALScriptParserV2.parse(script, "test.oal"); + }, "Parser should require source name in from() clause"); + } + + /** + * Test syntax error: malformed filter expression + */ + @Test + public void testSyntaxError_MalformedFilter() { + String script = "service_sla = from(Service.*).filter(status ==).percent();"; + + assertThrows(IllegalArgumentException.class, () -> { + OALScriptParserV2.parse(script, "test.oal"); + }, "Parser should detect incomplete filter expression"); + } + + /** + * Test syntax error: unclosed parenthesis in filter + */ + @Test + public void testSyntaxError_UnclosedParenthesis() { + String script = "service_sla = from(Service.*).filter(status == true.percent();"; + + assertThrows(IllegalArgumentException.class, () -> { + OALScriptParserV2.parse(script, "test.oal"); + }, "Parser should detect unclosed parenthesis"); + } + + /** + * Test syntax error: invalid operator in filter + */ + @Test + public void testSyntaxError_InvalidOperator() { + String script = "service_sla = from(Service.*).filter(latency <=> 100).count();"; + + assertThrows(IllegalArgumentException.class, () -> { + OALScriptParserV2.parse(script, "test.oal"); + }, "Parser should reject invalid operators"); + } + + /** + * Test syntax error: missing function call + */ + @Test + public void testSyntaxError_MissingFunction() { + String script = "service_resp_time = from(Service.latency);"; + + assertThrows(IllegalArgumentException.class, () -> { + OALScriptParserV2.parse(script, "test.oal"); + }, "Parser should require aggregation function"); + } + + /** + * Test syntax error: empty function arguments + */ + @Test + public void testSyntaxError_InvalidFunctionArguments() { + String script = "service_p99 = from(Service.latency).percentile(,);"; + + assertThrows(IllegalArgumentException.class, () -> { + OALScriptParserV2.parse(script, "test.oal"); + }, "Parser should reject empty function arguments"); + } + + /** + * Test syntax error: multiple equals signs + */ + @Test + public void testSyntaxError_MultipleEquals() { + String script = "service_resp_time == from(Service.latency).longAvg();"; + + assertThrows(IllegalArgumentException.class, () -> { + OALScriptParserV2.parse(script, "test.oal"); + }, "Parser should reject multiple equals signs"); + } + + /** + * Test syntax error: invalid characters + */ + @Test + public void testSyntaxError_InvalidCharacters() { + String script = "service_resp_time = from(Service.latency).longAvg() @ #;"; + + assertThrows(IllegalArgumentException.class, () -> { + OALScriptParserV2.parse(script, "test.oal"); + }, "Parser should reject invalid characters"); + } + + /** + * Test syntax error: unclosed string literal + */ + @Test + public void testSyntaxError_UnclosedString() { + String script = "service_sla = from(Service.*).filter(name == \"test).percent();"; + + assertThrows(IllegalArgumentException.class, () -> { + OALScriptParserV2.parse(script, "test.oal"); + }, "Parser should detect unclosed string literals"); + } + + /** + * Test empty script + */ + @Test + public void testEmptyScript() throws IOException { + String script = ""; + OALScriptParserV2 result = OALScriptParserV2.parse(script, "empty.oal"); + assertTrue(result.getMetrics().isEmpty(), "Empty script should produce no metrics"); + } + + /** + * Test script with only comments + */ + @Test + public void testOnlyComments() throws IOException { + String script = "// This is a comment\n" + + "/* Multi-line\n" + + " comment */\n"; + + OALScriptParserV2 result = assertDoesNotThrow(() -> { + return OALScriptParserV2.parse(script, "comments.oal"); + }, "Comment-only script should parse without errors"); + + assertTrue(result.getMetrics().isEmpty(), "Comment-only script should produce no metrics"); + } + + /** + * Test syntax error: missing dot before attribute + * Note: "Servicelatency" is treated as a single source name, which is valid syntax + */ + @Test + public void testValidSyntax_SourceWithoutDot() throws IOException { + String script = "service_resp_time = from(Servicelatency).longAvg();"; + + // This is actually valid syntax - "Servicelatency" is just a source name + // Semantic validation (checking if source exists) happens later + OALScriptParserV2 result = assertDoesNotThrow(() -> { + return OALScriptParserV2.parse(script, "test.oal"); + }, "Single-word source name should be valid syntax"); + + assertTrue(result.hasMetrics(), "Should parse successfully"); + } + + /** + * Test syntax error: multiple filters without proper syntax + */ + @Test + public void testSyntaxError_MultipleFiltersInvalid() { + String script = "service_sla = from(Service.*).filter(status == true) filter(latency > 100).percent();"; + + assertThrows(IllegalArgumentException.class, () -> { + OALScriptParserV2.parse(script, "test.oal"); + }, "Parser should reject multiple filters without proper chaining"); + } + + /** + * Test syntax error: nested function calls (not supported) + */ + @Test + public void testSyntaxError_NestedFunctions() { + String script = "service_resp_time = from(Service.latency).longAvg(sum());"; + + assertThrows(IllegalArgumentException.class, () -> { + OALScriptParserV2.parse(script, "test.oal"); + }, "Parser should reject nested function calls"); + } + + /** + * Test that valid scripts parse without errors + */ + @Test + public void testValidScript_NoErrors() throws IOException { + String script = + "service_resp_time = from(Service.latency).longAvg();\n" + + "service_sla = from(Service.*).filter(status == true).percent();\n" + + "endpoint_calls = from(Endpoint.*).count();\n"; + + OALScriptParserV2 result = assertDoesNotThrow(() -> { + return OALScriptParserV2.parse(script, "valid.oal"); + }, "Valid script should parse without throwing exceptions"); + + assertTrue(result.hasMetrics(), "Valid script should produce metrics"); + assertTrue(result.getMetricsCount() == 3, "Should parse exactly 3 metrics"); + } + + /** + * Test mixed valid and invalid statements + */ + @Test + public void testMixedValidInvalid() { + String script = + "service_resp_time = from(Service.latency).longAvg();\n" + + "invalid_metric = from().badFunction();\n" + // Invalid - missing source + "service_sla = from(Service.*).percent();\n"; + + Exception exception = assertThrows(IllegalArgumentException.class, () -> { + OALScriptParserV2.parse(script, "mixed.oal"); + }, "Parser should fail on first syntax error"); + + String message = exception.getMessage(); + assertTrue(message.contains("mixed.oal"), "Error should reference file name"); + assertTrue(message.contains("2:"), "Error should reference line 2 where error occurs"); + } + + /** + * Test error message contains helpful information + */ + @Test + public void testErrorMessage_ContainsLocation() { + String script = + "service_resp_time = from(Service.latency).longAvg();\n" + + "service_sla = from(Service.*).filter(status == ).percent();\n"; // Error on line 2 + + Exception exception = assertThrows(IllegalArgumentException.class, () -> { + OALScriptParserV2.parse(script, "error.oal"); + }); + + String message = exception.getMessage(); + assertTrue(message.contains("error.oal"), "Error message should include file name"); + assertTrue(message.contains("2:"), "Error message should include line number"); + assertTrue(message.contains("OAL parsing failed"), "Error message should have clear header"); + } + + /** + * Test multiple errors are collected and reported + */ + @Test + public void testMultipleErrors_AllReported() { + String script = + "service_resp_time = from(Service.latency).longAvg()\n" + // Missing semicolon + "service_sla = from().percent();\n"; // Missing source + + Exception exception = assertThrows(IllegalArgumentException.class, () -> { + OALScriptParserV2.parse(script, "multi-error.oal"); + }); + + String message = exception.getMessage(); + // After first error, ANTLR may or may not continue - depends on error recovery + assertTrue(message.contains("multi-error.oal"), "Should reference file name"); + assertTrue(message.contains("error"), "Should indicate errors occurred"); + } +}
