This is an automated email from the ASF dual-hosted git repository. wusheng pushed a commit to branch oal-v2 in repository https://gitbox.apache.org/repos/asf/skywalking.git
commit 9eefa9224bacb88cba277ab3534463ee7fbf9cae Author: Wu Sheng <[email protected]> AuthorDate: Wed Feb 11 20:45:31 2026 +0800 Fix OAL V2 nested boolean attribute handling in function arguments The code generator was producing invalid code like `source.isProtocol.success()` instead of `source.getProtocol().isSuccess()` when a nested boolean attribute (e.g., `protocol.success`) was used as a function argument in OAL metrics like `apdex(serviceName, protocol.success)`. This fix handles nested attributes in @Arg parameters by: - Detecting dot-separated attributes - Splitting into a list and using ClassMethodUtil.toIsMethod(List) or toGetMethod(List) - Generating correct chained getter calls Added regression tests for both enrichment and bytecode generation. Co-Authored-By: Claude Opus 4.5 <[email protected]> --- .../oal/v2/generator/MetricDefinitionEnricher.java | 18 ++++++-- .../v2/generator/MetricDefinitionEnricherTest.java | 47 +++++++++++++++++++ .../oal/v2/generator/OALClassGeneratorV2Test.java | 54 ++++++++++++++++++++++ 3 files changed, 115 insertions(+), 4 deletions(-) diff --git a/oap-server/oal-rt/src/main/java/org/apache/skywalking/oal/v2/generator/MetricDefinitionEnricher.java b/oap-server/oal-rt/src/main/java/org/apache/skywalking/oal/v2/generator/MetricDefinitionEnricher.java index de67f5a24f..12225419be 100644 --- a/oap-server/oal-rt/src/main/java/org/apache/skywalking/oal/v2/generator/MetricDefinitionEnricher.java +++ b/oap-server/oal-rt/src/main/java/org/apache/skywalking/oal/v2/generator/MetricDefinitionEnricher.java @@ -273,10 +273,20 @@ public class MetricDefinitionEnricher { } else if (funcArg.isAttribute()) { // Use isXxx() for boolean parameters, getXxx() otherwise String attr = funcArg.asAttribute(); - String accessor = parameterType.equals(boolean.class) - ? ClassMethodUtil.toIsMethod(attr) - : ClassMethodUtil.toGetMethod(attr); - String expression = "source." + accessor + "()"; + String expression; + // Handle nested attributes by splitting on dot + if (attr.contains(".")) { + List<String> attributes = java.util.Arrays.asList(attr.split("\\.")); + String accessor = parameterType.equals(boolean.class) + ? ClassMethodUtil.toIsMethod(attributes) + : ClassMethodUtil.toGetMethod(attributes); + expression = "source." + accessor; + } else { + String accessor = parameterType.equals(boolean.class) + ? ClassMethodUtil.toIsMethod(attr) + : ClassMethodUtil.toGetMethod(attr); + expression = "source." + accessor + "()"; + } // Cast numeric types to match parameter type if (parameterType.equals(long.class)) { expression = "(long)(" + expression + ")"; diff --git a/oap-server/oal-rt/src/test/java/org/apache/skywalking/oal/v2/generator/MetricDefinitionEnricherTest.java b/oap-server/oal-rt/src/test/java/org/apache/skywalking/oal/v2/generator/MetricDefinitionEnricherTest.java index 0bb6220100..9b59a8cf62 100644 --- a/oap-server/oal-rt/src/test/java/org/apache/skywalking/oal/v2/generator/MetricDefinitionEnricherTest.java +++ b/oap-server/oal-rt/src/test/java/org/apache/skywalking/oal/v2/generator/MetricDefinitionEnricherTest.java @@ -24,6 +24,7 @@ import org.apache.skywalking.oal.v2.parser.OALScriptParserV2; import org.apache.skywalking.oap.server.core.source.DefaultScopeDefine; import org.apache.skywalking.oap.server.core.source.Endpoint; import org.apache.skywalking.oap.server.core.source.K8SService; +import org.apache.skywalking.oap.server.core.source.K8SServiceInstance; import org.apache.skywalking.oap.server.core.source.Service; import org.apache.skywalking.oap.server.core.source.ServiceRelation; import org.apache.skywalking.oap.server.core.source.TCPService; @@ -58,6 +59,7 @@ public class MetricDefinitionEnricherTest { listener.notify(Endpoint.class); listener.notify(ServiceRelation.class); listener.notify(K8SService.class); + listener.notify(K8SServiceInstance.class); listener.notify(TCPService.class); } catch (RuntimeException e) { // Scopes may already be registered by other tests @@ -246,6 +248,51 @@ public class MetricDefinitionEnricherTest { assertTrue(foundIsStatus, "Expected isStatus() accessor for boolean argument"); } + /** + * Test nested boolean attribute as function argument: apdex(serviceName, protocol.success) + * Should generate: source.getProtocol().isSuccess() + * NOT: source.isProtocol.success() + * + * This is a regression test for the bug where nested boolean attributes were not + * handled correctly in function arguments. + */ + @Test + public void testNestedBooleanArgAccessor() throws Exception { + String oal = "kubernetes_service_instance_apdex = from(K8SServiceInstance.protocol.http.latency)" + + ".filter(detectPoint == DetectPoint.SERVER)" + + ".filter(type == \"protocol\")" + + ".filter(protocol.type == \"http\")" + + ".apdex(serviceName, protocol.success);"; + + MetricDefinitionEnricher enricher = new MetricDefinitionEnricher(SOURCE_PACKAGE, METRICS_PACKAGE); + OALScriptParserV2 parser = OALScriptParserV2.parse(oal); + MetricDefinition metric = parser.getMetrics().get(0); + CodeGenModel model = enricher.enrich(metric); + + // Verify entrance method args + assertNotNull(model.getEntranceMethod()); + List<Object> argsExpressions = model.getEntranceMethod().getArgsExpressions(); + + // Find the protocol.success argument (should use getProtocol().isSuccess()) + boolean foundNestedBoolean = false; + for (Object arg : argsExpressions) { + String argStr = arg.toString(); + if (argStr.contains("getProtocol") && argStr.contains("isSuccess")) { + foundNestedBoolean = true; + // Verify it's using the correct format: getProtocol().isSuccess() + assertTrue(argStr.contains("getProtocol()"), + "Expected getProtocol() but got: " + argStr); + assertTrue(argStr.contains("isSuccess()"), + "Expected isSuccess() but got: " + argStr); + } + // Should NOT generate invalid code like isProtocol.success() + assertFalse(argStr.contains("isProtocol.success"), + "Should not generate isProtocol.success(): " + argStr); + } + assertTrue(foundNestedBoolean, + "Expected getProtocol().isSuccess() accessor for nested boolean argument"); + } + /** * Test type casting for numeric @Arg parameters: labelAvg(name, duration) * where entrance method expects long but source field might be int. diff --git a/oap-server/oal-rt/src/test/java/org/apache/skywalking/oal/v2/generator/OALClassGeneratorV2Test.java b/oap-server/oal-rt/src/test/java/org/apache/skywalking/oal/v2/generator/OALClassGeneratorV2Test.java index 4a34e3ed9a..de27ffd416 100644 --- a/oap-server/oal-rt/src/test/java/org/apache/skywalking/oal/v2/generator/OALClassGeneratorV2Test.java +++ b/oap-server/oal-rt/src/test/java/org/apache/skywalking/oal/v2/generator/OALClassGeneratorV2Test.java @@ -38,6 +38,7 @@ 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.source.Endpoint; import org.apache.skywalking.oap.server.core.source.K8SService; +import org.apache.skywalking.oap.server.core.source.K8SServiceInstance; import org.apache.skywalking.oap.server.core.source.Service; import org.apache.skywalking.oap.server.core.source.ServiceRelation; import org.apache.skywalking.oap.server.core.source.TCPService; @@ -80,6 +81,7 @@ public class OALClassGeneratorV2Test { listener.notify(Endpoint.class); listener.notify(ServiceRelation.class); listener.notify(K8SService.class); + listener.notify(K8SServiceInstance.class); listener.notify(TCPService.class); } catch (RuntimeException e) { // Scopes may already be registered by other tests @@ -450,6 +452,58 @@ public class OALClassGeneratorV2Test { ); } + /** + * Test code generation with nested boolean attribute as function argument. + * Uses K8SServiceInstance source with apdex(serviceName, protocol.success). + * + * This is a regression test for the bug where nested boolean attributes were not + * handled correctly in function arguments. The bug generated invalid code like: + * source.isProtocol.success() + * instead of: + * source.getProtocol().isSuccess() + * + * @throws Exception if code generation fails + */ + @Test + public void testNestedBooleanAttributeInFunctionArgument() throws Exception { + // This OAL pattern uses apdex(serviceName, protocol.success) where: + // - protocol is a nested object (Protocol class) + // - success is a boolean field on Protocol + // The generated code must be: source.getProtocol().isSuccess() + String oal = "kubernetes_service_instance_apdex = from(K8SServiceInstance.protocol.http.latency)" + + ".filter(detectPoint == DetectPoint.SERVER)" + + ".filter(type == \"protocol\")" + + ".filter(protocol.type == \"http\")" + + ".apdex(serviceName, protocol.success);"; + + OALClassGeneratorV2 generator = createGenerator(); + MetricDefinitionEnricher enricher = new MetricDefinitionEnricher(SOURCE_PACKAGE, METRICS_PACKAGE); + + List<Class> metricsClasses = new ArrayList<>(); + List<Class> dispatcherClasses = new ArrayList<>(); + + // This will throw OALCompileException if the generated code is invalid + // (e.g., "no such class: source.isProtocol") + generateClasses(oal, generator, enricher, metricsClasses, dispatcherClasses); + + // Verify metrics class generated + assertEquals(1, metricsClasses.size()); + assertEquals(1, dispatcherClasses.size()); + + Class<?> apdexClass = findClassBySimpleName(metricsClasses, "KubernetesServiceInstanceApdexMetrics"); + assertNotNull(apdexClass, "KubernetesServiceInstanceApdexMetrics should be generated"); + + // Verify dispatcher class + Class<?> dispatcherClass = dispatcherClasses.get(0); + assertEquals("K8SServiceInstanceDispatcher", dispatcherClass.getSimpleName()); + assertTrue(SourceDispatcher.class.isAssignableFrom(dispatcherClass)); + + // Verify doMetrics method exists + Method doApdex = findMethodByName(dispatcherClass, "doKubernetesServiceInstanceApdex"); + assertNotNull(doApdex, "doKubernetesServiceInstanceApdex method should be generated"); + assertTrue(Modifier.isPrivate(doApdex.getModifiers())); + } + /** * Verify all required methods exist on a metrics class. */
