This is an automated email from the ASF dual-hosted git repository. wusheng pushed a commit to branch groovy-replace in repository https://gitbox.apache.org/repos/asf/skywalking.git
commit 85b9fb0eaa743026540ad30eca70bb66723025ea Author: Wu Sheng <[email protected]> AuthorDate: Sun Mar 1 10:05:33 2026 +0800 Fix MAL/LAL/Hierarchy compiler gaps for full v1-v2 parity - Fix MAL sample collection regression: skip downsampling() method arguments to prevent enum values (MAX, SUM, MIN) from being collected as sample names - Fix MAL safe navigation (?.): parser now correctly propagates safeNav flag to chain segments; code generator uses local StringBuilder to avoid corrupting parent buffer - Fix MAL filter grammar: add closureCondition alternatives to closureBody rule for bare conditions like { tags -> tags.x == 'v' } - Fix MAL downsampling detection for bare identifiers parsed as ExprArgument wrapping MetricExpr - Fix MAL sample ordering: use LinkedHashSet for consistent order - Fix LAL tag() function call: add functionName rule allowing TAG token in functionInvocation for if(tag("LOG_KIND") == ...) patterns - Fix LAL ProcessRegistry support: add PROCESS_REGISTRY to valueAccessPrimary grammar rule - Fix LAL tag statement code generation: wrap single tag entries in Collections.singletonMap() since ExtractorSpec.tag() accepts Map - Fix LAL makeComparison to handle CondFunctionCallContext properly - Add debug logging to all three code generators (MAL, LAL, Hierarchy) showing AST and generated Java source at DEBUG level - Add generateFilterSource() to MALClassGenerator for testing - Add error handling unit tests with demo error comments for MAL (5), LAL (4), and Hierarchy (4) generators - All 1248 checker tests pass: MAL 1187, Filter 29, LAL 10, Hierarchy 22 Co-Authored-By: Claude Opus 4.6 <[email protected]> --- oap-server/analyzer/hierarchy/CLAUDE.md | 11 +- .../compiler/CompiledHierarchyRuleProvider.java | 68 +++++++ .../compiler/HierarchyRuleClassGenerator.java | 12 +- .../rule}/rt/HierarchyRulePackageHolder.java | 2 +- ...ierarchyDefinitionService$HierarchyRuleProvider | 1 + .../compiler/HierarchyRuleClassGeneratorTest.java | 36 ++++ .../apache/skywalking/lal/rt/grammar/LALParser.g4 | 8 +- .../log/analyzer/compiler/LALClassGenerator.java | 46 ++++- .../oap/log/analyzer/compiler/LALScriptParser.java | 20 +- .../skywalking/oap/log/analyzer/dsl/Binding.java | 19 +- .../skywalking/oap/log/analyzer/dsl/DSL.java | 115 +++-------- .../oap/log/analyzer/dsl/LalExpression.java | 9 +- .../log/analyzer/dsl/spec/filter/FilterSpec.java | 21 +++ .../oap/log/analyzer/provider/log/LogAnalyzer.java | 19 +- .../provider/log/listener/LogFilterListener.java | 23 +++ .../analyzer/compiler/LALClassGeneratorTest.java | 33 ++++ .../skywalking/oap/log/analyzer/dsl/DSLV2Test.java | 39 ++-- .../apache/skywalking/mal/rt/grammar/MALParser.g4 | 11 +- .../skywalking/oap/meter/analyzer/Analyzer.java | 40 +++- .../oap/meter/analyzer/MetricConvert.java | 33 +++- .../meter/analyzer/compiler/MALClassGenerator.java | 210 ++++++++++++++++++--- .../analyzer/compiler/MALExpressionModel.java | 2 +- .../meter/analyzer/compiler/MALScriptParser.java | 53 +++++- .../oap/meter/analyzer/dsl/Expression.java | 12 +- .../oap/meter/analyzer/dsl/FilterExpression.java | 63 +------ .../oap/meter/analyzer/dsl/MalExpression.java | 1 - .../oap/meter/analyzer/dsl/MalFilter.java | 3 +- .../meter/analyzer/dsl/SampleFamilyFunctions.java | 9 +- .../analyzer/compiler/MALClassGeneratorTest.java | 54 ++++++ oap-server/server-core/pom.xml | 4 - .../core/config/HierarchyDefinitionService.java | 85 +++++---- .../server/core/hierarchy/HierarchyService.java | 27 +++ .../core/config/HierarchyRuleComparisonTest.java | 3 +- .../oap/server/checker/lal/LalComparisonTest.java | 73 ++----- .../oap/server/checker/mal/MalComparisonTest.java | 110 ++++------- .../checker/mal/MalFilterComparisonTest.java | 61 ++---- .../oap/meter/analyzer/dsl/MalExpression.java | 10 +- 37 files changed, 878 insertions(+), 468 deletions(-) diff --git a/oap-server/analyzer/hierarchy/CLAUDE.md b/oap-server/analyzer/hierarchy/CLAUDE.md index f2bf5d1d9a..4038086264 100644 --- a/oap-server/analyzer/hierarchy/CLAUDE.md +++ b/oap-server/analyzer/hierarchy/CLAUDE.md @@ -34,9 +34,13 @@ oap-server/analyzer/hierarchy/ HierarchyRuleScriptParser.java — ANTLR4 facade: expression → AST HierarchyRuleModel.java — Immutable AST model classes HierarchyRuleClassGenerator.java — Javassist code generator - rt/ + CompiledHierarchyRuleProvider.java — SPI provider: compiles rule expressions + hierarchy/rule/rt/ HierarchyRulePackageHolder.java — Class loading anchor (empty marker) + src/main/resources/META-INF/services/ + ...HierarchyDefinitionService$HierarchyRuleProvider — SPI registration + src/test/java/.../compiler/ HierarchyRuleScriptParserTest.java — 5 parser tests HierarchyRuleClassGeneratorTest.java — 4 generator tests @@ -47,8 +51,9 @@ oap-server/analyzer/hierarchy/ | Component | Package / Name | |-----------|---------------| | Parser/Model/Generator | `org.apache.skywalking.oap.server.core.config.compiler` | -| Generated classes | `org.apache.skywalking.oap.server.core.config.compiler.rt.HierarchyRule_<N>` | -| Package holder | `org.apache.skywalking.oap.server.core.config.compiler.rt.HierarchyRulePackageHolder` | +| Generated classes | `org.apache.skywalking.oap.server.core.config.compiler.hierarchy.rule.rt.HierarchyRule_<N>` | +| Package holder | `org.apache.skywalking.oap.server.core.config.compiler.hierarchy.rule.rt.HierarchyRulePackageHolder` | +| SPI provider | `org.apache.skywalking.oap.server.core.config.compiler.CompiledHierarchyRuleProvider` | | Service type | `org.apache.skywalking.oap.server.core.query.type.Service` (in server-core) | `<N>` is a global `AtomicInteger` counter. diff --git a/oap-server/analyzer/hierarchy/src/main/java/org/apache/skywalking/oap/server/core/config/compiler/CompiledHierarchyRuleProvider.java b/oap-server/analyzer/hierarchy/src/main/java/org/apache/skywalking/oap/server/core/config/compiler/CompiledHierarchyRuleProvider.java new file mode 100644 index 0000000000..00c9b58975 --- /dev/null +++ b/oap-server/analyzer/hierarchy/src/main/java/org/apache/skywalking/oap/server/core/config/compiler/CompiledHierarchyRuleProvider.java @@ -0,0 +1,68 @@ +/* + * 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.oap.server.core.config.compiler; + +import java.util.HashMap; +import java.util.Map; +import java.util.function.BiFunction; +import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oap.server.core.config.HierarchyDefinitionService; +import org.apache.skywalking.oap.server.core.query.type.Service; + +/** + * SPI implementation of {@link HierarchyDefinitionService.HierarchyRuleProvider} + * that compiles hierarchy matching rule expressions using ANTLR4 + Javassist. + * + * <p>Discovered at startup via {@code ServiceLoader} by + * {@link HierarchyDefinitionService}. For each rule expression + * (e.g., {@code "{ (u, l) -> u.name == l.name }"}): + * <ol> + * <li>{@link HierarchyRuleClassGenerator#compile} parses the expression + * with ANTLR4 into an AST, then generates a Java class implementing + * {@code BiFunction<Service, Service, Boolean>} via Javassist.</li> + * <li>The generated class casts both arguments to {@link Service}, + * evaluates the expression body, and returns a {@code Boolean}.</li> + * </ol> + * + * <p>The compiled matchers are returned to {@link HierarchyDefinitionService} + * and used at runtime by + * {@link org.apache.skywalking.oap.server.core.hierarchy.HierarchyService} + * to match service pairs. + */ +@Slf4j +public class CompiledHierarchyRuleProvider implements HierarchyDefinitionService.HierarchyRuleProvider { + + private final HierarchyRuleClassGenerator generator = new HierarchyRuleClassGenerator(); + + @Override + public Map<String, BiFunction<Service, Service, Boolean>> buildRules( + final Map<String, String> ruleExpressions) { + final Map<String, BiFunction<Service, Service, Boolean>> rules = new HashMap<>(); + ruleExpressions.forEach((name, expression) -> { + try { + rules.put(name, generator.compile(name, expression)); + log.debug("Compiled hierarchy rule: {}", name); + } catch (Exception e) { + throw new IllegalStateException( + "Failed to compile hierarchy rule: " + name + + ", expression: " + expression, e); + } + }); + return rules; + } +} diff --git a/oap-server/analyzer/hierarchy/src/main/java/org/apache/skywalking/oap/server/core/config/compiler/HierarchyRuleClassGenerator.java b/oap-server/analyzer/hierarchy/src/main/java/org/apache/skywalking/oap/server/core/config/compiler/HierarchyRuleClassGenerator.java index 754bffa278..e6a460feb4 100644 --- a/oap-server/analyzer/hierarchy/src/main/java/org/apache/skywalking/oap/server/core/config/compiler/HierarchyRuleClassGenerator.java +++ b/oap-server/analyzer/hierarchy/src/main/java/org/apache/skywalking/oap/server/core/config/compiler/HierarchyRuleClassGenerator.java @@ -24,19 +24,21 @@ import javassist.ClassPool; import javassist.CtClass; import javassist.CtNewConstructor; import javassist.CtNewMethod; -import org.apache.skywalking.oap.server.core.config.compiler.rt.HierarchyRulePackageHolder; +import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oap.server.core.config.compiler.hierarchy.rule.rt.HierarchyRulePackageHolder; import org.apache.skywalking.oap.server.core.query.type.Service; /** * Generates {@link BiFunction BiFunction<Service, Service, Boolean>} implementation classes * from {@link HierarchyRuleModel} AST using Javassist bytecode generation. */ +@Slf4j public final class HierarchyRuleClassGenerator { private static final AtomicInteger CLASS_COUNTER = new AtomicInteger(0); private static final String PACKAGE_PREFIX = - "org.apache.skywalking.oap.server.core.config.compiler.rt."; + "org.apache.skywalking.oap.server.core.config.compiler.hierarchy.rule.rt."; private final ClassPool classPool; @@ -68,6 +70,12 @@ public final class HierarchyRuleClassGenerator { ctClass.addConstructor(CtNewConstructor.defaultConstructor(ctClass)); final String applyBody = generateApplyMethod(model); + + if (log.isDebugEnabled()) { + log.debug("Hierarchy compile [{}] AST: {}", ruleName, model); + log.debug("Hierarchy compile [{}] apply():\n{}", ruleName, applyBody); + } + ctClass.addMethod(CtNewMethod.make(applyBody, ctClass)); final Class<?> clazz = ctClass.toClass(HierarchyRulePackageHolder.class); diff --git a/oap-server/analyzer/hierarchy/src/main/java/org/apache/skywalking/oap/server/core/config/compiler/rt/HierarchyRulePackageHolder.java b/oap-server/analyzer/hierarchy/src/main/java/org/apache/skywalking/oap/server/core/config/compiler/hierarchy/rule/rt/HierarchyRulePackageHolder.java similarity index 92% rename from oap-server/analyzer/hierarchy/src/main/java/org/apache/skywalking/oap/server/core/config/compiler/rt/HierarchyRulePackageHolder.java rename to oap-server/analyzer/hierarchy/src/main/java/org/apache/skywalking/oap/server/core/config/compiler/hierarchy/rule/rt/HierarchyRulePackageHolder.java index 31d74d245d..367c1c541e 100644 --- a/oap-server/analyzer/hierarchy/src/main/java/org/apache/skywalking/oap/server/core/config/compiler/rt/HierarchyRulePackageHolder.java +++ b/oap-server/analyzer/hierarchy/src/main/java/org/apache/skywalking/oap/server/core/config/compiler/hierarchy/rule/rt/HierarchyRulePackageHolder.java @@ -15,7 +15,7 @@ * limitations under the License. */ -package org.apache.skywalking.oap.server.core.config.compiler.rt; +package org.apache.skywalking.oap.server.core.config.compiler.hierarchy.rule.rt; /** * Empty marker class used as the class loading anchor for Javassist diff --git a/oap-server/analyzer/hierarchy/src/main/resources/META-INF/services/org.apache.skywalking.oap.server.core.config.HierarchyDefinitionService$HierarchyRuleProvider b/oap-server/analyzer/hierarchy/src/main/resources/META-INF/services/org.apache.skywalking.oap.server.core.config.HierarchyDefinitionService$HierarchyRuleProvider new file mode 100644 index 0000000000..734565b725 --- /dev/null +++ b/oap-server/analyzer/hierarchy/src/main/resources/META-INF/services/org.apache.skywalking.oap.server.core.config.HierarchyDefinitionService$HierarchyRuleProvider @@ -0,0 +1 @@ +org.apache.skywalking.oap.server.core.config.compiler.CompiledHierarchyRuleProvider diff --git a/oap-server/analyzer/hierarchy/src/test/java/org/apache/skywalking/oap/server/core/config/compiler/HierarchyRuleClassGeneratorTest.java b/oap-server/analyzer/hierarchy/src/test/java/org/apache/skywalking/oap/server/core/config/compiler/HierarchyRuleClassGeneratorTest.java index 1054bcb27b..816d12be41 100644 --- a/oap-server/analyzer/hierarchy/src/test/java/org/apache/skywalking/oap/server/core/config/compiler/HierarchyRuleClassGeneratorTest.java +++ b/oap-server/analyzer/hierarchy/src/test/java/org/apache/skywalking/oap/server/core/config/compiler/HierarchyRuleClassGeneratorTest.java @@ -25,6 +25,7 @@ import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; class HierarchyRuleClassGeneratorTest { @@ -119,4 +120,39 @@ class HierarchyRuleClassGeneratorTest { upper.setShortName("no-port"); assertFalse(fn.apply(upper, lower)); } + + // ==================== Error handling tests ==================== + + @Test + void emptyExpressionThrows() { + // Demo error: Hierarchy rule parsing failed: 1:0 mismatched input '<EOF>' + // expecting '{' + assertThrows(Exception.class, + () -> generator.compile("empty", "")); + } + + @Test + void missingClosureBracesThrows() { + // Demo error: Hierarchy rule parsing failed: 1:0 mismatched input 'u' + // expecting '{' + assertThrows(Exception.class, + () -> generator.compile("test", "u.name == l.name")); + } + + @Test + void missingParametersThrows() { + // Demo error: Hierarchy rule parsing failed: 1:2 mismatched input '}' + // expecting '(' + assertThrows(Exception.class, + () -> generator.compile("test", "{ }")); + } + + @Test + void invalidFieldAccessThrows() { + // Demo error: [source error] getNonExistent() not found in Service + // (Javassist cannot find the getter for a non-existent field) + assertThrows(Exception.class, + () -> generator.compile("test", + "{ (u, l) -> u.nonExistent == l.nonExistent }")); + } } diff --git a/oap-server/analyzer/log-analyzer/src/main/antlr4/org/apache/skywalking/lal/rt/grammar/LALParser.g4 b/oap-server/analyzer/log-analyzer/src/main/antlr4/org/apache/skywalking/lal/rt/grammar/LALParser.g4 index b381aa1573..8c90cc344d 100644 --- a/oap-server/analyzer/log-analyzer/src/main/antlr4/org/apache/skywalking/lal/rt/grammar/LALParser.g4 +++ b/oap-server/analyzer/log-analyzer/src/main/antlr4/org/apache/skywalking/lal/rt/grammar/LALParser.g4 @@ -404,6 +404,7 @@ valueAccess valueAccessPrimary : PARSED # valueParsed | LOG # valueLog + | PROCESS_REGISTRY # valueProcessRegistry | IDENTIFIER # valueIdentifier | STRING # valueString | NUMBER # valueNumber @@ -420,7 +421,12 @@ valueAccessSegment ; functionInvocation - : IDENTIFIER L_PAREN functionArgList? R_PAREN + : functionName L_PAREN functionArgList? R_PAREN + ; + +functionName + : IDENTIFIER + | TAG ; functionArgList diff --git a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/compiler/LALClassGenerator.java b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/compiler/LALClassGenerator.java index 092d9f45f1..a5cdcab939 100644 --- a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/compiler/LALClassGenerator.java +++ b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/compiler/LALClassGenerator.java @@ -26,6 +26,7 @@ import javassist.CtClass; import javassist.CtField; import javassist.CtNewConstructor; import javassist.CtNewMethod; +import lombok.extern.slf4j.Slf4j; import org.apache.skywalking.oap.log.analyzer.compiler.rt.LalExpressionPackageHolder; import org.apache.skywalking.oap.log.analyzer.dsl.LalExpression; @@ -37,6 +38,7 @@ import org.apache.skywalking.oap.log.analyzer.dsl.LalExpression; * Consumer callbacks are pre-compiled as separate classes and * stored as fields on the main class. */ +@Slf4j public final class LALClassGenerator { private static final AtomicInteger CLASS_COUNTER = new AtomicInteger(0); @@ -106,6 +108,12 @@ public final class LALClassGenerator { // Phase 4: Generate execute method referencing consumer fields final int[] counter = {0}; final String executeBody = generateExecuteMethod(model, counter); + + if (log.isDebugEnabled()) { + log.debug("LAL compile AST: {}", model); + log.debug("LAL compile execute():\n{}", executeBody); + } + ctClass.addMethod(CtNewMethod.make(executeBody, ctClass)); final Class<?> clazz = ctClass.toClass(LalExpressionPackageHolder.class); @@ -221,16 +229,36 @@ public final class LALClassGenerator { } else if (stmt instanceof LALScriptModel.TagAssignment) { final LALScriptModel.TagAssignment tag = (LALScriptModel.TagAssignment) stmt; - if (tag.getTags().size() == 1) { - final Map.Entry<String, LALScriptModel.TagValue> entry = - tag.getTags().entrySet().iterator().next(); - sb.append(" _t.tag(\"") - .append(escapeJava(entry.getKey())).append("\", "); - generateCastedValueAccess(sb, entry.getValue().getValue(), - entry.getValue().getCastType()); - sb.append(");\n"); - } + generateTagAssignment(sb, tag); + } + } + } + + private void generateTagAssignment(final StringBuilder sb, + final LALScriptModel.TagAssignment tag) { + final Map<String, LALScriptModel.TagValue> tags = tag.getTags(); + if (tags.isEmpty()) { + return; + } + if (tags.size() == 1) { + final Map.Entry<String, LALScriptModel.TagValue> entry = + tags.entrySet().iterator().next(); + sb.append(" _t.tag(java.util.Collections.singletonMap(\"") + .append(escapeJava(entry.getKey())).append("\", "); + generateCastedValueAccess(sb, entry.getValue().getValue(), + entry.getValue().getCastType()); + sb.append("));\n"); + } else { + sb.append(" { java.util.Map _tagMap = new java.util.LinkedHashMap();\n"); + for (final Map.Entry<String, LALScriptModel.TagValue> entry + : tags.entrySet()) { + sb.append(" _tagMap.put(\"") + .append(escapeJava(entry.getKey())).append("\", "); + generateCastedValueAccess(sb, entry.getValue().getValue(), + entry.getValue().getCastType()); + sb.append(");\n"); } + sb.append(" _t.tag(_tagMap); }\n"); } } diff --git a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/compiler/LALScriptParser.java b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/compiler/LALScriptParser.java index 128f325573..9bd9d241c1 100644 --- a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/compiler/LALScriptParser.java +++ b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/compiler/LALScriptParser.java @@ -538,7 +538,15 @@ public final class LALScriptParser { return new ComparisonCondition(left, leftCast, op, visitConditionExprAsValue(rightCtx)); } - // For function calls and other forms, wrap as expression condition + if (leftCtx instanceof LALParser.CondFunctionCallContext) { + final LALParser.FunctionInvocationContext fi = + ((LALParser.CondFunctionCallContext) leftCtx).functionInvocation(); + final ValueAccess left = new ValueAccess( + List.of(fi.getText()), false, false, List.of()); + return new ComparisonCondition(left, null, op, + visitConditionExprAsValue(rightCtx)); + } + // For other forms, wrap as expression condition return new ExprCondition( new ValueAccess(List.of(leftCtx.getText()), false, false, List.of()), null); } @@ -592,6 +600,8 @@ public final class LALScriptParser { } else if (primary instanceof LALParser.ValueLogContext) { logRef = true; segments.add("log"); + } else if (primary instanceof LALParser.ValueProcessRegistryContext) { + segments.add("ProcessRegistry"); } else if (primary instanceof LALParser.ValueIdentifierContext) { segments.add(((LALParser.ValueIdentifierContext) primary).IDENTIFIER().getText()); } else if (primary instanceof LALParser.ValueStringContext) { @@ -622,15 +632,15 @@ public final class LALScriptParser { } else if (seg instanceof LALParser.SegmentMethodContext) { final LALParser.FunctionInvocationContext fi = ((LALParser.SegmentMethodContext) seg).functionInvocation(); - segments.add(fi.IDENTIFIER().getText() + "()"); + segments.add(fi.functionName().getText() + "()"); chain.add(new LALScriptModel.MethodSegment( - fi.IDENTIFIER().getText(), List.of(), false)); + fi.functionName().getText(), List.of(), false)); } else if (seg instanceof LALParser.SegmentSafeMethodContext) { final LALParser.FunctionInvocationContext fi = ((LALParser.SegmentSafeMethodContext) seg).functionInvocation(); - segments.add(fi.IDENTIFIER().getText() + "()"); + segments.add(fi.functionName().getText() + "()"); chain.add(new LALScriptModel.MethodSegment( - fi.IDENTIFIER().getText(), List.of(), true)); + fi.functionName().getText(), List.of(), true)); } } diff --git a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/Binding.java b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/Binding.java index ee2beaa474..41404fbdb0 100644 --- a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/Binding.java +++ b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/Binding.java @@ -33,9 +33,22 @@ import org.apache.skywalking.oap.server.analyzer.provider.trace.parser.listener. import org.apache.skywalking.oap.server.core.source.Log; /** - * Same-FQCN replacement for upstream Binding. - * Pure Java implementation that does not extend groovy.lang.Binding. - * Uses a simple HashMap for property storage instead of Groovy's binding mechanism. + * Mutable property storage for a single LAL script execution cycle. + * + * <p>A new Binding is created for each incoming log in + * {@link org.apache.skywalking.oap.log.analyzer.provider.log.listener.LogFilterListener#parse}. + * It carries all per-log state through the compiled LAL pipeline: + * <ul> + * <li>{@code log} — the incoming {@code LogData.Builder}</li> + * <li>{@code parsed} — structured data extracted by json/text/yaml parsers</li> + * <li>{@code save}/{@code abort} — control flags set by extractor/sink logic</li> + * <li>{@code metrics_container} — optional list for LAL-extracted metrics (log-MAL)</li> + * <li>{@code log_container} — optional container for the built {@code Log} source object</li> + * </ul> + * + * <p>The Binding is injected into {@link org.apache.skywalking.oap.log.analyzer.dsl.spec.AbstractSpec} + * via a ThreadLocal ({@code BINDING}), so all Spec methods ({@code FilterSpec}, {@code ExtractorSpec}, + * {@code SinkSpec}) can access the current log data without explicit parameter passing. */ public class Binding { public static final String KEY_LOG = "log"; diff --git a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/DSL.java b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/DSL.java index b3f4e4c977..a7d37b7b20 100644 --- a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/DSL.java +++ b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/DSL.java @@ -17,35 +17,45 @@ package org.apache.skywalking.oap.log.analyzer.dsl; -import java.io.BufferedReader; -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.nio.charset.StandardCharsets; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; -import java.util.HashMap; -import java.util.Map; -import java.util.concurrent.atomic.AtomicInteger; import lombok.AccessLevel; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oap.log.analyzer.compiler.LALClassGenerator; import org.apache.skywalking.oap.log.analyzer.dsl.spec.filter.FilterSpec; import org.apache.skywalking.oap.log.analyzer.provider.LogAnalyzerModuleConfig; import org.apache.skywalking.oap.server.library.module.ModuleManager; import org.apache.skywalking.oap.server.library.module.ModuleStartException; /** - * Same-FQCN replacement for upstream LAL DSL. - * Loads pre-compiled {@link LalExpression} classes from lal-expressions.txt manifest - * (keyed by SHA-256 hash) instead of Groovy {@code GroovyShell} runtime compilation. + * DSL compiles a LAL (Log Analysis Language) expression string into a + * {@link LalExpression} object and wraps it with runtime state management. + * + * <p>One DSL instance is created per LAL rule entry defined in a {@code .yaml} + * config file under {@code lal/}. Instances are compiled once at startup and + * reused for every incoming log. + * + * <p>Compilation ({@link #of}): + * <pre> + * LAL DSL string (e.g., "filter { json {} extractor { service ... } sink {} }") + * → LALClassGenerator.compile(dsl) + * → ANTLR4 parse → AST → Javassist bytecode → LalExpression class + * → new FilterSpec(moduleManager, config) + * → DSL(expression, filterSpec) + * </pre> + * + * <p>Runtime (per-log execution by {@link org.apache.skywalking.oap.log.analyzer.provider.log.listener.LogFilterListener}): + * <ol> + * <li>{@link #bind(Binding)} — sets the current log data into the {@link FilterSpec} + * via a ThreadLocal, making it available to all Spec methods.</li> + * <li>{@link #evaluate()} — invokes the compiled {@link LalExpression#execute}, + * which calls FilterSpec methods (json/text/yaml, extractor, sink) in the + * order defined by the LAL script.</li> + * </ol> */ @Slf4j @RequiredArgsConstructor(access = AccessLevel.PRIVATE) public class DSL { - private static final String MANIFEST_PATH = "META-INF/lal-expressions.txt"; - private static volatile Map<String, String> EXPRESSION_MAP; - private static final AtomicInteger LOADED_COUNT = new AtomicInteger(); + private static final LALClassGenerator GENERATOR = new LALClassGenerator(); private final LalExpression expression; private final FilterSpec filterSpec; @@ -54,28 +64,13 @@ public class DSL { public static DSL of(final ModuleManager moduleManager, final LogAnalyzerModuleConfig config, final String dsl) throws ModuleStartException { - final Map<String, String> exprMap = loadManifest(); - final String dslHash = sha256(dsl); - final String className = exprMap.get(dslHash); - if (className == null) { - throw new ModuleStartException( - "Pre-compiled LAL expression not found for DSL hash: " + dslHash - + ". Available: " + exprMap.size() + " expressions."); - } - try { - final Class<?> exprClass = Class.forName(className); - final LalExpression expression = (LalExpression) exprClass.getDeclaredConstructor().newInstance(); + final LalExpression expression = GENERATOR.compile(dsl); final FilterSpec filterSpec = new FilterSpec(moduleManager, config); - final int count = LOADED_COUNT.incrementAndGet(); - log.debug("Loaded pre-compiled LAL expression [{}/{}]: {}", count, exprMap.size(), className); return new DSL(expression, filterSpec); - } catch (ClassNotFoundException e) { + } catch (Exception e) { throw new ModuleStartException( - "Pre-compiled LAL expression class not found: " + className, e); - } catch (ReflectiveOperationException e) { - throw new ModuleStartException( - "Pre-compiled LAL expression instantiation failed: " + className, e); + "Failed to compile LAL expression: " + dsl, e); } } @@ -87,56 +82,4 @@ public class DSL { public void evaluate() { expression.execute(filterSpec, binding); } - - private static Map<String, String> loadManifest() { - if (EXPRESSION_MAP != null) { - return EXPRESSION_MAP; - } - synchronized (DSL.class) { - if (EXPRESSION_MAP != null) { - return EXPRESSION_MAP; - } - final Map<String, String> map = new HashMap<>(); - try (InputStream is = DSL.class.getClassLoader().getResourceAsStream(MANIFEST_PATH)) { - if (is == null) { - log.warn("LAL expression manifest not found: {}", MANIFEST_PATH); - EXPRESSION_MAP = map; - return map; - } - try (BufferedReader reader = new BufferedReader( - new InputStreamReader(is, StandardCharsets.UTF_8))) { - String line; - while ((line = reader.readLine()) != null) { - line = line.trim(); - if (line.isEmpty()) { - continue; - } - final String[] parts = line.split("=", 2); - if (parts.length == 2) { - map.put(parts[0], parts[1]); - } - } - } - } catch (IOException e) { - throw new IllegalStateException("Failed to load LAL expression manifest", e); - } - log.info("Loaded {} pre-compiled LAL expressions from manifest", map.size()); - EXPRESSION_MAP = map; - return map; - } - } - - static String sha256(final String input) { - try { - final MessageDigest digest = MessageDigest.getInstance("SHA-256"); - final byte[] hash = digest.digest(input.getBytes(StandardCharsets.UTF_8)); - final StringBuilder hex = new StringBuilder(); - for (final byte b : hash) { - hex.append(String.format("%02x", b)); - } - return hex.toString(); - } catch (NoSuchAlgorithmException e) { - throw new IllegalStateException("SHA-256 not available", e); - } - } } diff --git a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/LalExpression.java b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/LalExpression.java index f96b02f485..f40c7e95c0 100644 --- a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/LalExpression.java +++ b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/LalExpression.java @@ -21,8 +21,13 @@ package org.apache.skywalking.oap.log.analyzer.dsl; import org.apache.skywalking.oap.log.analyzer.dsl.spec.filter.FilterSpec; /** - * Pure Java replacement for Groovy-based LAL DelegatingScript. - * Each transpiled LAL expression implements this interface. + * Functional interface implemented by each compiled LAL class. + * + * <p>Generated at startup by + * {@link org.apache.skywalking.oap.log.analyzer.compiler.LALClassGenerator} + * via ANTLR4 parsing and Javassist bytecode generation. + * The generated {@code execute} method calls {@link FilterSpec} methods + * (json/text/yaml, extractor, sink) in the order defined by the LAL script. */ @FunctionalInterface public interface LalExpression { diff --git a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/spec/filter/FilterSpec.java b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/spec/filter/FilterSpec.java index c9e3338df6..34ac9a7db1 100644 --- a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/spec/filter/FilterSpec.java +++ b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/spec/filter/FilterSpec.java @@ -47,6 +47,27 @@ import org.apache.skywalking.oap.server.library.module.ModuleStartException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +/** + * The top-level runtime API that compiled LAL expressions invoke. + * + * <p>A compiled {@link org.apache.skywalking.oap.log.analyzer.dsl.LalExpression} + * calls methods on this class in the order defined by the LAL script: + * <ol> + * <li><b>Parser</b>: {@code json()}, {@code text()}, or {@code yaml()} — parses the log body + * into structured data stored in {@link Binding#parsed()}.</li> + * <li><b>Extractor</b>: {@code extractor(Consumer)} — extracts service name, layer, tags, + * metrics, slow SQL, sampled traces, etc.</li> + * <li><b>Sink</b>: {@code sink()} or {@code sink(Consumer)} — materializes the log into + * storage via {@link LogSinkListenerFactory} instances (RecordSinkListener, + * TrafficSinkListener), unless the log has been dropped or aborted.</li> + * </ol> + * + * <p>All methods read the current log data from the ThreadLocal {@code BINDING} + * (inherited from {@link AbstractSpec}), which is set by + * {@link org.apache.skywalking.oap.log.analyzer.dsl.DSL#bind(Binding)} before each execution. + * Every method checks {@code shouldAbort()} first and short-circuits if a previous + * step aborted the pipeline. + */ public class FilterSpec extends AbstractSpec { private static final Logger LOGGER = LoggerFactory.getLogger(FilterSpec.class); diff --git a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/provider/log/LogAnalyzer.java b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/provider/log/LogAnalyzer.java index 73909b1813..b9e5b57d70 100644 --- a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/provider/log/LogAnalyzer.java +++ b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/provider/log/LogAnalyzer.java @@ -33,7 +33,24 @@ import org.apache.skywalking.oap.log.analyzer.provider.log.listener.LogAnalysisL import org.apache.skywalking.oap.server.library.module.ModuleManager; /** - * Analyze the collected log data, is the entry point for log analysis. + * Entry point for log analysis. Created per-request by the log receiver. + * + * <p>Runtime execution ({@link #doAnalysis}): + * <ol> + * <li>Validates the incoming log (service name must be non-empty, layer must be valid).</li> + * <li>Calls {@code createAnalysisListeners(layer)} — asks all registered + * {@link org.apache.skywalking.oap.log.analyzer.provider.log.listener.LogAnalysisListenerFactory} + * instances to create listeners for the log's layer. For LAL, this is + * {@link org.apache.skywalking.oap.log.analyzer.provider.log.listener.LogFilterListener.Factory}, + * which returns a listener wrapping all compiled {@link org.apache.skywalking.oap.log.analyzer.dsl.DSL} + * instances for that layer.</li> + * <li>{@code notifyAnalysisListener(builder, extraLog)} — calls + * {@link org.apache.skywalking.oap.log.analyzer.provider.log.listener.LogAnalysisListener#parse} + * on each listener, which binds the log data to the compiled LAL scripts.</li> + * <li>{@code notifyAnalysisListenerToBuild()} — calls + * {@link org.apache.skywalking.oap.log.analyzer.provider.log.listener.LogAnalysisListener#build} + * on each listener, which evaluates the compiled LAL scripts (extractors, sinks).</li> + * </ol> */ @Slf4j @RequiredArgsConstructor diff --git a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/provider/log/listener/LogFilterListener.java b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/provider/log/listener/LogFilterListener.java index 8b9f0b1fb8..75af6b1012 100644 --- a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/provider/log/listener/LogFilterListener.java +++ b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/provider/log/listener/LogFilterListener.java @@ -38,6 +38,29 @@ import org.apache.skywalking.oap.server.core.analysis.Layer; import org.apache.skywalking.oap.server.library.module.ModuleManager; import org.apache.skywalking.oap.server.library.module.ModuleStartException; +/** + * Runtime listener that executes compiled LAL rules against incoming log data. + * + * <p>Each instance wraps a collection of {@link DSL} objects — one per LAL rule + * defined for a specific {@link Layer}. Created per-log by {@link Factory#create(Layer)}. + * + * <p>Two-phase execution (called by {@link org.apache.skywalking.oap.log.analyzer.provider.log.LogAnalyzer}): + * <ol> + * <li>{@link #parse} — creates a fresh {@link Binding} with the current log data + * and binds it to every DSL instance (sets the ThreadLocal in each Spec).</li> + * <li>{@link #build} — calls {@link DSL#evaluate()} on every DSL instance, + * which invokes the compiled {@link org.apache.skywalking.oap.log.analyzer.dsl.LalExpression} + * to run the filter/extractor/sink pipeline.</li> + * </ol> + * + * <p>The inner {@link Factory} is created once at startup by + * {@link org.apache.skywalking.oap.log.analyzer.provider.LogAnalyzerModuleProvider#start()}. + * It loads all {@code .yaml} LAL config files, compiles each rule's DSL string + * into a {@link DSL} instance via + * {@link DSL#of(org.apache.skywalking.oap.server.library.module.ModuleManager, + * org.apache.skywalking.oap.log.analyzer.provider.LogAnalyzerModuleConfig, String)}, + * and organizes them by {@link Layer}. + */ @Slf4j @RequiredArgsConstructor public class LogFilterListener implements LogAnalysisListener { diff --git a/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/compiler/LALClassGeneratorTest.java b/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/compiler/LALClassGeneratorTest.java index 7bde461a4f..11cee26a83 100644 --- a/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/compiler/LALClassGeneratorTest.java +++ b/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/compiler/LALClassGeneratorTest.java @@ -23,6 +23,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; class LALClassGeneratorTest { @@ -94,4 +95,36 @@ class LALClassGeneratorTest { org.junit.jupiter.api.Assertions.assertTrue( source.contains("filterSpec.sink()")); } + + // ==================== Error handling tests ==================== + + @Test + void emptyScriptThrows() { + // Demo error: LAL script parsing failed: 1:0 mismatched input '<EOF>' + // expecting 'filter' + assertThrows(Exception.class, () -> generator.compile("")); + } + + @Test + void missingFilterKeywordThrows() { + // Demo error: LAL script parsing failed: 1:0 extraneous input 'json' + // expecting 'filter' + assertThrows(Exception.class, () -> generator.compile("json {}")); + } + + @Test + void unclosedBraceThrows() { + // Demo error: LAL script parsing failed: 1:15 mismatched input '<EOF>' + // expecting '}' + assertThrows(Exception.class, + () -> generator.compile("filter { json {")); + } + + @Test + void invalidStatementInFilterThrows() { + // Demo error: LAL script parsing failed: 1:9 extraneous input 'invalid' + // expecting {'text', 'json', 'yaml', 'extractor', 'sink', 'abort', 'if', '}'} + assertThrows(Exception.class, + () -> generator.compile("filter { invalid {} }")); + } } diff --git a/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/dsl/DSLV2Test.java b/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/dsl/DSLV2Test.java index a8f19d382e..8a8178f6c6 100644 --- a/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/dsl/DSLV2Test.java +++ b/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/dsl/DSLV2Test.java @@ -17,44 +17,33 @@ package org.apache.skywalking.oap.log.analyzer.dsl; -import org.apache.skywalking.oap.server.library.module.ModuleStartException; +import org.apache.skywalking.oap.log.analyzer.compiler.LALClassGenerator; import org.junit.jupiter.api.Test; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; class DSLV2Test { @Test - void ofThrowsWhenManifestMissing() { - // No META-INF/lal-expressions.txt on test classpath - assertThrows(ModuleStartException.class, - () -> DSL.of(null, null, "filter { json {} sink {} }")); + void compileSimpleFilterExpression() throws Exception { + final LALClassGenerator generator = new LALClassGenerator(); + final LalExpression expr = generator.compile("filter { json {} sink {} }"); + assertNotNull(expr); } @Test - void sha256Deterministic() { - final String input = "filter { json {} sink {} }"; - final String hash1 = DSL.sha256(input); - final String hash2 = DSL.sha256(input); - assertNotNull(hash1); - assertEquals(64, hash1.length()); - assertEquals(hash1, hash2); + void compileFilterWithExtractor() throws Exception { + final LALClassGenerator generator = new LALClassGenerator(); + final LalExpression expr = generator.compile( + "filter { json {} extractor { service parsed.service as String } sink {} }"); + assertNotNull(expr); } @Test - void sha256DifferentInputsDifferentHashes() { - final String hash1 = DSL.sha256("filter { json {} sink {} }"); - final String hash2 = DSL.sha256("filter { text {} sink {} }"); - assertNotNull(hash1); - assertNotNull(hash2); - assertNotEquals(hash1, hash2); - } - - private static void assertNotEquals(final String a, final String b) { - if (a.equals(b)) { - throw new AssertionError("Expected different values but got: " + a); - } + void compileThrowsOnInvalidExpression() { + final LALClassGenerator generator = new LALClassGenerator(); + assertThrows(Exception.class, + () -> generator.compile("??? invalid !!!")); } } diff --git a/oap-server/analyzer/meter-analyzer/src/main/antlr4/org/apache/skywalking/mal/rt/grammar/MALParser.g4 b/oap-server/analyzer/meter-analyzer/src/main/antlr4/org/apache/skywalking/mal/rt/grammar/MALParser.g4 index 59af9b14d0..0306ec4a2b 100644 --- a/oap-server/analyzer/meter-analyzer/src/main/antlr4/org/apache/skywalking/mal/rt/grammar/MALParser.g4 +++ b/oap-server/analyzer/meter-analyzer/src/main/antlr4/org/apache/skywalking/mal/rt/grammar/MALParser.g4 @@ -37,6 +37,11 @@ expression : additiveExpression EOF ; +// A standalone filter closure: { tags -> tags.job_name == 'value' } +filterExpression + : closureExpression EOF + ; + // ==================== Arithmetic ==================== additiveExpression @@ -123,8 +128,10 @@ closureParams ; closureBody - : closureStatement+ - | L_BRACE closureStatement+ R_BRACE // optional extra braces: { tags -> { ... } } + : closureCondition // bare condition: { tags -> tags.x == 'v' } + | L_BRACE closureCondition R_BRACE // braced condition: { tags -> { tags.x == 'v' } } + | closureStatement+ + | L_BRACE closureStatement+ R_BRACE // optional extra braces: { tags -> { ... } } ; closureStatement diff --git a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/Analyzer.java b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/Analyzer.java index 0d698a9b6b..9b87b7d92a 100644 --- a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/Analyzer.java +++ b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/Analyzer.java @@ -69,6 +69,29 @@ import static java.util.stream.Collectors.toList; /** * Analyzer analyses DSL expression with input samples, then to generate meter-system metrics. + * + * <p>One Analyzer is created per {@code metricsRules} entry in a MAL config YAML file. + * + * <p>Initialization ({@link #build}): + * <ol> + * <li>Compiles the MAL expression string into a + * {@link org.apache.skywalking.oap.meter.analyzer.dsl.MalExpression MalExpression} + * via ANTLR4 + Javassist.</li> + * <li>Extracts compile-time {@link ExpressionMetadata} from the AST (sample names, scope type, + * aggregation labels, downsampling, histogram/percentile info).</li> + * <li>Registers the metric in {@link MeterSystem} (generates storage class via Javassist).</li> + * </ol> + * + * <p>Runtime ({@link #analyse}): + * <ol> + * <li>Receives the full {@code sampleFamilies} map (all metrics from one scrape).</li> + * <li>Selects only the entries matching {@code this.samples} (e.g., ["node_cpu_seconds_total"]). + * This is an O(n) lookup where n is the number of input metric names in the expression + * (typically 1-2), not the size of the full map.</li> + * <li>Applies the optional tag filter (e.g., {@code job_name == 'vm-monitoring'}).</li> + * <li>Executes the compiled MAL expression on the filtered input.</li> + * <li>Sends computed metric values to MeterSystem for storage.</li> + * </ol> */ @Slf4j @RequiredArgsConstructor(access = AccessLevel.PRIVATE) @@ -112,11 +135,17 @@ public class Analyzer { private int[] percentiles; /** - * analyse intends to parse expression with input samples to meter-system metrics. + * Analyse the full sample family map and produce meter-system metrics. + * + * <p>The {@code sampleFamilies} map contains ALL metrics from one scrape batch. + * This method first selects only the entries matching {@code this.samples} + * (the input metric names extracted from the MAL expression AST at compile time), + * then applies the optional filter and runs the expression on the selected subset. * - * @param sampleFamilies input samples. + * @param sampleFamilies all sample families from one scrape, keyed by metric name. */ public void analyse(final ImmutableMap<String, SampleFamily> sampleFamilies) { + // Select only the metric families this expression references (typically 1-2 keys). Map<String, SampleFamily> input = samples.stream() .map(s -> Tuple.of(s, sampleFamilies.get(s))) .filter(t -> t._2 != null) @@ -232,6 +261,13 @@ public class Analyzer { private final String literal; } + /** + * Initializes runtime state from compile-time metadata. + * + * <p>{@code ctx.getSamples()} provides the Prometheus metric names this expression references + * (e.g., ["node_cpu_seconds_total"]). These are used at runtime to select relevant entries + * from the full sample family map, avoiding unnecessary expression evaluation. + */ private void init() { this.samples = ctx.getSamples(); if (ctx.isHistogram()) { diff --git a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/MetricConvert.java b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/MetricConvert.java index 5c89f42b5e..ffcfa159b6 100644 --- a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/MetricConvert.java +++ b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/MetricConvert.java @@ -38,6 +38,30 @@ import static java.util.stream.Collectors.toList; /** * MetricConvert converts {@link SampleFamily} collection to meter-system metrics, then store them to backend storage. + * + * <p>One MetricConvert instance is created per MAL config YAML file (e.g., {@code vm.yaml}). + * It holds a list of {@link Analyzer}s, one per {@code metricsRules} entry in the YAML. + * + * <p>Construction (at startup): + * <pre> + * YAML file (e.g., vm.yaml) + * metricPrefix: meter_vm + * expSuffix: service(['host'], Layer.OS_LINUX) + * filter: { tags -> tags.job_name == 'vm-monitoring' } + * metricsRules: + * - name: cpu_total_percentage + * exp: (node_cpu_seconds_total * 100).sum(['host']).rate('PT1M') + * + * MetricConvert(rule, meterSystem) + * for each rule: + * metricName = metricPrefix + "_" + name → "meter_vm_cpu_total_percentage" + * finalExp = (exp).expSuffix → "(...).service(['host'], Layer.OS_LINUX)" + * → Analyzer.build(metricName, filter, finalExp, meterSystem) + * </pre> + * + * <p>Runtime ({@link #toMeter}): receives the full {@code sampleFamilies} map (all metrics + * from one scrape) and broadcasts it to every Analyzer. Each Analyzer self-filters to only + * the input metrics it needs (via {@code this.samples} from compile-time metadata). */ @Slf4j public class MetricConvert { @@ -95,9 +119,14 @@ public class MetricConvert { } /** - * toMeter transforms {@link SampleFamily} collection to meter-system metrics. + * Broadcasts the full sample family map to every Analyzer in this config file. + * + * <p>The map contains ALL metrics from a single scrape batch keyed by Prometheus metric name + * (e.g., "node_cpu_seconds_total", "node_memory_MemTotal_bytes", ...). + * Each Analyzer selects only the entries it needs via O(1) HashMap lookups on + * {@code this.samples} (derived from compile-time AST metadata). * - * @param sampleFamilies {@link SampleFamily} collection. + * @param sampleFamilies all sample families from one scrape, keyed by metric name. */ public void toMeter(final ImmutableMap<String, SampleFamily> sampleFamilies) { Preconditions.checkNotNull(sampleFamilies); diff --git a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/compiler/MALClassGenerator.java b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/compiler/MALClassGenerator.java index 0d42dd12aa..e2f10c7966 100644 --- a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/compiler/MALClassGenerator.java +++ b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/compiler/MALClassGenerator.java @@ -18,7 +18,6 @@ package org.apache.skywalking.oap.meter.analyzer.compiler; import java.util.ArrayList; -import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; @@ -27,10 +26,12 @@ import javassist.ClassPool; import javassist.CtClass; import javassist.CtNewConstructor; import javassist.CtNewMethod; +import lombok.extern.slf4j.Slf4j; import org.apache.skywalking.oap.meter.analyzer.compiler.rt.MalExpressionPackageHolder; import org.apache.skywalking.oap.meter.analyzer.dsl.DownsamplingType; import org.apache.skywalking.oap.meter.analyzer.dsl.ExpressionMetadata; import org.apache.skywalking.oap.meter.analyzer.dsl.MalExpression; +import org.apache.skywalking.oap.meter.analyzer.dsl.MalFilter; import org.apache.skywalking.oap.server.core.analysis.meter.ScopeType; /** @@ -42,6 +43,7 @@ import org.apache.skywalking.oap.server.core.analysis.meter.ScopeType; * SampleFamily run(Map<String, SampleFamily> samples) * </pre> */ +@Slf4j public final class MALClassGenerator { private static final AtomicInteger CLASS_COUNTER = new AtomicInteger(0); @@ -93,6 +95,73 @@ public final class MALClassGenerator { return compileFromModel(metricName, ast); } + /** + * Compiles a MAL filter closure into a {@link MalFilter} implementation. + * + * @param filterExpression e.g. {@code "{ tags -> tags.job_name == 'mysql-monitoring' }"} + * @return a MalFilter instance + */ + @SuppressWarnings("unchecked") + public MalFilter compileFilter(final String filterExpression) throws Exception { + final MALExpressionModel.ClosureArgument closure = + MALScriptParser.parseFilter(filterExpression); + + final String className = PACKAGE_PREFIX + "MalFilter_" + + CLASS_COUNTER.getAndIncrement(); + + final CtClass ctClass = classPool.makeClass(className); + ctClass.addInterface(classPool.get( + "org.apache.skywalking.oap.meter.analyzer.dsl.MalFilter")); + + ctClass.addConstructor(CtNewConstructor.defaultConstructor(ctClass)); + + final List<String> params = closure.getParams(); + final String paramName = params.isEmpty() ? "it" : params.get(0); + + final StringBuilder sb = new StringBuilder(); + sb.append("public boolean test(java.util.Map ").append(paramName) + .append(") {\n"); + + final List<MALExpressionModel.ClosureStatement> body = closure.getBody(); + if (body.size() == 1 + && body.get(0) instanceof MALExpressionModel.ClosureExprStatement) { + // Single expression → evaluate as condition and return boolean + final MALExpressionModel.ClosureExpr expr = + ((MALExpressionModel.ClosureExprStatement) body.get(0)).getExpr(); + if (expr instanceof MALExpressionModel.ClosureCondition) { + sb.append(" return "); + generateClosureCondition( + sb, (MALExpressionModel.ClosureCondition) expr, paramName); + sb.append(";\n"); + } else { + // Truthy evaluation of the expression + sb.append(" Object _v = "); + generateClosureExpr(sb, expr, paramName); + sb.append(";\n"); + sb.append(" return _v != null && !Boolean.FALSE.equals(_v);\n"); + } + } else { + // Multi-statement body — generate statements, last expression is the return + for (final MALExpressionModel.ClosureStatement stmt : body) { + generateClosureStatement(sb, stmt, paramName); + } + sb.append(" return false;\n"); + } + sb.append("}\n"); + + final String filterBody = sb.toString(); + if (log.isDebugEnabled()) { + log.debug("MAL compileFilter AST: {}", closure); + log.debug("MAL compileFilter test():\n{}", filterBody); + } + + ctClass.addMethod(CtNewMethod.make(filterBody, ctClass)); + + final Class<?> clazz = ctClass.toClass(MalExpressionPackageHolder.class); + ctClass.detach(); + return (MalFilter) clazz.getDeclaredConstructor().newInstance(); + } + /** * Compiles from a pre-parsed AST model. */ @@ -128,10 +197,16 @@ public final class MALClassGenerator { ctClass.addConstructor(CtNewConstructor.defaultConstructor(ctClass)); final String runBody = generateRunMethod(ast); - ctClass.addMethod(CtNewMethod.make(runBody, ctClass)); - final ExpressionMetadata metadata = extractMetadata(ast); final String metadataBody = generateMetadataMethod(metadata); + + if (log.isDebugEnabled()) { + log.debug("MAL compile [{}] AST: {}", metricName, ast); + log.debug("MAL compile [{}] run():\n{}", metricName, runBody); + log.debug("MAL compile [{}] metadata():\n{}", metricName, metadataBody); + } + + ctClass.addMethod(CtNewMethod.make(runBody, ctClass)); ctClass.addMethod(CtNewMethod.make(metadataBody, ctClass)); final Class<?> clazz = ctClass.toClass(MalExpressionPackageHolder.class); @@ -599,32 +674,49 @@ public final class MALClassGenerator { ((MALExpressionModel.ClosureIndexAccess) segs.get(0)).getIndex(), paramName); sb.append(")"); } else { - // General chain - sb.append(chain.getTarget()); + // General chain: build in a local buffer to support safe navigation + final StringBuilder local = new StringBuilder(); + local.append(chain.getTarget()); for (final MALExpressionModel.ClosureChainSegment seg : segs) { if (seg instanceof MALExpressionModel.ClosureFieldAccess) { - sb.append(".get(\"") + local.append(".get(\"") .append(escapeJava( ((MALExpressionModel.ClosureFieldAccess) seg).getName())) .append("\")"); } else if (seg instanceof MALExpressionModel.ClosureIndexAccess) { - sb.append(".get("); - generateClosureExpr(sb, + local.append(".get("); + generateClosureExpr(local, ((MALExpressionModel.ClosureIndexAccess) seg).getIndex(), paramName); - sb.append(")"); + local.append(")"); } else if (seg instanceof MALExpressionModel.ClosureMethodCallSeg) { final MALExpressionModel.ClosureMethodCallSeg mc = (MALExpressionModel.ClosureMethodCallSeg) seg; - sb.append('.').append(mc.getName()).append('('); - for (int i = 0; i < mc.getArguments().size(); i++) { - if (i > 0) { - sb.append(", "); + if (mc.isSafeNav()) { + final String prior = local.toString(); + local.setLength(0); + local.append("(").append(prior).append(" == null ? null : ") + .append("((String) ").append(prior).append(").") + .append(mc.getName()).append('('); + for (int i = 0; i < mc.getArguments().size(); i++) { + if (i > 0) { + local.append(", "); + } + generateClosureExpr(local, mc.getArguments().get(i), paramName); + } + local.append("))"); + } else { + local.append('.').append(mc.getName()).append('('); + for (int i = 0; i < mc.getArguments().size(); i++) { + if (i > 0) { + local.append(", "); + } + generateClosureExpr(local, mc.getArguments().get(i), paramName); } - generateClosureExpr(sb, mc.getArguments().get(i), paramName); + local.append(')'); } - sb.append(')'); } } + sb.append(local); } } @@ -693,7 +785,9 @@ public final class MALClassGenerator { private static void collectSampleNames(final MALExpressionModel.Expr expr, final Set<String> names) { if (expr instanceof MALExpressionModel.MetricExpr) { - names.add(((MALExpressionModel.MetricExpr) expr).getMetricName()); + final MALExpressionModel.MetricExpr me = (MALExpressionModel.MetricExpr) expr; + names.add(me.getMetricName()); + collectSampleNamesFromChain(me.getMethodChain(), names); } else if (expr instanceof MALExpressionModel.BinaryExpr) { collectSampleNames(((MALExpressionModel.BinaryExpr) expr).getLeft(), names); collectSampleNames(((MALExpressionModel.BinaryExpr) expr).getRight(), names); @@ -701,8 +795,10 @@ public final class MALClassGenerator { collectSampleNames( ((MALExpressionModel.UnaryNegExpr) expr).getOperand(), names); } else if (expr instanceof MALExpressionModel.ParenChainExpr) { - collectSampleNames( - ((MALExpressionModel.ParenChainExpr) expr).getInner(), names); + final MALExpressionModel.ParenChainExpr pce = + (MALExpressionModel.ParenChainExpr) expr; + collectSampleNames(pce.getInner(), names); + collectSampleNamesFromChain(pce.getMethodChain(), names); } else if (expr instanceof MALExpressionModel.FunctionCallExpr) { for (final MALExpressionModel.Argument arg : ((MALExpressionModel.FunctionCallExpr) expr).getArguments()) { @@ -714,11 +810,27 @@ public final class MALClassGenerator { } } + private static void collectSampleNamesFromChain( + final List<MALExpressionModel.MethodCall> chain, + final Set<String> names) { + for (final MALExpressionModel.MethodCall mc : chain) { + if ("downsampling".equals(mc.getName())) { + continue; + } + for (final MALExpressionModel.Argument arg : mc.getArguments()) { + if (arg instanceof MALExpressionModel.ExprArgument) { + collectSampleNames( + ((MALExpressionModel.ExprArgument) arg).getExpr(), names); + } + } + } + } + /** * Extracts compile-time metadata from the AST by walking all method chains. */ static ExpressionMetadata extractMetadata(final MALExpressionModel.Expr ast) { - final Set<String> sampleNames = new HashSet<>(); + final Set<String> sampleNames = new LinkedHashSet<>(); collectSampleNames(ast, sampleNames); ScopeType scopeType = null; @@ -786,11 +898,21 @@ public final class MALClassGenerator { } break; case "downsampling": - if (!mc.getArguments().isEmpty() - && mc.getArguments().get(0) instanceof MALExpressionModel.EnumRefArgument) { - final String val = - ((MALExpressionModel.EnumRefArgument) mc.getArguments().get(0)).getEnumValue(); - downsampling = DownsamplingType.valueOf(val); + if (!mc.getArguments().isEmpty()) { + final MALExpressionModel.Argument dsArg = mc.getArguments().get(0); + if (dsArg instanceof MALExpressionModel.EnumRefArgument) { + final String val = + ((MALExpressionModel.EnumRefArgument) dsArg).getEnumValue(); + downsampling = DownsamplingType.valueOf(val); + } else if (dsArg instanceof MALExpressionModel.ExprArgument) { + final MALExpressionModel.Expr dsExpr = + ((MALExpressionModel.ExprArgument) dsArg).getExpr(); + if (dsExpr instanceof MALExpressionModel.MetricExpr) { + final String val = + ((MALExpressionModel.MetricExpr) dsExpr).getMetricName(); + downsampling = DownsamplingType.valueOf(val); + } + } } break; default: @@ -968,4 +1090,44 @@ public final class MALClassGenerator { final MALExpressionModel.Expr ast = MALScriptParser.parse(expression); return generateRunMethod(ast); } + + /** + * Generates the Java source body of the filter test method for debugging/testing. + */ + public String generateFilterSource(final String filterExpression) { + final MALExpressionModel.ClosureArgument closure = + MALScriptParser.parseFilter(filterExpression); + + final List<String> params = closure.getParams(); + final String paramName = params.isEmpty() ? "it" : params.get(0); + + final StringBuilder sb = new StringBuilder(); + sb.append("public boolean test(java.util.Map ").append(paramName) + .append(") {\n"); + + final List<MALExpressionModel.ClosureStatement> body = closure.getBody(); + if (body.size() == 1 + && body.get(0) instanceof MALExpressionModel.ClosureExprStatement) { + final MALExpressionModel.ClosureExpr expr = + ((MALExpressionModel.ClosureExprStatement) body.get(0)).getExpr(); + if (expr instanceof MALExpressionModel.ClosureCondition) { + sb.append(" return "); + generateClosureCondition( + sb, (MALExpressionModel.ClosureCondition) expr, paramName); + sb.append(";\n"); + } else { + sb.append(" Object _v = "); + generateClosureExpr(sb, expr, paramName); + sb.append(";\n"); + sb.append(" return _v != null && !Boolean.FALSE.equals(_v);\n"); + } + } else { + for (final MALExpressionModel.ClosureStatement stmt : body) { + generateClosureStatement(sb, stmt, paramName); + } + sb.append(" return false;\n"); + } + sb.append("}\n"); + return sb.toString(); + } } diff --git a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/compiler/MALExpressionModel.java b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/compiler/MALExpressionModel.java index 3ba103ed83..455b5db178 100644 --- a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/compiler/MALExpressionModel.java +++ b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/compiler/MALExpressionModel.java @@ -399,7 +399,7 @@ public final class MALExpressionModel { // ==================== Closure conditions ==================== - public interface ClosureCondition { + public interface ClosureCondition extends ClosureExpr { } @Getter diff --git a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/compiler/MALScriptParser.java b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/compiler/MALScriptParser.java index ed8c7e04fb..2c21670387 100644 --- a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/compiler/MALScriptParser.java +++ b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/compiler/MALScriptParser.java @@ -108,6 +108,40 @@ public final class MALScriptParser { return new MALExprVisitor().visit(tree.additiveExpression()); } + /** + * Parse a standalone filter closure expression into a {@link ClosureArgument}. + * + * @param filterExpression e.g. {@code "{ tags -> tags.job_name == 'mysql-monitoring' }"} + */ + public static ClosureArgument parseFilter(final String filterExpression) { + final MALLexer lexer = new MALLexer(CharStreams.fromString(filterExpression)); + final CommonTokenStream tokens = new CommonTokenStream(lexer); + final MALParser parser = new MALParser(tokens); + + final List<String> errors = new ArrayList<>(); + parser.removeErrorListeners(); + parser.addErrorListener(new BaseErrorListener() { + @Override + public void syntaxError(final Recognizer<?, ?> recognizer, + final Object offendingSymbol, + final int line, + final int charPositionInLine, + final String msg, + final RecognitionException e) { + errors.add(line + ":" + charPositionInLine + " " + msg); + } + }); + + final MALParser.FilterExpressionContext tree = parser.filterExpression(); + if (!errors.isEmpty()) { + throw new IllegalArgumentException( + "MAL filter expression parsing failed: " + String.join("; ", errors) + + " in expression: " + filterExpression); + } + + return new ClosureVisitor().visitClosure(tree.closureExpression()); + } + /** * Visitor transforming ANTLR4 parse tree into MAL expression AST. */ @@ -246,6 +280,11 @@ public final class MALScriptParser { private List<ClosureStatement> convertClosureBody( final MALParser.ClosureBodyContext ctx) { + // Bare condition or braced condition: { tags -> tags.x == 'v' } + if (ctx.closureCondition() != null) { + final ClosureCondition cond = convertCondition(ctx.closureCondition()); + return List.of(new ClosureExprStatement(cond)); + } final List<ClosureStatement> stmts = new ArrayList<>(); for (final MALParser.ClosureStatementContext stmtCtx : ctx.closureStatement()) { stmts.add(convertClosureStatement(stmtCtx)); @@ -444,12 +483,14 @@ public final class MALScriptParser { final String target = ctx.closureTarget().IDENTIFIER().getText(); final List<ClosureChainSegment> segments = new ArrayList<>(); - for (final MALParser.ClosureChainSegmentContext seg : ctx.closureChainSegment()) { - final boolean safeNav = segments.size() < ctx.closureChainSegment().size() - && ctx.safeNav().size() > 0; - // Determine if this segment uses safe navigation - // by checking position relative to safeNav tokens - segments.add(convertClosureChainSegment(seg, false)); + final int totalSegs = ctx.closureChainSegment().size(); + final int safeNavCount = ctx.safeNav().size(); + final int dotSegCount = totalSegs - safeNavCount; + + for (int i = 0; i < totalSegs; i++) { + final boolean isSafeNav = i >= dotSegCount; + segments.add(convertClosureChainSegment( + ctx.closureChainSegment().get(i), isSafeNav)); } return new ClosureMethodChain(target, segments); diff --git a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/Expression.java b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/Expression.java index 2701f8337c..2081935639 100644 --- a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/Expression.java +++ b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/Expression.java @@ -22,8 +22,16 @@ import lombok.ToString; import lombok.extern.slf4j.Slf4j; /** - * Same-FQCN replacement for upstream Expression. - * Wraps a compiled {@link MalExpression} (pure Java) instead of a Groovy DelegatingScript. + * Wraps a compiled {@link MalExpression} with runtime state management. + * + * <p>Two-phase usage: + * <ul> + * <li>{@link #parse()} — returns compile-time {@link ExpressionMetadata} extracted from the AST. + * Called once at startup by {@link org.apache.skywalking.oap.meter.analyzer.Analyzer#build} + * to discover sample names, scope type, aggregation labels, and metric type.</li> + * <li>{@link #run(Map)} — executes the compiled expression on actual sample data. + * Called at every ingestion cycle. Pure computation, no side effects.</li> + * </ul> */ @Slf4j @ToString(of = {"literal"}) diff --git a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/FilterExpression.java b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/FilterExpression.java index b92318d9d3..320cc1e656 100644 --- a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/FilterExpression.java +++ b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/FilterExpression.java @@ -17,54 +17,32 @@ package org.apache.skywalking.oap.meter.analyzer.dsl; -import java.io.IOException; -import java.io.InputStream; import java.util.HashMap; import java.util.Map; import java.util.Objects; -import java.util.Properties; -import java.util.concurrent.atomic.AtomicInteger; import lombok.ToString; import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oap.meter.analyzer.compiler.MALClassGenerator; /** - * Same-FQCN replacement for upstream FilterExpression. - * Loads transpiled {@link MalFilter} classes from mal-filter-expressions.properties - * manifest instead of Groovy filter closures -- no Groovy runtime needed. + * Compiles a MAL filter closure expression into a {@link MalFilter} + * using ANTLR4 parsing and Javassist bytecode generation. */ @Slf4j @ToString(of = {"literal"}) public class FilterExpression { - private static final String MANIFEST_PATH = "META-INF/mal-filter-expressions.properties"; - private static volatile Map<String, String> FILTER_MAP; - private static final AtomicInteger LOADED_COUNT = new AtomicInteger(); + private static final MALClassGenerator GENERATOR = new MALClassGenerator(); private final String literal; private final MalFilter malFilter; - @SuppressWarnings("unchecked") public FilterExpression(final String literal) { this.literal = literal; - - final Map<String, String> filterMap = loadManifest(); - final String className = filterMap.get(literal); - if (className == null) { - throw new IllegalStateException( - "Transpiled MAL filter not found for: " + literal - + ". Available filters: " + filterMap.size()); - } - try { - final Class<?> filterClass = Class.forName(className); - malFilter = (MalFilter) filterClass.getDeclaredConstructor().newInstance(); - final int count = LOADED_COUNT.incrementAndGet(); - log.debug("Loaded transpiled MAL filter [{}/{}]: {}", count, filterMap.size(), literal); - } catch (ClassNotFoundException e) { - throw new IllegalStateException( - "Transpiled MAL filter class not found: " + className, e); - } catch (ReflectiveOperationException e) { + this.malFilter = GENERATOR.compileFilter(literal); + } catch (Exception e) { throw new IllegalStateException( - "Failed to instantiate transpiled MAL filter: " + className, e); + "Failed to compile MAL filter expression: " + literal, e); } } @@ -83,31 +61,4 @@ public class FilterExpression { } return sampleFamilies; } - - private static Map<String, String> loadManifest() { - if (FILTER_MAP != null) { - return FILTER_MAP; - } - synchronized (FilterExpression.class) { - if (FILTER_MAP != null) { - return FILTER_MAP; - } - final Map<String, String> map = new HashMap<>(); - try (InputStream is = FilterExpression.class.getClassLoader().getResourceAsStream(MANIFEST_PATH)) { - if (is == null) { - log.warn("MAL filter manifest not found: {}", MANIFEST_PATH); - FILTER_MAP = map; - return map; - } - final Properties props = new Properties(); - props.load(is); - props.forEach((k, v) -> map.put((String) k, (String) v)); - } catch (IOException e) { - throw new IllegalStateException("Failed to load MAL filter manifest", e); - } - log.info("Loaded {} transpiled MAL filters from manifest", map.size()); - FILTER_MAP = map; - return map; - } - } } diff --git a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/MalExpression.java b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/MalExpression.java index 2400cf319a..66737dfea7 100644 --- a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/MalExpression.java +++ b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/MalExpression.java @@ -21,7 +21,6 @@ package org.apache.skywalking.oap.meter.analyzer.dsl; import java.util.Map; /** - * Pure Java replacement for Groovy-based MAL DelegatingScript. * Each compiled MAL expression implements this interface. */ public interface MalExpression { diff --git a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/MalFilter.java b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/MalFilter.java index 9d8eaa259e..c221fcba40 100644 --- a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/MalFilter.java +++ b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/MalFilter.java @@ -21,8 +21,7 @@ package org.apache.skywalking.oap.meter.analyzer.dsl; import java.util.Map; /** - * Pure Java replacement for Groovy Closure-based MAL filter expressions. - * Each transpiled filter expression implements this interface. + * Each compiled MAL filter expression implements this interface. */ @FunctionalInterface public interface MalFilter { diff --git a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/SampleFamilyFunctions.java b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/SampleFamilyFunctions.java index 1c9747b56a..752eb0f898 100644 --- a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/SampleFamilyFunctions.java +++ b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/SampleFamilyFunctions.java @@ -25,7 +25,7 @@ import java.util.function.Predicate; import org.apache.skywalking.oap.server.core.analysis.meter.MeterEntity; /** - * Pure Java functional interfaces replacing Groovy Closure parameters in SampleFamily methods. + * Functional interfaces used as parameters in {@link SampleFamily} methods. */ public final class SampleFamilyFunctions { @@ -33,7 +33,6 @@ public final class SampleFamilyFunctions { } /** - * Replaces {@code Closure<?>} in {@link SampleFamily#tag(groovy.lang.Closure)}. * Receives a mutable label map and returns the (possibly modified) map. */ @FunctionalInterface @@ -41,7 +40,6 @@ public final class SampleFamilyFunctions { } /** - * Replaces {@code Closure<Boolean>} in {@link SampleFamily#filter(groovy.lang.Closure)}. * Tests whether a sample's labels match the filter criteria. */ @FunctionalInterface @@ -49,7 +47,6 @@ public final class SampleFamilyFunctions { } /** - * Replaces {@code Closure<Void>} in {@link SampleFamily#forEach(java.util.List, groovy.lang.Closure)}. * Called for each element in the array with the element value and a mutable labels map. */ @FunctionalInterface @@ -58,7 +55,6 @@ public final class SampleFamilyFunctions { } /** - * Replaces {@code Closure<Void>} in {@link SampleFamily#decorate(groovy.lang.Closure)}. * Decorates service meter entities. */ @FunctionalInterface @@ -66,9 +62,6 @@ public final class SampleFamilyFunctions { } /** - * Replaces {@code Closure<Map<String, String>>} in - * {@link SampleFamily#instance(java.util.List, String, java.util.List, String, - * org.apache.skywalking.oap.server.core.analysis.Layer, groovy.lang.Closure)}. * Extracts instance properties from sample labels. */ @FunctionalInterface diff --git a/oap-server/analyzer/meter-analyzer/src/test/java/org/apache/skywalking/oap/meter/analyzer/compiler/MALClassGeneratorTest.java b/oap-server/analyzer/meter-analyzer/src/test/java/org/apache/skywalking/oap/meter/analyzer/compiler/MALClassGeneratorTest.java index 3e6cbbb8f5..11b96642bb 100644 --- a/oap-server/analyzer/meter-analyzer/src/test/java/org/apache/skywalking/oap/meter/analyzer/compiler/MALClassGeneratorTest.java +++ b/oap-server/analyzer/meter-analyzer/src/test/java/org/apache/skywalking/oap/meter/analyzer/compiler/MALClassGeneratorTest.java @@ -23,6 +23,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; class MALClassGeneratorTest { @@ -112,4 +114,56 @@ class MALClassGeneratorTest { org.junit.jupiter.api.Assertions.assertTrue( source.contains("getOrDefault")); } + + @Test + void filterSafeNavCompiles() throws Exception { + final String source = generator.generateFilterSource( + "{ tags -> tags.job_name == 'aws-cloud-eks-monitoring'" + + " && tags.Service?.trim() }"); + assertNotNull(source); + assertTrue(source.contains("trim"), "Generated source should contain trim()"); + assertNotNull(generator.compileFilter( + "{ tags -> tags.job_name == 'aws-cloud-eks-monitoring'" + + " && tags.Service?.trim() }")); + } + + // ==================== Error handling tests ==================== + + @Test + void emptyExpressionThrows() { + // Demo error: MAL expression parsing failed: 1:0 mismatched input '<EOF>' + // expecting {IDENTIFIER, NUMBER, '(', '-'} + assertThrows(Exception.class, () -> generator.compile("test", "")); + } + + @Test + void malformedExpressionThrows() { + // Demo error: MAL expression parsing failed: 1:7 token recognition error at: '@' + assertThrows(Exception.class, + () -> generator.compile("test", "metric.@invalid")); + } + + @Test + void unclosedParenthesisThrows() { + // Demo error: MAL expression parsing failed: 1:8 mismatched input '<EOF>' + // expecting {')', '+', '-', '*', '/'} + assertThrows(Exception.class, + () -> generator.compile("test", "(metric1 ")); + } + + @Test + void invalidFilterClosureThrows() { + // Demo error: MAL filter parsing failed: 1:0 mismatched input 'invalid' + // expecting '{' + assertThrows(Exception.class, + () -> generator.compileFilter("invalid filter")); + } + + @Test + void emptyFilterBodyThrows() { + // Demo error: MAL filter parsing failed: 1:1 mismatched input '}' + // expecting {IDENTIFIER, ...} + assertThrows(Exception.class, + () -> generator.compileFilter("{ }")); + } } diff --git a/oap-server/server-core/pom.xml b/oap-server/server-core/pom.xml index 056f3c9fa3..fd12143671 100644 --- a/oap-server/server-core/pom.xml +++ b/oap-server/server-core/pom.xml @@ -107,10 +107,6 @@ <groupId>io.zipkin.zipkin2</groupId> <artifactId>zipkin</artifactId> </dependency> - <dependency> - <groupId>org.apache.groovy</groupId> - <artifactId>groovy</artifactId> - </dependency> </dependencies> <build> diff --git a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/HierarchyDefinitionService.java b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/HierarchyDefinitionService.java index 737e50ebed..f84cb3cd9c 100644 --- a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/HierarchyDefinitionService.java +++ b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/HierarchyDefinitionService.java @@ -22,7 +22,7 @@ import java.io.FileNotFoundException; import java.io.Reader; import java.util.HashMap; import java.util.Map; -import java.util.Objects; +import java.util.ServiceLoader; import java.util.function.BiFunction; import lombok.Getter; import lombok.extern.slf4j.Slf4j; @@ -35,12 +35,38 @@ import org.yaml.snakeyaml.Yaml; import static java.util.stream.Collectors.toMap; +/** + * Loads hierarchy definitions from {@code hierarchy-definition.yml} and compiles + * matching rules into executable {@code BiFunction<Service, Service, Boolean>} + * matchers via a pluggable {@link HierarchyRuleProvider} (discovered through Java SPI). + * + * <p>Initialization (at startup, in CoreModuleProvider): + * <ol> + * <li>Reads {@code hierarchy-definition.yml} containing three sections: + * {@code hierarchy} (layer-to-lower-layer mapping with rule names), + * {@code auto-matching-rules} (rule name to expression string), + * and {@code layer-levels} (layer to numeric level).</li> + * <li>Discovers a {@link HierarchyRuleProvider} via {@code ServiceLoader} + * (e.g., {@code CompiledHierarchyRuleProvider} from the hierarchy module).</li> + * <li>Calls {@link HierarchyRuleProvider#buildRules} which compiles each rule + * expression (e.g., {@code "{ (u, l) -> u.name == l.name }"}) into a + * {@code BiFunction} via ANTLR4 + Javassist.</li> + * <li>Wraps each compiled matcher in a {@link MatchingRule} and maps them + * to the layer hierarchy structure.</li> + * <li>Validates all layers exist in the {@code Layer} enum and that upper + * layers have higher level numbers than their lower layers.</li> + * </ol> + * + * <p>The resulting {@link #getHierarchyDefinition()} map is consumed by + * {@link org.apache.skywalking.oap.server.core.hierarchy.HierarchyService} + * for runtime service matching. + */ @Slf4j public class HierarchyDefinitionService implements org.apache.skywalking.oap.server.library.module.Service { /** * Functional interface for building hierarchy matching rules. - * Implementations are provided by hierarchy-v1 (Groovy) or hierarchy-v2 (pure Java). + * Discovered via Java SPI ({@code ServiceLoader}). */ @FunctionalInterface public interface HierarchyRuleProvider { @@ -64,48 +90,29 @@ public class HierarchyDefinitionService implements org.apache.skywalking.oap.ser } /** - * Convenience constructor that uses the default Java rule provider. + * Convenience constructor that discovers a {@link HierarchyRuleProvider} + * via Java SPI ({@code ServiceLoader}). Only loads the provider when + * hierarchy is enabled. */ public HierarchyDefinitionService(final CoreModuleConfig moduleConfig) { - this(moduleConfig, new DefaultJavaRuleProvider()); + this.hierarchyDefinition = new HashMap<>(); + this.layerLevels = new HashMap<>(); + if (moduleConfig.isEnableHierarchy()) { + this.init(loadProvider()); + this.checkLayers(); + } } - /** - * Default pure Java rule provider with 4 built-in hierarchy matching rules. - * No Groovy dependency. - */ - private static class DefaultJavaRuleProvider implements HierarchyRuleProvider { - @Override - public Map<String, BiFunction<Service, Service, Boolean>> buildRules( - final Map<String, String> ruleExpressions) { - final Map<String, BiFunction<Service, Service, Boolean>> registry = new HashMap<>(); - registry.put("name", (u, l) -> Objects.equals(u.getName(), l.getName())); - registry.put("short-name", (u, l) -> Objects.equals(u.getShortName(), l.getShortName())); - registry.put("lower-short-name-remove-ns", (u, l) -> { - final String sn = l.getShortName(); - final int dot = sn.lastIndexOf('.'); - return dot > 0 && Objects.equals(u.getShortName(), sn.substring(0, dot)); - }); - registry.put("lower-short-name-with-fqdn", (u, l) -> { - final String sn = u.getShortName(); - final int colon = sn.lastIndexOf(':'); - return colon > 0 && Objects.equals( - sn.substring(0, colon), - l.getShortName() + ".svc.cluster.local"); - }); - - final Map<String, BiFunction<Service, Service, Boolean>> rules = new HashMap<>(); - ruleExpressions.forEach((name, expression) -> { - final BiFunction<Service, Service, Boolean> fn = registry.get(name); - if (fn == null) { - throw new IllegalArgumentException( - "Unknown hierarchy matching rule: " + name - + ". Known rules: " + registry.keySet()); - } - rules.put(name, fn); - }); - return rules; + private static HierarchyRuleProvider loadProvider() { + final ServiceLoader<HierarchyRuleProvider> loader = + ServiceLoader.load(HierarchyRuleProvider.class); + for (final HierarchyRuleProvider provider : loader) { + log.info("Using hierarchy rule provider: {}", provider.getClass().getName()); + return provider; } + throw new IllegalStateException( + "No HierarchyRuleProvider found on classpath. " + + "Ensure the hierarchy analyzer module is included."); } @SuppressWarnings("unchecked") diff --git a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/hierarchy/HierarchyService.java b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/hierarchy/HierarchyService.java index 5eaa22752e..6f18f28995 100644 --- a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/hierarchy/HierarchyService.java +++ b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/hierarchy/HierarchyService.java @@ -38,6 +38,33 @@ import org.apache.skywalking.oap.server.core.source.SourceReceiver; import org.apache.skywalking.oap.server.library.module.ModuleManager; import org.apache.skywalking.oap.server.library.util.RunnableWithExceptionProtection; +/** + * Runtime service that builds hierarchy relations between services and instances. + * + * <p>Uses the compiled matching rules from + * {@link HierarchyDefinitionService} to determine if two services are + * hierarchically related (e.g., a MESH service sitting above a K8S_SERVICE). + * + * <p>Two paths for creating relations: + * <ol> + * <li><b>Explicit</b> (from agent telemetry): receivers call + * {@link #toServiceHierarchyRelation} or {@link #toInstanceHierarchyRelation} + * when agents report detected service-to-service relationships.</li> + * <li><b>Auto-matching</b> (scheduled background task): {@link #startAutoMatchingServiceHierarchy()} + * starts a background task that runs every 20 seconds, comparing all known + * service pairs against the compiled hierarchy rules: + * <ul> + * <li>Retrieves all services from {@link MetadataQueryService}.</li> + * <li>For each pair (i, j), checks if hierarchy rules exist for + * layer[i]→layer[j] or layer[j]→layer[i].</li> + * <li>Invokes {@link HierarchyDefinitionService.MatchingRule#match} + * which executes the compiled {@code BiFunction}.</li> + * <li>If matched, creates a {@link ServiceHierarchyRelation} and sends it + * to {@link SourceReceiver} for persistence.</li> + * </ul> + * </li> + * </ol> + */ @Slf4j public class HierarchyService implements org.apache.skywalking.oap.server.library.module.Service { private final ModuleManager moduleManager; diff --git a/test/script-compiler/hierarchy-v1-v2-checker/src/test/java/org/apache/skywalking/oap/server/core/config/HierarchyRuleComparisonTest.java b/test/script-compiler/hierarchy-v1-v2-checker/src/test/java/org/apache/skywalking/oap/server/core/config/HierarchyRuleComparisonTest.java index b5be35e468..c859bf1e6d 100644 --- a/test/script-compiler/hierarchy-v1-v2-checker/src/test/java/org/apache/skywalking/oap/server/core/config/HierarchyRuleComparisonTest.java +++ b/test/script-compiler/hierarchy-v1-v2-checker/src/test/java/org/apache/skywalking/oap/server/core/config/HierarchyRuleComparisonTest.java @@ -28,6 +28,7 @@ import org.apache.skywalking.oap.server.core.query.type.Service; import org.apache.skywalking.oap.server.library.util.ResourceUtils; import org.junit.jupiter.api.DynamicTest; import org.junit.jupiter.api.TestFactory; +import org.apache.skywalking.oap.server.core.config.compiler.CompiledHierarchyRuleProvider; import org.yaml.snakeyaml.Yaml; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -70,7 +71,7 @@ class HierarchyRuleComparisonTest { final Map<String, String> ruleExpressions = (Map<String, String>) config.get("auto-matching-rules"); final GroovyHierarchyRuleProvider groovyProvider = new GroovyHierarchyRuleProvider(); - final JavaHierarchyRuleProvider javaProvider = new JavaHierarchyRuleProvider(); + final CompiledHierarchyRuleProvider javaProvider = new CompiledHierarchyRuleProvider(); final Map<String, BiFunction<Service, Service, Boolean>> v1Rules = groovyProvider.buildRules(ruleExpressions); diff --git a/test/script-compiler/mal-lal-v1-v2-checker/src/test/java/org/apache/skywalking/oap/server/checker/lal/LalComparisonTest.java b/test/script-compiler/mal-lal-v1-v2-checker/src/test/java/org/apache/skywalking/oap/server/checker/lal/LalComparisonTest.java index c428c26d2b..2abd32cdf6 100644 --- a/test/script-compiler/mal-lal-v1-v2-checker/src/test/java/org/apache/skywalking/oap/server/checker/lal/LalComparisonTest.java +++ b/test/script-compiler/mal-lal-v1-v2-checker/src/test/java/org/apache/skywalking/oap/server/checker/lal/LalComparisonTest.java @@ -23,17 +23,13 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; +import org.apache.skywalking.oap.log.analyzer.compiler.LALClassGenerator; import org.apache.skywalking.oap.log.analyzer.dsl.LalExpression; -import org.apache.skywalking.oap.server.checker.InMemoryCompiler; -import org.apache.skywalking.oap.server.transpiler.lal.LalToJavaTranspiler; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.DynamicTest; import org.junit.jupiter.api.TestFactory; import org.yaml.snakeyaml.Yaml; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; /** @@ -41,32 +37,14 @@ import static org.junit.jupiter.api.Assertions.fail; * For each LAL rule across all LAL YAML files: * <ul> * <li>Path A (v1): Verify Groovy compiles the DSL without error</li> - * <li>Path B (v2): Transpile to Java, compile in-memory, verify it - * implements {@link LalExpression}</li> + * <li>Path B (v2): ANTLR4 + Javassist compilation via {@link LALClassGenerator}, + * verify it produces a valid {@link LalExpression}</li> * </ul> - * Both paths must accept the same DSL input. The transpiled Java class - * must compile and be instantiable. */ class LalComparisonTest { - private static InMemoryCompiler COMPILER; - private static int CLASS_COUNTER; - - @BeforeAll - static void initCompiler() throws Exception { - COMPILER = new InMemoryCompiler(); - CLASS_COUNTER = 0; - } - - @AfterAll - static void closeCompiler() throws Exception { - if (COMPILER != null) { - COMPILER.close(); - } - } - @TestFactory - Collection<DynamicTest> lalScriptsTranspileAndCompile() throws Exception { + Collection<DynamicTest> lalScriptsCompile() throws Exception { final List<DynamicTest> tests = new ArrayList<>(); final Map<String, List<LalRule>> yamlRules = loadAllLalYamlFiles(); @@ -75,7 +53,7 @@ class LalComparisonTest { for (final LalRule rule : entry.getValue()) { tests.add(DynamicTest.dynamicTest( yamlFile + " | " + rule.name, - () -> verifyTranspileAndCompile(rule.name, rule.dsl) + () -> verifyCompilation(rule.name, rule.dsl) )); } } @@ -83,8 +61,8 @@ class LalComparisonTest { return tests; } - private void verifyTranspileAndCompile(final String ruleName, - final String dsl) throws Exception { + private void verifyCompilation(final String ruleName, + final String dsl) throws Exception { // ---- V1: Verify Groovy can parse the DSL ---- try { final groovy.lang.GroovyShell sh = new groovy.lang.GroovyShell(); @@ -95,22 +73,11 @@ class LalComparisonTest { return; } - // ---- V2: Transpile and compile ---- + // ---- V2: ANTLR4 + Javassist compilation ---- try { - final LalToJavaTranspiler transpiler = new LalToJavaTranspiler(); - final String className = "LalExpr_check_" + (CLASS_COUNTER++); - final String javaSource = transpiler.transpile(className, dsl); - assertNotNull(javaSource, "V2 transpiler should produce source for '" + ruleName + "'"); - - final Class<?> clazz = COMPILER.compile( - LalToJavaTranspiler.GENERATED_PACKAGE, className, javaSource); - - assertTrue(LalExpression.class.isAssignableFrom(clazz), - "Generated class should implement LalExpression for '" + ruleName + "'"); - - final LalExpression expr = (LalExpression) clazz - .getDeclaredConstructor().newInstance(); - assertNotNull(expr, "V2 should instantiate for '" + ruleName + "'"); + final LALClassGenerator generator = new LALClassGenerator(); + final LalExpression expr = generator.compile(dsl); + assertNotNull(expr, "V2 should compile '" + ruleName + "'"); } catch (Exception e) { fail("V2 (Java) failed for LAL rule '" + ruleName + "': " + e.getMessage()); } @@ -161,15 +128,15 @@ class LalComparisonTest { } private Path findResourceDir(final String name) { - final Path starterResources = Path.of( - "oap-server/server-starter/src/main/resources/" + name); - if (Files.isDirectory(starterResources)) { - return starterResources; - } - final Path fromRoot = Path.of( - System.getProperty("user.dir")).resolve("../../server-starter/src/main/resources/" + name); - if (Files.isDirectory(fromRoot)) { - return fromRoot; + final String[] candidates = { + "oap-server/server-starter/src/main/resources/" + name, + "../../../oap-server/server-starter/src/main/resources/" + name + }; + for (final String candidate : candidates) { + final Path path = Path.of(candidate); + if (Files.isDirectory(path)) { + return path; + } } return null; } diff --git a/test/script-compiler/mal-lal-v1-v2-checker/src/test/java/org/apache/skywalking/oap/server/checker/mal/MalComparisonTest.java b/test/script-compiler/mal-lal-v1-v2-checker/src/test/java/org/apache/skywalking/oap/server/checker/mal/MalComparisonTest.java index 5a6a5ac93a..3b61172175 100644 --- a/test/script-compiler/mal-lal-v1-v2-checker/src/test/java/org/apache/skywalking/oap/server/checker/mal/MalComparisonTest.java +++ b/test/script-compiler/mal-lal-v1-v2-checker/src/test/java/org/apache/skywalking/oap/server/checker/mal/MalComparisonTest.java @@ -26,16 +26,13 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; -import com.google.common.collect.ImmutableMap; import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oap.meter.analyzer.compiler.MALClassGenerator; import org.apache.skywalking.oap.meter.analyzer.dsl.DSL; import org.apache.skywalking.oap.meter.analyzer.dsl.Expression; +import org.apache.skywalking.oap.meter.analyzer.dsl.ExpressionMetadata; import org.apache.skywalking.oap.meter.analyzer.dsl.ExpressionParsingContext; import org.apache.skywalking.oap.meter.analyzer.dsl.MalExpression; -import org.apache.skywalking.oap.server.checker.InMemoryCompiler; -import org.apache.skywalking.oap.server.transpiler.mal.MalToJavaTranspiler; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.DynamicTest; import org.junit.jupiter.api.TestFactory; import org.yaml.snakeyaml.Yaml; @@ -46,34 +43,16 @@ import static org.junit.jupiter.api.Assertions.assertEquals; * Dual-path comparison test for MAL (Meter Analysis Language) expressions. * For each metric rule across all MAL YAML files: * <ul> - * <li>Path A (v1): Groovy compilation via upstream {@link DSL#parse(String, String)}</li> - * <li>Path B (v2): Transpiled Java via {@link MalToJavaTranspiler}, compiled in-memory</li> + * <li>Path A (v1): Groovy compilation via upstream {@code DSL.parse()}</li> + * <li>Path B (v2): ANTLR4 + Javassist compilation via {@link MALClassGenerator}</li> * </ul> - * Both paths run {@code parse()} with empty input and compare the resulting - * {@link ExpressionParsingContext} (samples, scope, downsampling, aggregation labels). + * Both paths run metadata extraction and compare the resulting metadata + * (samples, scope, downsampling, aggregation labels). */ @Slf4j class MalComparisonTest { - private static InMemoryCompiler COMPILER; - private static final AtomicInteger CLASS_COUNTER = new AtomicInteger(); - private static final AtomicInteger V2_TRANSPILE_GAPS = new AtomicInteger(); - - @BeforeAll - static void initCompiler() throws Exception { - COMPILER = new InMemoryCompiler(); - } - - @AfterAll - static void closeCompiler() throws Exception { - if (COMPILER != null) { - COMPILER.close(); - } - final int gaps = V2_TRANSPILE_GAPS.get(); - if (gaps > 0) { - log.warn("{} MAL expressions could not be transpiled to Java (known transpiler gaps)", gaps); - } - } + private static final AtomicInteger V2_COMPILE_GAPS = new AtomicInteger(); @TestFactory Collection<DynamicTest> malExpressionsMatch() throws Exception { @@ -97,70 +76,49 @@ class MalComparisonTest { final String expression) throws Exception { // ---- V1: Groovy path ---- ExpressionParsingContext v1Ctx = null; - String v1Error = null; try { final Expression v1Expr = DSL.parse(metricName, expression); v1Ctx = v1Expr.parse(); } catch (Exception e) { - v1Error = e.getMessage(); + // V1 failed - skip comparison } - // ---- V2: Transpiled Java path ---- - ExpressionParsingContext v2Ctx = null; + // ---- V2: ANTLR4 + Javassist compilation ---- + ExpressionMetadata v2Meta = null; String v2Error = null; try { - final MalToJavaTranspiler transpiler = new MalToJavaTranspiler(); - final String className = "MalExpr_check_" + CLASS_COUNTER.getAndIncrement(); - final String javaSource = transpiler.transpileExpression(className, expression); - - final Class<?> clazz = COMPILER.compile( - MalToJavaTranspiler.GENERATED_PACKAGE, className, javaSource); - final MalExpression malExpr = (MalExpression) clazz - .getDeclaredConstructor().newInstance(); - - // Run parse: create parsing context, execute with empty map, extract context - try (ExpressionParsingContext ctx = ExpressionParsingContext.create()) { - try { - malExpr.run(ImmutableMap.of()); - } catch (Exception ignored) { - // Expected: expressions fail with empty input - } - ctx.validate(expression); - v2Ctx = ctx; - } + final MALClassGenerator generator = new MALClassGenerator(); + final MalExpression malExpr = generator.compile(metricName, expression); + v2Meta = malExpr.metadata(); } catch (Exception e) { v2Error = e.getMessage(); } // ---- Compare ---- - if (v1Ctx == null && v2Ctx == null) { - // Both failed - acceptable (known limitations in both paths) + if (v1Ctx == null && v2Meta == null) { return; } if (v1Ctx == null) { - // V1 failed but V2 succeeded - V2 is more capable, acceptable return; } - if (v2Ctx == null) { - // V2 transpiler/compilation gap - log and count, not a test failure. - // These are known limitations of the transpiler that will be addressed incrementally. - V2_TRANSPILE_GAPS.incrementAndGet(); - log.info("V2 transpile gap for '{}': {}", metricName, v2Error); + if (v2Meta == null) { + V2_COMPILE_GAPS.incrementAndGet(); + log.info("V2 compile gap for '{}': {}", metricName, v2Error); return; } - // Both succeeded - compare contexts - assertEquals(v1Ctx.getSamples(), v2Ctx.getSamples(), + // Both succeeded - compare metadata + assertEquals(v1Ctx.getSamples(), v2Meta.getSamples(), metricName + ": samples mismatch"); - assertEquals(v1Ctx.getScopeType(), v2Ctx.getScopeType(), + assertEquals(v1Ctx.getScopeType(), v2Meta.getScopeType(), metricName + ": scopeType mismatch"); - assertEquals(v1Ctx.getDownsampling(), v2Ctx.getDownsampling(), + assertEquals(v1Ctx.getDownsampling(), v2Meta.getDownsampling(), metricName + ": downsampling mismatch"); - assertEquals(v1Ctx.isHistogram(), v2Ctx.isHistogram(), + assertEquals(v1Ctx.isHistogram(), v2Meta.isHistogram(), metricName + ": isHistogram mismatch"); - assertEquals(v1Ctx.getScopeLabels(), v2Ctx.getScopeLabels(), + assertEquals(v1Ctx.getScopeLabels(), v2Meta.getScopeLabels(), metricName + ": scopeLabels mismatch"); - assertEquals(v1Ctx.getAggregationLabels(), v2Ctx.getAggregationLabels(), + assertEquals(v1Ctx.getAggregationLabels(), v2Meta.getAggregationLabels(), metricName + ": aggregationLabels mismatch"); } @@ -240,17 +198,15 @@ class MalComparisonTest { } private Path findResourceDir(final String name) { - // Look in server-starter resources - final Path starterResources = Path.of( - "oap-server/server-starter/src/main/resources/" + name); - if (Files.isDirectory(starterResources)) { - return starterResources; - } - // Try from project root - final Path fromRoot = Path.of( - System.getProperty("user.dir")).resolve("../../server-starter/src/main/resources/" + name); - if (Files.isDirectory(fromRoot)) { - return fromRoot; + final String[] candidates = { + "oap-server/server-starter/src/main/resources/" + name, + "../../../oap-server/server-starter/src/main/resources/" + name + }; + for (final String candidate : candidates) { + final Path path = Path.of(candidate); + if (Files.isDirectory(path)) { + return path; + } } return null; } diff --git a/test/script-compiler/mal-lal-v1-v2-checker/src/test/java/org/apache/skywalking/oap/server/checker/mal/MalFilterComparisonTest.java b/test/script-compiler/mal-lal-v1-v2-checker/src/test/java/org/apache/skywalking/oap/server/checker/mal/MalFilterComparisonTest.java index 2c3ee71667..4400701691 100644 --- a/test/script-compiler/mal-lal-v1-v2-checker/src/test/java/org/apache/skywalking/oap/server/checker/mal/MalFilterComparisonTest.java +++ b/test/script-compiler/mal-lal-v1-v2-checker/src/test/java/org/apache/skywalking/oap/server/checker/mal/MalFilterComparisonTest.java @@ -29,11 +29,8 @@ import java.util.Map; import java.util.Set; import groovy.lang.Closure; import groovy.lang.GroovyShell; +import org.apache.skywalking.oap.meter.analyzer.compiler.MALClassGenerator; import org.apache.skywalking.oap.meter.analyzer.dsl.MalFilter; -import org.apache.skywalking.oap.server.checker.InMemoryCompiler; -import org.apache.skywalking.oap.server.transpiler.mal.MalToJavaTranspiler; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.DynamicTest; import org.junit.jupiter.api.TestFactory; import org.yaml.snakeyaml.Yaml; @@ -46,28 +43,12 @@ import static org.junit.jupiter.api.Assertions.fail; * For each unique filter expression across all MAL YAML files: * <ul> * <li>Path A (v1): Groovy {@code GroovyShell.evaluate()} -> {@code Closure<Boolean>}</li> - * <li>Path B (v2): Transpile to {@link MalFilter}, compile in-memory</li> + * <li>Path B (v2): ANTLR4 + Javassist compilation via {@link MALClassGenerator}</li> * </ul> * Both paths are invoked with representative tag maps and results compared. */ class MalFilterComparisonTest { - private static InMemoryCompiler COMPILER; - private static int CLASS_COUNTER; - - @BeforeAll - static void initCompiler() throws Exception { - COMPILER = new InMemoryCompiler(); - CLASS_COUNTER = 0; - } - - @AfterAll - static void closeCompiler() throws Exception { - if (COMPILER != null) { - COMPILER.close(); - } - } - @TestFactory Collection<DynamicTest> filterExpressionsMatch() throws Exception { final Set<String> filters = collectAllFilterExpressions(); @@ -85,7 +66,6 @@ class MalFilterComparisonTest { @SuppressWarnings("unchecked") private void compareFilter(final String filterExpr) throws Exception { - // Extract the tag key from the filter expression for test data final List<Map<String, String>> testTags = buildTestTags(filterExpr); // ---- V1: Groovy closure ---- @@ -97,16 +77,11 @@ class MalFilterComparisonTest { return; } - // ---- V2: Transpiled MalFilter ---- + // ---- V2: ANTLR4 + Javassist compilation ---- final MalFilter v2Filter; try { - final MalToJavaTranspiler transpiler = new MalToJavaTranspiler(); - final String className = "MalFilter_check_" + (CLASS_COUNTER++); - final String javaSource = transpiler.transpileFilter(className, filterExpr); - - final Class<?> clazz = COMPILER.compile( - MalToJavaTranspiler.GENERATED_PACKAGE, className, javaSource); - v2Filter = (MalFilter) clazz.getDeclaredConstructor().newInstance(); + final MALClassGenerator generator = new MALClassGenerator(); + v2Filter = generator.compileFilter(filterExpr); } catch (Exception e) { fail("V2 (Java) failed for filter: " + filterExpr + " - " + e.getMessage()); return; @@ -118,14 +93,12 @@ class MalFilterComparisonTest { try { v1Result = v1Closure.call(tags); } catch (Exception e) { - // Some filters error on empty/missing tags in Groovy too continue; } boolean v2Result; try { v2Result = v2Filter.test(tags); } catch (NullPointerException e) { - // List.of().contains(null) throws NPE; Groovy 'in' returns false v2Result = false; } assertEquals(v1Result, v2Result, @@ -137,12 +110,8 @@ class MalFilterComparisonTest { private List<Map<String, String>> buildTestTags(final String filterExpr) { final List<Map<String, String>> testTags = new ArrayList<>(); - // Always test with an empty map testTags.add(new HashMap<>()); - // Extract key-value patterns from the expression to build matching and non-matching tags. - // Common patterns: tags.job_name == 'mysql-monitoring', tags.Namespace == 'AWS/DynamoDB' - // We build: one matching map, one non-matching map final java.util.regex.Pattern kvPattern = java.util.regex.Pattern.compile("tags\\.(\\w+)\\s*==\\s*'([^']+)'"); final java.util.regex.Matcher matcher = kvPattern.matcher(filterExpr); @@ -161,7 +130,6 @@ class MalFilterComparisonTest { testTags.add(mismatchTags); } - // Also test with a random unrelated key final Map<String, String> unrelatedTags = new HashMap<>(); unrelatedTags.put("unrelated_key", "some_value"); testTags.add(unrelatedTags); @@ -183,7 +151,6 @@ class MalFilterComparisonTest { collectFiltersFromDir(dirPath.toFile(), yaml, filters); } - // Also check log-mal-rules and envoy-metrics-rules for (final String dir : new String[]{"log-mal-rules", "envoy-metrics-rules"}) { final Path dirPath = findResourceDir(dir); if (dirPath != null) { @@ -225,15 +192,15 @@ class MalFilterComparisonTest { } private Path findResourceDir(final String name) { - final Path starterResources = Path.of( - "oap-server/server-starter/src/main/resources/" + name); - if (Files.isDirectory(starterResources)) { - return starterResources; - } - final Path fromRoot = Path.of( - System.getProperty("user.dir")).resolve("../../server-starter/src/main/resources/" + name); - if (Files.isDirectory(fromRoot)) { - return fromRoot; + final String[] candidates = { + "oap-server/server-starter/src/main/resources/" + name, + "../../../oap-server/server-starter/src/main/resources/" + name + }; + for (final String candidate : candidates) { + final Path path = Path.of(candidate); + if (Files.isDirectory(path)) { + return path; + } } return null; } diff --git a/test/script-compiler/mal-v1-with-groovy/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/MalExpression.java b/test/script-compiler/mal-v1-with-groovy/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/MalExpression.java index cfaf1d5bee..66737dfea7 100644 --- a/test/script-compiler/mal-v1-with-groovy/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/MalExpression.java +++ b/test/script-compiler/mal-v1-with-groovy/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/MalExpression.java @@ -21,10 +21,14 @@ package org.apache.skywalking.oap.meter.analyzer.dsl; import java.util.Map; /** - * Pure Java replacement for Groovy-based MAL DelegatingScript. - * Each transpiled MAL expression implements this interface. + * Each compiled MAL expression implements this interface. */ -@FunctionalInterface public interface MalExpression { SampleFamily run(Map<String, SampleFamily> samples); + + /** + * Returns compile-time metadata extracted from the expression AST: + * sample names, scope type, aggregation labels, downsampling, etc. + */ + ExpressionMetadata metadata(); }
