Copilot commented on code in PR #13699: URL: https://github.com/apache/skywalking/pull/13699#discussion_r2793870773
########## oap-server/oal-rt/src/test/java/org/apache/skywalking/oal/v2/parser/RealOALScriptsTest.java: ########## @@ -0,0 +1,237 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oal.v2.parser; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oal.v2.model.MetricDefinition; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Test V2 parser with real OAL scripts from server-starter/resources/oal. + * + * This validates that V2 can parse all production OAL scripts. + */ +@Slf4j +public class RealOALScriptsTest { + + /** + * Find the OAL scripts directory in the project. + * + * Tries multiple paths to locate the directory: + * 1. From current working directory (Maven default) + * 2. From user.dir system property + * 3. Relative from this module + */ + private File findOALScriptsDir() { + String[] possiblePaths = { + "oap-server/server-starter/src/main/resources/oal", + "../server-starter/src/main/resources/oal", + "../../server-starter/src/main/resources/oal" + }; + + for (String path : possiblePaths) { + File dir = new File(path); + if (dir.exists() && dir.isDirectory()) { + log.debug("Found OAL scripts directory at: {}", dir.getAbsolutePath()); + return dir; + } + } + + throw new IllegalStateException("Could not find OAL scripts directory. Tried: " + + String.join(", ", possiblePaths)); + } + + /** + * Test parsing core.oal with V2. + * + * core.oal contains the main service and endpoint metrics. + */ + @Test + public void testParseCoreOAL() throws IOException { + File oalDir = findOALScriptsDir(); + File oalFile = new File(oalDir, "core.oal"); + assertTrue(oalFile.exists(), "core.oal not found at: " + oalFile.getAbsolutePath()); + + OALScriptParserV2 parser = OALScriptParserV2.parse(new FileReader(oalFile), "core.oal"); + Review Comment: `FileReader` instances are created without being closed. Please wrap the reader in try-with-resources (or use `Files.newBufferedReader(..., UTF_8)`) to avoid leaking file handles during test runs. ########## oap-server/oal-rt/src/test/java/org/apache/skywalking/oal/v2/generator/RuntimeOALGenerationTest.java: ########## @@ -0,0 +1,342 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oal.v2.generator; + +import java.io.File; +import java.io.FileReader; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import javassist.ClassPool; +import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oal.v2.model.MetricDefinition; +import org.apache.skywalking.oal.v2.parser.OALScriptParserV2; +import org.apache.skywalking.oap.server.core.analysis.SourceDecoratorManager; +import org.apache.skywalking.oap.server.core.oal.rt.OALDefine; +import org.apache.skywalking.oap.server.core.source.DefaultScopeDefine; +import org.apache.skywalking.oap.server.core.storage.StorageBuilderFactory; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Test OAL V2 generation with all runtime OAL scripts. + * + * <p>This test loads all OAL scripts defined via OALDefine implementations and generates + * classes exactly as the runtime does. Generated classes are written to target/generated-test-sources + * for IDE inspection. + * + * <p>Key behaviors: + * <ul> + * <li>Scans all OALDefine implementations to map OAL scripts</li> + * <li>Verifies OAL scripts exist and can be parsed</li> + * <li>Detects dispatcher conflicts when same source appears in multiple OAL files</li> + * <li>Generates classes with correct package structure matching runtime</li> + * <li>Allows developers to inspect generated sources via IDE decompiler</li> + * </ul> + */ +@Slf4j +public class RuntimeOALGenerationTest { + + private static final String SOURCE_PACKAGE = "org.apache.skywalking.oap.server.core.source."; + private static final String BROWSER_SOURCE_PACKAGE = "org.apache.skywalking.oap.server.core.browser.source."; + private static final String METRICS_PACKAGE = "org.apache.skywalking.oap.server.core.source.oal.rt.metrics."; + + private static final String[] POSSIBLE_PATHS = { + "oap-server/server-starter/src/main/resources/", + "../server-starter/src/main/resources/", + "../../server-starter/src/main/resources/" + }; + + /** + * All known OALDefine implementations mapped to their OAL script paths. + */ + private static final Map<String, OALDefine> OAL_DEFINES = new HashMap<>(); + + @BeforeAll + public static void setup() { + // Initialize all scopes + initializeScopes(); + + // Register all known OALDefine implementations (matching runtime OALDefine classes) + // CoreOALDefine - no catalog + registerOALDefine("core", createOALDefine("oal/core.oal", SOURCE_PACKAGE, "")); + // JVMOALDefine - no catalog + registerOALDefine("java-agent", createOALDefine("oal/java-agent.oal", SOURCE_PACKAGE, "")); + // CLROALDefine - no catalog + registerOALDefine("dotnet-agent", createOALDefine("oal/dotnet-agent.oal", SOURCE_PACKAGE, "")); + // BrowserOALDefine - different source package, no catalog + registerOALDefine("browser", createOALDefine("oal/browser.oal", BROWSER_SOURCE_PACKAGE, "")); + // MeshOALDefine - catalog: "ServiceMesh" + registerOALDefine("mesh", createOALDefine("oal/mesh.oal", SOURCE_PACKAGE, "ServiceMesh")); + // TCPOALDefine - catalog: "EnvoyTCP" + registerOALDefine("tcp", createOALDefine("oal/tcp.oal", SOURCE_PACKAGE, "EnvoyTCP")); + // EBPFOALDefine - no catalog + registerOALDefine("ebpf", createOALDefine("oal/ebpf.oal", SOURCE_PACKAGE, "")); + // CiliumOALDefine - no catalog + registerOALDefine("cilium", createOALDefine("oal/cilium.oal", SOURCE_PACKAGE, "")); + // DisableOALDefine - no catalog + registerOALDefine("disable", createOALDefine("oal/disable.oal", SOURCE_PACKAGE, "")); + + // Set generated file path for IDE inspection + OALClassGeneratorV2.setGeneratedFilePath("target/test-classes"); + } + + private static void registerOALDefine(String name, OALDefine define) { + OAL_DEFINES.put(name, define); + } + + private static OALDefine createOALDefine(String configFile, String sourcePackage, String catalog) { + return new OALDefine(configFile, sourcePackage, catalog) { + }; + } + + private static void initializeScopes() { + DefaultScopeDefine.Listener listener = new DefaultScopeDefine.Listener(); + + // Core sources + notifyClass(listener, SOURCE_PACKAGE, "Service"); + notifyClass(listener, SOURCE_PACKAGE, "ServiceInstance"); + notifyClass(listener, SOURCE_PACKAGE, "Endpoint"); + notifyClass(listener, SOURCE_PACKAGE, "ServiceRelation"); + notifyClass(listener, SOURCE_PACKAGE, "ServiceInstanceRelation"); + notifyClass(listener, SOURCE_PACKAGE, "EndpointRelation"); + notifyClass(listener, SOURCE_PACKAGE, "DatabaseAccess"); + notifyClass(listener, SOURCE_PACKAGE, "CacheAccess"); + notifyClass(listener, SOURCE_PACKAGE, "MQAccess"); + notifyClass(listener, SOURCE_PACKAGE, "MQEndpointAccess"); + + // JVM sources + notifyClass(listener, SOURCE_PACKAGE, "ServiceInstanceJVMCPU"); + notifyClass(listener, SOURCE_PACKAGE, "ServiceInstanceJVMMemory"); + notifyClass(listener, SOURCE_PACKAGE, "ServiceInstanceJVMMemoryPool"); + notifyClass(listener, SOURCE_PACKAGE, "ServiceInstanceJVMGC"); + notifyClass(listener, SOURCE_PACKAGE, "ServiceInstanceJVMThread"); + notifyClass(listener, SOURCE_PACKAGE, "ServiceInstanceJVMClass"); + + // CLR sources + notifyClass(listener, SOURCE_PACKAGE, "ServiceInstanceCLRCPU"); + notifyClass(listener, SOURCE_PACKAGE, "ServiceInstanceCLRGC"); + notifyClass(listener, SOURCE_PACKAGE, "ServiceInstanceCLRThread"); + + // Browser sources + notifyClass(listener, BROWSER_SOURCE_PACKAGE, "BrowserAppTraffic"); + notifyClass(listener, BROWSER_SOURCE_PACKAGE, "BrowserAppPageTraffic"); + notifyClass(listener, BROWSER_SOURCE_PACKAGE, "BrowserAppSingleVersionTraffic"); + notifyClass(listener, BROWSER_SOURCE_PACKAGE, "BrowserAppPerf"); + notifyClass(listener, BROWSER_SOURCE_PACKAGE, "BrowserAppPagePerf"); + notifyClass(listener, BROWSER_SOURCE_PACKAGE, "BrowserAppSingleVersionPerf"); + notifyClass(listener, BROWSER_SOURCE_PACKAGE, "BrowserAppResourcePerf"); + notifyClass(listener, BROWSER_SOURCE_PACKAGE, "BrowserAppWebInteractionPerf"); + notifyClass(listener, BROWSER_SOURCE_PACKAGE, "BrowserAppWebVitalsPerf"); + notifyClass(listener, BROWSER_SOURCE_PACKAGE, "BrowserErrorLog"); + + // Mesh sources + notifyClass(listener, SOURCE_PACKAGE, "ServiceMesh"); + notifyClass(listener, SOURCE_PACKAGE, "ServiceMeshService"); + notifyClass(listener, SOURCE_PACKAGE, "ServiceMeshServiceInstance"); + notifyClass(listener, SOURCE_PACKAGE, "ServiceMeshServiceRelation"); + notifyClass(listener, SOURCE_PACKAGE, "ServiceMeshServiceInstanceRelation"); + + // TCP sources + notifyClass(listener, SOURCE_PACKAGE, "TCPService"); + notifyClass(listener, SOURCE_PACKAGE, "TCPServiceInstance"); + notifyClass(listener, SOURCE_PACKAGE, "TCPServiceRelation"); + notifyClass(listener, SOURCE_PACKAGE, "TCPServiceInstanceRelation"); + + // eBPF sources + notifyClass(listener, SOURCE_PACKAGE, "EBPFProfilingSchedule"); + + // Cilium sources + notifyClass(listener, SOURCE_PACKAGE, "CiliumService"); + notifyClass(listener, SOURCE_PACKAGE, "CiliumServiceInstance"); + notifyClass(listener, SOURCE_PACKAGE, "CiliumEndpoint"); + notifyClass(listener, SOURCE_PACKAGE, "CiliumServiceRelation"); + notifyClass(listener, SOURCE_PACKAGE, "CiliumServiceInstanceRelation"); + + // K8s sources + notifyClass(listener, SOURCE_PACKAGE, "K8SService"); + notifyClass(listener, SOURCE_PACKAGE, "K8SServiceInstance"); + notifyClass(listener, SOURCE_PACKAGE, "K8SEndpoint"); + notifyClass(listener, SOURCE_PACKAGE, "K8SServiceRelation"); + notifyClass(listener, SOURCE_PACKAGE, "K8SServiceInstanceRelation"); + + // Process sources + notifyClass(listener, SOURCE_PACKAGE, "Process"); + notifyClass(listener, SOURCE_PACKAGE, "ProcessRelation"); + + // Register decorators + registerDecorator(SOURCE_PACKAGE, "ServiceDecorator"); + registerDecorator(SOURCE_PACKAGE, "EndpointDecorator"); + registerDecorator(SOURCE_PACKAGE, "K8SServiceDecorator"); + registerDecorator(SOURCE_PACKAGE, "K8SEndpointDecorator"); + } + + private static void notifyClass(DefaultScopeDefine.Listener listener, String packageName, String className) { + try { + Class<?> clazz = Class.forName(packageName + className); + listener.notify(clazz); + } catch (Exception e) { + log.debug("Scope {} registration: {}", className, e.getMessage()); + } + } + + private static void registerDecorator(String packageName, String decoratorName) { + try { + Class<?> clazz = Class.forName(packageName + decoratorName); + SourceDecoratorManager manager = new SourceDecoratorManager(); + manager.addIfAsSourceDecorator(clazz); + } catch (Exception e) { + log.debug("Decorator {} registration: {}", decoratorName, e.getMessage()); + } + } + + /** + * Test that all OAL scripts can be loaded, parsed, and classes generated. + * + * <p>This test validates: + * <ul> + * <li>All OAL script files exist</li> + * <li>All scripts parse successfully</li> + * <li>Classes can be generated without conflicts</li> + * <li>Generated classes match runtime structure</li> + * </ul> + */ + @Test + public void testGenerateAllRuntimeOALScripts() throws Exception { + // Single ClassPool for all generated classes + // No conflicts because catalog prefix creates unique dispatcher class names + // (e.g., ServiceDispatcher vs ServiceMeshServiceDispatcher) + ClassPool classPool = new ClassPool(true); + + int totalMetrics = 0; + int totalDispatchers = 0; + int totalGeneratedClasses = 0; + List<String> errors = new ArrayList<>(); + + for (Map.Entry<String, OALDefine> entry : OAL_DEFINES.entrySet()) { + String oalName = entry.getKey(); + OALDefine define = entry.getValue(); + + File oalFile = findOALFile(define.getConfigFile()); + assertNotNull(oalFile, "OAL file not found: " + define.getConfigFile() + + ". Tried paths: " + String.join(", ", POSSIBLE_PATHS)); + + try { + // Parse OAL script + OALScriptParserV2 parser = OALScriptParserV2.parse(new FileReader(oalFile), define.getConfigFile()); + List<MetricDefinition> metrics = parser.getMetrics(); Review Comment: `new FileReader(oalFile)` is not closed here. Use try-with-resources (and ideally a UTF-8 `BufferedReader`) so repeated test runs don’t leak file descriptors. ########## oap-server/oal-rt/src/main/java/org/apache/skywalking/oal/v2/registry/MetricsFunctionRegistry.java: ########## @@ -0,0 +1,91 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oal.v2.registry; + +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; + +/** + * Registry for metrics aggregation functions. + * + * This interface abstracts the discovery and lookup of metrics functions + * annotated with @MetricsFunction. Implementations can use different strategies: + * - Classpath scanning (default) + * - Manual registration + * - Configuration-based registration + * + * Example usage: + * <pre> + * MetricsFunctionRegistry registry = MetricsFunctionRegistryFactory.createDefault(); + * + * Optional<MetricsFunctionDescriptor> longAvg = registry.findFunction("longAvg"); + * if (longAvg.isPresent()) { + * Class<? extends Metrics> metricsClass = longAvg.get().getMetricsClass(); + * Method entranceMethod = longAvg.get().getEntranceMethod(); + * // ... use for code generation + * } + * </pre> Review Comment: Javadoc example references `MetricsFunctionRegistryFactory`, but that type doesn’t exist in this module. Consider updating the example to use `DefaultMetricsFunctionRegistry.create()` (or remove the factory reference) to avoid misleading API documentation. ########## oap-server/oal-rt/src/main/java/org/apache/skywalking/oal/v2/OALEngineV2.java: ########## @@ -0,0 +1,184 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oal.v2; + +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.Reader; +import java.util.ArrayList; +import java.util.List; +import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oal.v2.generator.CodeGenModel; +import org.apache.skywalking.oal.v2.generator.MetricDefinitionEnricher; +import org.apache.skywalking.oal.v2.generator.OALClassGeneratorV2; +import org.apache.skywalking.oal.v2.model.MetricDefinition; +import org.apache.skywalking.oal.v2.parser.OALScriptParserV2; +import org.apache.skywalking.oap.server.core.analysis.DispatcherDetectorListener; +import org.apache.skywalking.oap.server.core.analysis.StreamAnnotationListener; +import org.apache.skywalking.oap.server.core.oal.rt.OALCompileException; +import org.apache.skywalking.oap.server.core.oal.rt.OALDefine; +import org.apache.skywalking.oap.server.core.oal.rt.OALEngine; +import org.apache.skywalking.oap.server.core.storage.StorageBuilderFactory; +import org.apache.skywalking.oap.server.core.storage.StorageException; +import org.apache.skywalking.oap.server.library.module.ModuleStartException; +import org.apache.skywalking.oap.server.library.util.ResourceUtils; + +/** + * V2 OAL Engine - completely independent implementation. + * + * This engine: + * 1. Parses OAL scripts using V2 parser (immutable models) + * 2. Enriches V2 models with metadata (MetricDefinitionEnricher) + * 3. Generates classes using V2 templates (OALClassGeneratorV2) + * + * Benefits: + * - Clean immutable data models + * - Type-safe filter values and function arguments + * - Better error messages with source location tracking + * - Completely independent (no V1 code dependencies) + */ +@Slf4j +public class OALEngineV2 implements OALEngine { + + private final OALClassGeneratorV2 classGeneratorV2; + private final OALDefine oalDefine; + + private StreamAnnotationListener streamAnnotationListener; + private DispatcherDetectorListener dispatcherDetectorListener; + private final List<Class> metricsClasses; + private final List<Class> dispatcherClasses; + + public OALEngineV2(OALDefine define) { + this.oalDefine = define; + this.classGeneratorV2 = new OALClassGeneratorV2(define); + this.metricsClasses = new ArrayList<>(); + this.dispatcherClasses = new ArrayList<>(); + } + + @Override + public void setStreamListener(StreamAnnotationListener listener) { + this.streamAnnotationListener = listener; + } + + @Override + public void setDispatcherListener(DispatcherDetectorListener listener) { + this.dispatcherDetectorListener = listener; + } + + @Override + public void setStorageBuilderFactory(StorageBuilderFactory factory) { + classGeneratorV2.setStorageBuilderFactory(factory); + } + + @Override + public void start(ClassLoader currentClassLoader) throws ModuleStartException, OALCompileException { + log.info("Starting OAL Engine V2..."); + + // Prepare temp folder for generated classes + classGeneratorV2.prepareRTTempFolder(); + classGeneratorV2.setCurrentClassLoader(currentClassLoader); + + // Load OAL script + Reader reader; + try { + reader = ResourceUtils.read(oalDefine.getConfigFile()); + } catch (FileNotFoundException e) { + throw new ModuleStartException("Can't locate " + oalDefine.getConfigFile(), e); + } + + // Parse using V2 parser + OALScriptParserV2 v2Parser; + try { + v2Parser = OALScriptParserV2.parse(reader, oalDefine.getConfigFile()); + log.info("V2 Parser: Successfully parsed {} metrics", v2Parser.getMetricsCount()); + } catch (IOException e) { + throw new ModuleStartException("OAL V2 script parse failure", e); + } + + // Enrich V2 models with metadata for code generation + List<CodeGenModel> codeGenModels = enrichMetrics(v2Parser.getMetrics()); + log.info("V2 Enricher: Enriched {} metrics with metadata", codeGenModels.size()); + + // Generate classes using V2 generator + classGeneratorV2.generateClassAtRuntime( + codeGenModels, + v2Parser.getDisabledSources(), + metricsClasses, + dispatcherClasses + ); + + log.info("OAL Engine V2 started successfully. Generated {} metrics classes, {} dispatcher classes", + metricsClasses.size(), + dispatcherClasses.size() + ); Review Comment: The `Reader` returned by `ResourceUtils.read(...)` is never closed. Please use try-with-resources around the reader (and ensure it closes on parse failure) to avoid resource leaks in long-running OAP processes. ```suggestion // Load OAL script, parse, and generate classes with proper resource management try (Reader reader = ResourceUtils.read(oalDefine.getConfigFile())) { // Parse using V2 parser OALScriptParserV2 v2Parser; try { v2Parser = OALScriptParserV2.parse(reader, oalDefine.getConfigFile()); log.info("V2 Parser: Successfully parsed {} metrics", v2Parser.getMetricsCount()); } catch (IOException e) { throw new ModuleStartException("OAL V2 script parse failure", e); } // Enrich V2 models with metadata for code generation List<CodeGenModel> codeGenModels = enrichMetrics(v2Parser.getMetrics()); log.info("V2 Enricher: Enriched {} metrics with metadata", codeGenModels.size()); // Generate classes using V2 generator classGeneratorV2.generateClassAtRuntime( codeGenModels, v2Parser.getDisabledSources(), metricsClasses, dispatcherClasses ); log.info("OAL Engine V2 started successfully. Generated {} metrics classes, {} dispatcher classes", metricsClasses.size(), dispatcherClasses.size() ); } catch (FileNotFoundException e) { throw new ModuleStartException("Can't locate " + oalDefine.getConfigFile(), e); } catch (IOException e) { throw new ModuleStartException("OAL V2 script I/O failure", e); } ``` ########## oap-server/oal-rt/src/test/java/org/apache/skywalking/oal/v2/parser/RealOALScriptsTest.java: ########## @@ -0,0 +1,237 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oal.v2.parser; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oal.v2.model.MetricDefinition; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Test V2 parser with real OAL scripts from server-starter/resources/oal. + * + * This validates that V2 can parse all production OAL scripts. + */ +@Slf4j +public class RealOALScriptsTest { + + /** + * Find the OAL scripts directory in the project. + * + * Tries multiple paths to locate the directory: + * 1. From current working directory (Maven default) + * 2. From user.dir system property + * 3. Relative from this module + */ + private File findOALScriptsDir() { + String[] possiblePaths = { + "oap-server/server-starter/src/main/resources/oal", + "../server-starter/src/main/resources/oal", + "../../server-starter/src/main/resources/oal" + }; + + for (String path : possiblePaths) { + File dir = new File(path); + if (dir.exists() && dir.isDirectory()) { + log.debug("Found OAL scripts directory at: {}", dir.getAbsolutePath()); + return dir; + } + } + + throw new IllegalStateException("Could not find OAL scripts directory. Tried: " + + String.join(", ", possiblePaths)); + } + + /** + * Test parsing core.oal with V2. + * + * core.oal contains the main service and endpoint metrics. + */ + @Test + public void testParseCoreOAL() throws IOException { + File oalDir = findOALScriptsDir(); + File oalFile = new File(oalDir, "core.oal"); + assertTrue(oalFile.exists(), "core.oal not found at: " + oalFile.getAbsolutePath()); + + OALScriptParserV2 parser = OALScriptParserV2.parse(new FileReader(oalFile), "core.oal"); + + assertNotNull(parser); + assertTrue(parser.hasMetrics(), "core.oal should have metrics"); + + log.info("✅ Parsed core.oal: {} metrics", parser.getMetricsCount()); + + // Verify some expected metrics exist + boolean foundServiceRespTime = false; + boolean foundServiceSla = false; + boolean foundServiceCpm = false; + + for (MetricDefinition metric : parser.getMetrics()) { + String name = metric.getName(); + if (name.equals("service_resp_time")) { + foundServiceRespTime = true; + log.info(" - Found: service_resp_time (source: {}, function: {})", + metric.getSource().getName(), + metric.getAggregationFunction().getName()); + } else if (name.equals("service_sla")) { + foundServiceSla = true; + } else if (name.equals("service_cpm")) { + foundServiceCpm = true; + } + } + + assertTrue(foundServiceRespTime, "Should find service_resp_time"); + assertTrue(foundServiceSla, "Should find service_sla"); + assertTrue(foundServiceCpm, "Should find service_cpm"); + } + + /** + * Test parsing all OAL files from server-starter/resources/oal. + */ + @Test + public void testParseAllOALFiles() throws IOException { + File oalDir = findOALScriptsDir(); + String[] oalFiles = { + "core.oal", + "java-agent.oal", + "dotnet-agent.oal", + "browser.oal", + "mesh.oal", + "ebpf.oal", + "tcp.oal", + "cilium.oal", + "disable.oal" + }; + + int totalMetrics = 0; + int successCount = 0; + + for (String fileName : oalFiles) { + File oalFile = new File(oalDir, fileName); + assertTrue(oalFile.exists(), fileName + " not found at: " + oalFile.getAbsolutePath()); + + try { + OALScriptParserV2 parser = OALScriptParserV2.parse(new FileReader(oalFile), fileName); Review Comment: This FileReader is not always closed on method exit. ########## oap-server/oal-rt/src/main/java/org/apache/skywalking/oal/v2/generator/CodeGenModel.java: ########## @@ -0,0 +1,401 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oal.v2.generator; + +import java.util.ArrayList; +import java.util.List; +import lombok.Builder; +import lombok.Getter; +import org.apache.skywalking.oal.v2.model.MetricDefinition; + +/** + * Code generation model for OAL metrics. + * + * <p>This model contains all information needed by FreeMarker templates to generate + * metrics classes, builders, and dispatchers at runtime. + * + * <p>Built from {@link MetricDefinition} via {@link MetricDefinitionEnricher}, this model + * provides template-ready data structures with precomputed method names, type information, + * and serialization metadata. + * + * <p>Example OAL input: + * <pre> + * service_resp_time = from(Service.latency).longAvg(); + * </pre> + * + * <p>Produces a CodeGenModel with: + * <ul> + * <li>varName = "service_resp_time"</li> + * <li>metricsName = "ServiceRespTime"</li> + * <li>tableName = "service_resp_time"</li> + * <li>sourceName = "Service"</li> + * <li>functionName = "longAvg"</li> + * <li>metricsClassName = "LongAvgMetrics"</li> + * </ul> + * + * @see MetricDefinitionEnricher + * @see OALClassGeneratorV2 + */ +@Getter +@Builder Review Comment: Default toString(): EntranceMethodV2 inherits toString() from Object, and so is not suitable for printing. Default toString(): SerializeFieldsV2 inherits toString() from Object, and so is not suitable for printing. Default toString(): FiltersV2 inherits toString() from Object, and so is not suitable for printing. Default toString(): FromStmtV2 inherits toString() from Object, and so is not suitable for printing. ########## oap-server/oal-rt/src/test/java/org/apache/skywalking/oal/v2/parser/RealOALScriptsTest.java: ########## @@ -0,0 +1,237 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oal.v2.parser; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oal.v2.model.MetricDefinition; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Test V2 parser with real OAL scripts from server-starter/resources/oal. + * + * This validates that V2 can parse all production OAL scripts. + */ +@Slf4j +public class RealOALScriptsTest { + + /** + * Find the OAL scripts directory in the project. + * + * Tries multiple paths to locate the directory: + * 1. From current working directory (Maven default) + * 2. From user.dir system property + * 3. Relative from this module + */ + private File findOALScriptsDir() { + String[] possiblePaths = { + "oap-server/server-starter/src/main/resources/oal", + "../server-starter/src/main/resources/oal", + "../../server-starter/src/main/resources/oal" + }; + + for (String path : possiblePaths) { + File dir = new File(path); + if (dir.exists() && dir.isDirectory()) { + log.debug("Found OAL scripts directory at: {}", dir.getAbsolutePath()); + return dir; + } + } + + throw new IllegalStateException("Could not find OAL scripts directory. Tried: " + + String.join(", ", possiblePaths)); + } + + /** + * Test parsing core.oal with V2. + * + * core.oal contains the main service and endpoint metrics. + */ + @Test + public void testParseCoreOAL() throws IOException { + File oalDir = findOALScriptsDir(); + File oalFile = new File(oalDir, "core.oal"); + assertTrue(oalFile.exists(), "core.oal not found at: " + oalFile.getAbsolutePath()); + + OALScriptParserV2 parser = OALScriptParserV2.parse(new FileReader(oalFile), "core.oal"); + + assertNotNull(parser); + assertTrue(parser.hasMetrics(), "core.oal should have metrics"); + + log.info("✅ Parsed core.oal: {} metrics", parser.getMetricsCount()); + + // Verify some expected metrics exist + boolean foundServiceRespTime = false; + boolean foundServiceSla = false; + boolean foundServiceCpm = false; + + for (MetricDefinition metric : parser.getMetrics()) { + String name = metric.getName(); + if (name.equals("service_resp_time")) { + foundServiceRespTime = true; + log.info(" - Found: service_resp_time (source: {}, function: {})", + metric.getSource().getName(), + metric.getAggregationFunction().getName()); + } else if (name.equals("service_sla")) { + foundServiceSla = true; + } else if (name.equals("service_cpm")) { + foundServiceCpm = true; + } + } + + assertTrue(foundServiceRespTime, "Should find service_resp_time"); + assertTrue(foundServiceSla, "Should find service_sla"); + assertTrue(foundServiceCpm, "Should find service_cpm"); + } + + /** + * Test parsing all OAL files from server-starter/resources/oal. + */ + @Test + public void testParseAllOALFiles() throws IOException { + File oalDir = findOALScriptsDir(); + String[] oalFiles = { + "core.oal", + "java-agent.oal", + "dotnet-agent.oal", + "browser.oal", + "mesh.oal", + "ebpf.oal", + "tcp.oal", + "cilium.oal", + "disable.oal" + }; + + int totalMetrics = 0; + int successCount = 0; + + for (String fileName : oalFiles) { + File oalFile = new File(oalDir, fileName); + assertTrue(oalFile.exists(), fileName + " not found at: " + oalFile.getAbsolutePath()); + + try { + OALScriptParserV2 parser = OALScriptParserV2.parse(new FileReader(oalFile), fileName); + int metricsCount = parser.getMetricsCount(); + totalMetrics += metricsCount; + successCount++; + + log.info("✅ Parsed {}: {} metrics, {} disabled sources", + fileName, + metricsCount, + parser.getDisabledSources().size()); + + } catch (Exception e) { + log.error("❌ Failed to parse {}: {}", fileName, e.getMessage(), e); + throw e; + } + } + + log.info("✅ Successfully parsed {}/{} OAL files, total {} metrics", + successCount, oalFiles.length, totalMetrics); + + assertTrue(successCount >= 5, "Should successfully parse at least 5 OAL files"); + assertTrue(totalMetrics > 50, "Should have at least 50 metrics total"); + } + + /** + * Test that V2 parser handles comments correctly. + */ + @Test + public void testCommentsInRealOAL() throws IOException { + File oalDir = findOALScriptsDir(); + File oalFile = new File(oalDir, "core.oal"); + if (!oalFile.exists()) { + log.warn("Skipping test - core.oal not found"); + return; + } + + // core.oal has line comments like: // Multiple values including p50, p75, p90, p95, p99 + OALScriptParserV2 parser = OALScriptParserV2.parse(new FileReader(oalFile), "core.oal"); + + assertNotNull(parser); + assertTrue(parser.hasMetrics()); + + log.info("✅ V2 parser correctly handles comments in OAL scripts"); + } + + /** + * Test parsing OAL with decorators (real feature in core.oal). + */ + @Test + public void testDecoratorsInRealOAL() throws IOException { + File oalDir = findOALScriptsDir(); + File oalFile = new File(oalDir, "core.oal"); + if (!oalFile.exists()) { + log.warn("Skipping test - core.oal not found"); + return; + } + + OALScriptParserV2 parser = OALScriptParserV2.parse(new FileReader(oalFile), "core.oal"); + + // core.oal has: service_resp_time = from(Service.latency).longAvg().decorator("ServiceDecorator"); + boolean foundDecorator = false; + for (MetricDefinition metric : parser.getMetrics()) { + if (metric.getDecorator().isPresent()) { + foundDecorator = true; + log.info(" - Found decorator: {} in metric: {}", + metric.getDecorator().get(), + metric.getName()); + break; + } + } + + assertTrue(foundDecorator, "Should find at least one metric with decorator"); + log.info("✅ V2 parser correctly handles decorators"); + } + + /** + * Test parsing OAL with complex filters (DetectPoint, RequestType enums). + */ + @Test + public void testComplexFiltersInRealOAL() throws IOException { + File oalDir = findOALScriptsDir(); + File oalFile = new File(oalDir, "core.oal"); + if (!oalFile.exists()) { + log.warn("Skipping test - core.oal not found"); + return; + } + + OALScriptParserV2 parser = OALScriptParserV2.parse(new FileReader(oalFile), "core.oal"); Review Comment: This FileReader is not always closed on method exit. ########## oap-server/oal-rt/V2_IMPLEMENTATION_SUMMARY.md: ########## @@ -0,0 +1,386 @@ +# OAL Engine V2 - Implementation Summary + +## Overview + +OAL Engine V2 is now the **only OAL implementation** after V1 has been completely removed. V2 provides a clean architecture with immutable models, type safety, and comprehensive testing. + +## Architecture + +``` +OAL Script → OALScriptParserV2 → MetricDefinition (immutable) → +MetricDefinitionEnricher → CodeGenModel → OALClassGeneratorV2 → +FreeMarker Templates → Javassist → Generated Classes +``` + +### Key Design Principles + +1. **Immutable Models**: All data models are immutable and thread-safe +2. **Type Safety**: Strongly-typed filter values and function arguments +3. **Clean Separation**: Parse → Enrich → Generate pipeline +4. **Independent Package Structure**: All V2 code organized under `org.apache.skywalking.oal.v2` +5. **Comprehensive Testing**: 70+ unit tests covering all components + +## What Was Implemented + +### 1. V2 Immutable Model Classes ✅ +**Location**: `org.apache.skywalking.oal.v2.model` + +All model classes are immutable, type-safe, and use builder pattern: +- **SourceLocation**: Track source file location for error reporting +- **SourceReference**: Immutable source reference (e.g., `Service.latency`) +- **FilterOperator**: Type-safe enum for filter operators (==, >, <, like, etc.) +- **FilterValue**: Strongly-typed filter values (NUMBER, STRING, BOOLEAN, NULL, ARRAY) +- **FunctionArgument**: Type-safe function arguments (LITERAL, ATTRIBUTE, EXPRESSION) +- **FilterExpression**: Immutable filter expression +- **FunctionCall**: Function call with typed arguments +- **MetricDefinition**: Complete metric definition + +### 2. V2 Parser ✅ +**Location**: `org.apache.skywalking.oal.v2.parser` + +- **OALListenerV2**: ANTLR listener converting parse tree to V2 models + - Handles all OAL grammar rules + - Builds immutable objects + - Preserves source locations + - Comprehensive javadoc with parsing flow examples + +- **OALScriptParserV2**: Facade for parsing OAL scripts + - Simple API: `OALScriptParserV2.parse(script)` + - Returns list of `MetricDefinition` objects + - Helper methods: `hasMetrics()`, `getMetricsCount()` + +### 3. V2 Registry ✅ +**Location**: `org.apache.skywalking.oal.v2.registry` + +- **MetricsFunctionRegistry**: Interface for function registry +- **MetricsFunctionDescriptor**: Function metadata +- **DefaultMetricsFunctionRegistry**: Classpath scanning implementation + - Scans for `@MetricsFunction` annotations + - Finds `@Entrance` methods + - Lazy initialization with thread-safety + +### 4. V2 Code Generation Pipeline ✅ +**Location**: `org.apache.skywalking.oal.v2.generator` + +- **CodeGenModel**: V2 data model for templates + - Contains all information needed by Freemarker templates + - Separate classes for source fields, persistent fields, entrance methods + - Serialization field models + +- **MetricDefinitionEnricher**: V2 enricher (replaces V1's DeepAnalysis) + - Looks up metrics function classes + - Extracts source columns from metadata + - Finds entrance methods via reflection + - Builds entrance method arguments + - Collects persistent fields from @Column annotations + - Generates serialization fields + +- **OALClassGeneratorV2**: V2 class generator + - Uses **V2 templates** from `code-templates-v2/` + - Generates metrics classes with Javassist + - Generates builder classes + - Generates dispatcher classes + - Completely independent from V1 generator + +### 5. V2 Engine ✅ +**Location**: `org.apache.skywalking.oal.v2` + +- **OALEngineV2**: Main V2 engine (extends OALKernel) + - Uses V2 parser for parsing + - Uses V2 enricher for metadata extraction + - Uses V2 generator with V2 templates + - **No V1 dependencies** in the pipeline Review Comment: Doc inconsistency: this summary says `OALEngineV2` “extends OALKernel”, but the current implementation directly implements `OALEngine`. Please update the summary to match the code so readers aren’t sent looking for a superclass that no longer exists. ########## oap-server/oal-rt/src/test/java/org/apache/skywalking/oal/v2/parser/RealOALScriptsTest.java: ########## @@ -0,0 +1,237 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oal.v2.parser; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oal.v2.model.MetricDefinition; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Test V2 parser with real OAL scripts from server-starter/resources/oal. + * + * This validates that V2 can parse all production OAL scripts. + */ +@Slf4j +public class RealOALScriptsTest { + + /** + * Find the OAL scripts directory in the project. + * + * Tries multiple paths to locate the directory: + * 1. From current working directory (Maven default) + * 2. From user.dir system property + * 3. Relative from this module + */ + private File findOALScriptsDir() { + String[] possiblePaths = { + "oap-server/server-starter/src/main/resources/oal", + "../server-starter/src/main/resources/oal", + "../../server-starter/src/main/resources/oal" + }; + + for (String path : possiblePaths) { + File dir = new File(path); + if (dir.exists() && dir.isDirectory()) { + log.debug("Found OAL scripts directory at: {}", dir.getAbsolutePath()); + return dir; + } + } + + throw new IllegalStateException("Could not find OAL scripts directory. Tried: " + + String.join(", ", possiblePaths)); + } + + /** + * Test parsing core.oal with V2. + * + * core.oal contains the main service and endpoint metrics. + */ + @Test + public void testParseCoreOAL() throws IOException { + File oalDir = findOALScriptsDir(); + File oalFile = new File(oalDir, "core.oal"); + assertTrue(oalFile.exists(), "core.oal not found at: " + oalFile.getAbsolutePath()); + + OALScriptParserV2 parser = OALScriptParserV2.parse(new FileReader(oalFile), "core.oal"); + + assertNotNull(parser); + assertTrue(parser.hasMetrics(), "core.oal should have metrics"); + + log.info("✅ Parsed core.oal: {} metrics", parser.getMetricsCount()); + + // Verify some expected metrics exist + boolean foundServiceRespTime = false; + boolean foundServiceSla = false; + boolean foundServiceCpm = false; + + for (MetricDefinition metric : parser.getMetrics()) { + String name = metric.getName(); + if (name.equals("service_resp_time")) { + foundServiceRespTime = true; + log.info(" - Found: service_resp_time (source: {}, function: {})", + metric.getSource().getName(), + metric.getAggregationFunction().getName()); + } else if (name.equals("service_sla")) { + foundServiceSla = true; + } else if (name.equals("service_cpm")) { + foundServiceCpm = true; + } + } + + assertTrue(foundServiceRespTime, "Should find service_resp_time"); + assertTrue(foundServiceSla, "Should find service_sla"); + assertTrue(foundServiceCpm, "Should find service_cpm"); + } + + /** + * Test parsing all OAL files from server-starter/resources/oal. + */ + @Test + public void testParseAllOALFiles() throws IOException { + File oalDir = findOALScriptsDir(); + String[] oalFiles = { + "core.oal", + "java-agent.oal", + "dotnet-agent.oal", + "browser.oal", + "mesh.oal", + "ebpf.oal", + "tcp.oal", + "cilium.oal", + "disable.oal" + }; + + int totalMetrics = 0; + int successCount = 0; + + for (String fileName : oalFiles) { + File oalFile = new File(oalDir, fileName); + assertTrue(oalFile.exists(), fileName + " not found at: " + oalFile.getAbsolutePath()); + + try { + OALScriptParserV2 parser = OALScriptParserV2.parse(new FileReader(oalFile), fileName); + int metricsCount = parser.getMetricsCount(); + totalMetrics += metricsCount; + successCount++; + + log.info("✅ Parsed {}: {} metrics, {} disabled sources", + fileName, + metricsCount, + parser.getDisabledSources().size()); + + } catch (Exception e) { + log.error("❌ Failed to parse {}: {}", fileName, e.getMessage(), e); + throw e; + } + } + + log.info("✅ Successfully parsed {}/{} OAL files, total {} metrics", + successCount, oalFiles.length, totalMetrics); + + assertTrue(successCount >= 5, "Should successfully parse at least 5 OAL files"); + assertTrue(totalMetrics > 50, "Should have at least 50 metrics total"); + } + + /** + * Test that V2 parser handles comments correctly. + */ + @Test + public void testCommentsInRealOAL() throws IOException { + File oalDir = findOALScriptsDir(); + File oalFile = new File(oalDir, "core.oal"); + if (!oalFile.exists()) { + log.warn("Skipping test - core.oal not found"); + return; + } + + // core.oal has line comments like: // Multiple values including p50, p75, p90, p95, p99 + OALScriptParserV2 parser = OALScriptParserV2.parse(new FileReader(oalFile), "core.oal"); Review Comment: This FileReader is not always closed on method exit. ########## oap-server/oal-rt/src/main/resources/code-templates-v2/metrics/deserialize.ftl: ########## @@ -0,0 +1,29 @@ +public void deserialize(org.apache.skywalking.oap.server.core.remote.grpc.proto.RemoteData remoteData) { +<#if serializeFields.stringFields?? && serializeFields.stringFields?size gt 0> + <#list serializeFields.stringFields as field> + if (remoteData.getDataStrings(${field_index}) != "") { + ${field.setter}(remoteData.getDataStrings(${field_index})); + } Review Comment: Generated Java uses `remoteData.getDataStrings(i) != ""` which compares String references rather than contents. This can incorrectly treat empty strings as non-empty and set fields to "" instead of leaving them unset/null. Use a content check like `.isEmpty()` / `.equals("")` instead of `!=`. ########## oap-server/oal-rt/V2_IMPLEMENTATION_SUMMARY.md: ########## @@ -0,0 +1,386 @@ +# OAL Engine V2 - Implementation Summary + +## Overview + +OAL Engine V2 is now the **only OAL implementation** after V1 has been completely removed. V2 provides a clean architecture with immutable models, type safety, and comprehensive testing. + +## Architecture + +``` +OAL Script → OALScriptParserV2 → MetricDefinition (immutable) → +MetricDefinitionEnricher → CodeGenModel → OALClassGeneratorV2 → +FreeMarker Templates → Javassist → Generated Classes +``` + +### Key Design Principles + +1. **Immutable Models**: All data models are immutable and thread-safe +2. **Type Safety**: Strongly-typed filter values and function arguments +3. **Clean Separation**: Parse → Enrich → Generate pipeline +4. **Independent Package Structure**: All V2 code organized under `org.apache.skywalking.oal.v2` +5. **Comprehensive Testing**: 70+ unit tests covering all components + +## What Was Implemented + +### 1. V2 Immutable Model Classes ✅ +**Location**: `org.apache.skywalking.oal.v2.model` + +All model classes are immutable, type-safe, and use builder pattern: +- **SourceLocation**: Track source file location for error reporting +- **SourceReference**: Immutable source reference (e.g., `Service.latency`) +- **FilterOperator**: Type-safe enum for filter operators (==, >, <, like, etc.) +- **FilterValue**: Strongly-typed filter values (NUMBER, STRING, BOOLEAN, NULL, ARRAY) +- **FunctionArgument**: Type-safe function arguments (LITERAL, ATTRIBUTE, EXPRESSION) +- **FilterExpression**: Immutable filter expression +- **FunctionCall**: Function call with typed arguments +- **MetricDefinition**: Complete metric definition + +### 2. V2 Parser ✅ +**Location**: `org.apache.skywalking.oal.v2.parser` + +- **OALListenerV2**: ANTLR listener converting parse tree to V2 models + - Handles all OAL grammar rules + - Builds immutable objects + - Preserves source locations + - Comprehensive javadoc with parsing flow examples + +- **OALScriptParserV2**: Facade for parsing OAL scripts + - Simple API: `OALScriptParserV2.parse(script)` + - Returns list of `MetricDefinition` objects + - Helper methods: `hasMetrics()`, `getMetricsCount()` + +### 3. V2 Registry ✅ +**Location**: `org.apache.skywalking.oal.v2.registry` + +- **MetricsFunctionRegistry**: Interface for function registry +- **MetricsFunctionDescriptor**: Function metadata +- **DefaultMetricsFunctionRegistry**: Classpath scanning implementation + - Scans for `@MetricsFunction` annotations + - Finds `@Entrance` methods + - Lazy initialization with thread-safety + +### 4. V2 Code Generation Pipeline ✅ +**Location**: `org.apache.skywalking.oal.v2.generator` + +- **CodeGenModel**: V2 data model for templates + - Contains all information needed by Freemarker templates + - Separate classes for source fields, persistent fields, entrance methods + - Serialization field models + +- **MetricDefinitionEnricher**: V2 enricher (replaces V1's DeepAnalysis) + - Looks up metrics function classes + - Extracts source columns from metadata + - Finds entrance methods via reflection + - Builds entrance method arguments + - Collects persistent fields from @Column annotations + - Generates serialization fields + +- **OALClassGeneratorV2**: V2 class generator + - Uses **V2 templates** from `code-templates-v2/` + - Generates metrics classes with Javassist + - Generates builder classes + - Generates dispatcher classes + - Completely independent from V1 generator + +### 5. V2 Engine ✅ +**Location**: `org.apache.skywalking.oal.v2` + +- **OALEngineV2**: Main V2 engine (extends OALKernel) + - Uses V2 parser for parsing + - Uses V2 enricher for metadata extraction + - Uses V2 generator with V2 templates + - **No V1 dependencies** in the pipeline + +### 6. V2 Freemarker Templates ✅ +**Location**: `src/main/resources/code-templates-v2/` + +**Metrics templates** (9 files): +- `id.ftl` - Storage ID generation +- `hashCode.ftl` - Hash code for metrics identity +- `remoteHashCode.ftl` - Remote hash code +- `equals.ftl` - Equality comparison +- `serialize.ftl` - Remote data serialization +- `deserialize.ftl` - Remote data deserialization +- `getMeta.ftl` - Metadata (table name) +- `toHour.ftl` - Hour-level aggregation +- `toDay.ftl` - Day-level aggregation + +**Dispatcher templates** (2 files): +- `doMetrics.ftl` - Individual metric dispatcher method +- `dispatch.ftl` - Main dispatch method + +**Builder templates** (2 files): +- `entity2Storage.ftl` - Convert entity to storage format +- `storage2Entity.ftl` - Convert storage to entity format + +### 7. Comprehensive Tests ✅ +**Location**: `org.apache.skywalking.oal.v2.*Test` + +**Model Tests** (28 tests): +- FilterValueTest (10 tests) - All value types with type safety +- FilterExpressionTest (10 tests) - All operators and value combinations +- FunctionCallTest (8 tests) - Arguments, equality, type safety + +**Parser Tests** (15 tests): +- OALScriptParserV2Test - Complete integration tests + - Simple metrics + - Wildcards + - Filters (number, boolean, string) + - Multiple chained filters + - Function arguments + - Decorators + - Multiple metrics + - Disable statements + - All comparison operators + +**Total**: 62 tests (43 V2 + 19 V1), all passing ✅ + +## Package Structure + +After V1 removal, all code is organized under `org.apache.skywalking.oal.v2`: + +| Package | Purpose | +|---------|---------| +| `model` | Immutable data models (MetricDefinition, FilterExpression, etc.) | +| `parser` | OAL script parsing (OALListenerV2, OALScriptParserV2) | +| `generator` | Code generation (MetricDefinitionEnricher, OALClassGeneratorV2, CodeGenModel) | +| `metadata` | Metadata utilities (SourceColumnsFactory, FilterMatchers, MetricsHolder) | +| `util` | Code generation utilities (ClassMethodUtil, TypeCastUtil) | +| `registry` | Function registry (MetricsFunctionRegistry) | + +## Key Benefits + +### 1. Type Safety + +### 2. Type Safety Review Comment: Duplicate section headings: `### 1. Type Safety` and `### 2. Type Safety` are both present. This looks like an accidental duplication and makes the benefits section harder to read; please merge/rename one of them. ```suggestion ``` ########## oap-server/oal-rt/src/test/java/org/apache/skywalking/oal/v2/parser/RealOALScriptsTest.java: ########## @@ -0,0 +1,237 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oal.v2.parser; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oal.v2.model.MetricDefinition; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Test V2 parser with real OAL scripts from server-starter/resources/oal. + * + * This validates that V2 can parse all production OAL scripts. + */ +@Slf4j +public class RealOALScriptsTest { + + /** + * Find the OAL scripts directory in the project. + * + * Tries multiple paths to locate the directory: + * 1. From current working directory (Maven default) + * 2. From user.dir system property + * 3. Relative from this module + */ + private File findOALScriptsDir() { + String[] possiblePaths = { + "oap-server/server-starter/src/main/resources/oal", + "../server-starter/src/main/resources/oal", + "../../server-starter/src/main/resources/oal" + }; + + for (String path : possiblePaths) { + File dir = new File(path); + if (dir.exists() && dir.isDirectory()) { + log.debug("Found OAL scripts directory at: {}", dir.getAbsolutePath()); + return dir; + } + } + + throw new IllegalStateException("Could not find OAL scripts directory. Tried: " + + String.join(", ", possiblePaths)); + } + + /** + * Test parsing core.oal with V2. + * + * core.oal contains the main service and endpoint metrics. + */ + @Test + public void testParseCoreOAL() throws IOException { + File oalDir = findOALScriptsDir(); + File oalFile = new File(oalDir, "core.oal"); + assertTrue(oalFile.exists(), "core.oal not found at: " + oalFile.getAbsolutePath()); + + OALScriptParserV2 parser = OALScriptParserV2.parse(new FileReader(oalFile), "core.oal"); + + assertNotNull(parser); + assertTrue(parser.hasMetrics(), "core.oal should have metrics"); + + log.info("✅ Parsed core.oal: {} metrics", parser.getMetricsCount()); + + // Verify some expected metrics exist + boolean foundServiceRespTime = false; + boolean foundServiceSla = false; + boolean foundServiceCpm = false; + + for (MetricDefinition metric : parser.getMetrics()) { + String name = metric.getName(); + if (name.equals("service_resp_time")) { + foundServiceRespTime = true; + log.info(" - Found: service_resp_time (source: {}, function: {})", + metric.getSource().getName(), + metric.getAggregationFunction().getName()); + } else if (name.equals("service_sla")) { + foundServiceSla = true; + } else if (name.equals("service_cpm")) { + foundServiceCpm = true; + } + } + + assertTrue(foundServiceRespTime, "Should find service_resp_time"); + assertTrue(foundServiceSla, "Should find service_sla"); + assertTrue(foundServiceCpm, "Should find service_cpm"); + } + + /** + * Test parsing all OAL files from server-starter/resources/oal. + */ + @Test + public void testParseAllOALFiles() throws IOException { + File oalDir = findOALScriptsDir(); + String[] oalFiles = { + "core.oal", + "java-agent.oal", + "dotnet-agent.oal", + "browser.oal", + "mesh.oal", + "ebpf.oal", + "tcp.oal", + "cilium.oal", + "disable.oal" + }; + + int totalMetrics = 0; + int successCount = 0; + + for (String fileName : oalFiles) { + File oalFile = new File(oalDir, fileName); + assertTrue(oalFile.exists(), fileName + " not found at: " + oalFile.getAbsolutePath()); + + try { + OALScriptParserV2 parser = OALScriptParserV2.parse(new FileReader(oalFile), fileName); + int metricsCount = parser.getMetricsCount(); + totalMetrics += metricsCount; + successCount++; + + log.info("✅ Parsed {}: {} metrics, {} disabled sources", + fileName, + metricsCount, + parser.getDisabledSources().size()); + + } catch (Exception e) { + log.error("❌ Failed to parse {}: {}", fileName, e.getMessage(), e); + throw e; + } + } + + log.info("✅ Successfully parsed {}/{} OAL files, total {} metrics", + successCount, oalFiles.length, totalMetrics); + + assertTrue(successCount >= 5, "Should successfully parse at least 5 OAL files"); + assertTrue(totalMetrics > 50, "Should have at least 50 metrics total"); + } + + /** + * Test that V2 parser handles comments correctly. + */ + @Test + public void testCommentsInRealOAL() throws IOException { + File oalDir = findOALScriptsDir(); + File oalFile = new File(oalDir, "core.oal"); + if (!oalFile.exists()) { + log.warn("Skipping test - core.oal not found"); + return; + } + + // core.oal has line comments like: // Multiple values including p50, p75, p90, p95, p99 + OALScriptParserV2 parser = OALScriptParserV2.parse(new FileReader(oalFile), "core.oal"); + + assertNotNull(parser); + assertTrue(parser.hasMetrics()); + + log.info("✅ V2 parser correctly handles comments in OAL scripts"); + } + + /** + * Test parsing OAL with decorators (real feature in core.oal). + */ + @Test + public void testDecoratorsInRealOAL() throws IOException { + File oalDir = findOALScriptsDir(); + File oalFile = new File(oalDir, "core.oal"); + if (!oalFile.exists()) { + log.warn("Skipping test - core.oal not found"); + return; + } + + OALScriptParserV2 parser = OALScriptParserV2.parse(new FileReader(oalFile), "core.oal"); Review Comment: This FileReader is not always closed on method exit. ########## oap-server/oal-rt/src/main/java/org/apache/skywalking/oal/v2/model/FilterExpression.java: ########## @@ -0,0 +1,185 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oal.v2.model; + +import java.util.List; +import java.util.Objects; +import lombok.Getter; + +/** + * Immutable representation of a filter expression. + * + * Examples: + * <pre> + * latency > 100 → fieldName="latency", operator=">", value=NUMBER:100 + * status == true → fieldName="status", operator="==", value=BOOLEAN:true + * name like "serv%" → fieldName="name", operator="like", value=STRING:"serv%" + * tag["key"] != null → fieldName="tag[key]", operator="!=", value=NULL + * code in [404, 500] → fieldName="code", operator="in", value=ARRAY:[404,500] + * </pre> + */ +@Getter +public final class FilterExpression { + private final String fieldName; + private final FilterOperator operator; + private final FilterValue value; + private final SourceLocation location; + + private FilterExpression(Builder builder) { + this.fieldName = Objects.requireNonNull(builder.fieldName, "fieldName cannot be null"); + this.operator = Objects.requireNonNull(builder.operator, "operator cannot be null"); + this.value = Objects.requireNonNull(builder.value, "value cannot be null"); + this.location = builder.location != null ? builder.location : SourceLocation.UNKNOWN; + } + + public static Builder builder() { + return new Builder(); + } + + /** + * Create filter expression with auto-detection of value type. + */ + public static FilterExpression of(String fieldName, String operatorStr, Object value) { + FilterValue filterValue = convertToFilterValue(value); + return builder() + .fieldName(fieldName) + .operator(FilterOperator.fromString(operatorStr)) + .value(filterValue) + .build(); + } + + private static FilterValue convertToFilterValue(Object value) { + if (value == null) { + return FilterValue.ofNull(); + } else if (value instanceof Long || value instanceof Integer) { + return FilterValue.ofNumber(((Number) value).longValue()); + } else if (value instanceof Double || value instanceof Float) { + return FilterValue.ofNumber(((Number) value).doubleValue()); + } else if (value instanceof String) { + return FilterValue.ofString((String) value); + } else if (value instanceof Boolean) { + return FilterValue.ofBoolean((Boolean) value); + } else if (value instanceof List) { + return FilterValue.ofArray((List<?>) value); + } else if (value instanceof FilterValue) { + return (FilterValue) value; + } else { + throw new IllegalArgumentException("Unsupported value type: " + value.getClass()); + } + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + FilterExpression that = (FilterExpression) o; + return Objects.equals(fieldName, that.fieldName) && + operator == that.operator && + Objects.equals(value, that.value); + } + + @Override + public int hashCode() { + return Objects.hash(fieldName, operator, value); + } + + @Override + public String toString() { + return fieldName + " " + operator.getSymbol() + " " + value; + } + + public static class Builder { + private String fieldName; + private FilterOperator operator; + private FilterValue value; + private SourceLocation location; + + public Builder fieldName(String fieldName) { + this.fieldName = fieldName; + return this; + } + + public Builder operator(FilterOperator operator) { + this.operator = operator; + return this; + } + + public Builder operator(String operatorStr) { + this.operator = FilterOperator.fromString(operatorStr); + return this; + } + + public Builder value(FilterValue value) { + this.value = value; + return this; + } + + /** + * Set value with auto-type detection. + */ + public Builder value(Object value) { Review Comment: Method Builder.value(..) could be confused with overloaded method [value](1), since dispatch depends on static types. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
