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 5d8bf29882e4a0f4d1bd0b248f0d9099c647d553 Author: Wu Sheng <[email protected]> AuthorDate: Tue Mar 3 00:05:16 2026 +0800 Fix LAL sampler rateLimit in if-blocks and replace generic helpers with typed methods Fix rateLimit() calls inside if-blocks within sampler generating empty bytecode by handling the samplerContent grammar alternative in LALScriptParser.visitIfBody(). Replace generic LalRuntimeHelper.safeCall() and isTruthy() with specific typed methods: isTrue() for Boolean conditions, isNotEmpty() for String non-emptiness, toString() and trim() for null-safe navigation — making generated code explicit about intended type semantics. Co-Authored-By: Claude Opus 4.6 <[email protected]> --- oap-server/analyzer/log-analyzer/CLAUDE.md | 6 ++- .../analyzer/v2/compiler/LALClassGenerator.java | 20 ++++++--- .../log/analyzer/v2/compiler/LALScriptParser.java | 18 ++++++++ .../analyzer/v2/compiler/rt/LalRuntimeHelper.java | 51 ++++++++++++++-------- .../v2/compiler/LALClassGeneratorTest.java | 10 +++-- 5 files changed, 75 insertions(+), 30 deletions(-) diff --git a/oap-server/analyzer/log-analyzer/CLAUDE.md b/oap-server/analyzer/log-analyzer/CLAUDE.md index 2f1ed2ace6..cd8f7078e5 100644 --- a/oap-server/analyzer/log-analyzer/CLAUDE.md +++ b/oap-server/analyzer/log-analyzer/CLAUDE.md @@ -140,9 +140,11 @@ called by generated code via FQCN. This avoids duplicating helper methods in eve | `toInt(Object)` | Number/String → `int` | | `toStr(Object)` | Null-safe `String.valueOf()` (returns `null` for null input) | | `toBool(Object)` | Boolean coercion | -| `isTruthy(Object)` | Groovy-style truthiness (null/empty/zero → false) | +| `isTrue(Object)` | Boolean truthiness (null → false, Boolean → value, String → parseBoolean) | +| `isNotEmpty(Object)` | String non-emptiness (null → false, String → !isEmpty, Object → !toString().isEmpty) | +| `toString(Object)` | Null-safe toString for `?.toString()` safe navigation | +| `trim(Object)` | Null-safe trim for `?.trim()` safe navigation | | `tagValue(Binding, String)` | Log tag lookup via protobuf `KeyStringValuePair` | -| `safeCall(Object, String)` | Safe navigation `?.method()` (toString, trim, isEmpty) | ## Data-Driven Execution Tests diff --git a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGenerator.java b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGenerator.java index 8a396ce8bb..31d68e12a3 100644 --- a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGenerator.java +++ b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGenerator.java @@ -1054,10 +1054,13 @@ public final class LALClassGenerator { ((LALScriptModel.NotCondition) cond).getInner()); sb.append(")"); } else if (cond instanceof LALScriptModel.ExprCondition) { - sb.append(H).append(".isTruthy("); + final String ct = ((LALScriptModel.ExprCondition) cond).getCastType(); + final String method = "Boolean".equals(ct) || "boolean".equals(ct) + ? ".isTrue(" : ".isNotEmpty("; + sb.append(H).append(method); generateValueAccessObj(sb, ((LALScriptModel.ExprCondition) cond).getExpr(), - ((LALScriptModel.ExprCondition) cond).getCastType()); + ct); sb.append(")"); } } @@ -1214,9 +1217,16 @@ public final class LALClassGenerator { final LALScriptModel.MethodSegment ms = (LALScriptModel.MethodSegment) seg; if (ms.isSafeNav()) { - // Safe navigation: null-safe method call - current = H + ".safeCall(" + current + ", \"" - + escapeJava(ms.getName()) + "\")"; + // Safe navigation: dispatch to specific helper + final String mn = ms.getName(); + if ("toString".equals(mn)) { + current = H + ".toString(" + current + ")"; + } else if ("trim".equals(mn)) { + current = H + ".trim(" + current + ")"; + } else { + throw new IllegalArgumentException( + "Unsupported safe-nav method: ?." + mn + "()"); + } } else { if (ms.getArguments().isEmpty()) { current = current + "." + ms.getName() + "()"; diff --git a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALScriptParser.java b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALScriptParser.java index 8d227c34fe..3aebe99a78 100644 --- a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALScriptParser.java +++ b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALScriptParser.java @@ -507,6 +507,24 @@ public final class LALScriptParser { stmts.add((FilterStatement) visitSampledTraceField(stc)); } } + // Handle samplerContent alternative (rateLimit blocks inside if within sampler) + final LALParser.SamplerContentContext sc = ctx.samplerContent(); + if (sc != null) { + final List<SamplerContent> samplerItems = new ArrayList<>(); + for (final LALParser.RateLimitBlockContext rlc : sc.rateLimitBlock()) { + final String id = stripQuotes(rlc.rateLimitId().getText()); + final long rpm = Long.parseLong( + rlc.rateLimitContent().NUMBER().getText()); + final List<InterpolationPart> idParts = parseInterpolation(id); + samplerItems.add(new RateLimitBlock(id, idParts, rpm)); + } + for (final LALParser.IfStatementContext isc : sc.ifStatement()) { + samplerItems.add((SamplerContent) visitIfStatement(isc)); + } + if (!samplerItems.isEmpty()) { + stmts.add((FilterStatement) new SamplerBlock(samplerItems)); + } + } return stmts; } diff --git a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/rt/LalRuntimeHelper.java b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/rt/LalRuntimeHelper.java index 3db295e401..0bb825d8c4 100644 --- a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/rt/LalRuntimeHelper.java +++ b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/rt/LalRuntimeHelper.java @@ -78,7 +78,11 @@ public final class LalRuntimeHelper { return obj != null; } - public static boolean isTruthy(final Object obj) { + /** + * Boolean truthiness: null is false, Boolean delegates, String + * parses, anything else is true. + */ + public static boolean isTrue(final Object obj) { if (obj == null) { return false; } @@ -86,14 +90,25 @@ public final class LalRuntimeHelper { return ((Boolean) obj).booleanValue(); } if (obj instanceof String) { - return !((String) obj).isEmpty(); - } - if (obj instanceof Number) { - return ((Number) obj).doubleValue() != 0; + return Boolean.parseBoolean((String) obj); } return true; } + /** + * String non-emptiness: null is false, otherwise checks that + * toString() is non-empty. + */ + public static boolean isNotEmpty(final Object obj) { + if (obj == null) { + return false; + } + if (obj instanceof String) { + return !((String) obj).isEmpty(); + } + return !obj.toString().isEmpty(); + } + public static String tagValue(final Binding b, final String key) { final List dl = b.log().getTags().getDataList(); for (int i = 0; i < dl.size(); i++) { @@ -105,19 +120,17 @@ public final class LalRuntimeHelper { return ""; } - public static Object safeCall(final Object obj, final String method) { - if (obj == null) { - return null; - } - if ("toString".equals(method)) { - return obj.toString(); - } - if ("trim".equals(method)) { - return obj.toString().trim(); - } - if ("isEmpty".equals(method)) { - return Boolean.valueOf(obj.toString().isEmpty()); - } - return obj.toString(); + /** + * Null-safe toString: returns null when input is null. + */ + public static String toString(final Object obj) { + return obj == null ? null : obj.toString(); + } + + /** + * Null-safe trim: returns null when input is null. + */ + public static String trim(final Object obj) { + return obj == null ? null : obj.toString().trim(); } } diff --git a/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorTest.java b/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorTest.java index 297940e746..0f86f99359 100644 --- a/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorTest.java +++ b/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorTest.java @@ -203,16 +203,18 @@ class LALClassGeneratorTest { } @Test - void generateSourceSafeNavMethodEmitsSafeCall() { + void generateSourceSafeNavMethodEmitsSpecificHelper() { final String source = generator.generateSource( "filter {\n" + " if (parsed?.flags?.toString()) {\n" + " sink {}\n" + " }\n" + "}"); - // Safe method calls should use safeCall helper - assertTrue(source.contains("safeCall("), - "Expected safeCall for safe nav method but got: " + source); + // Safe method calls should emit specific helpers, not generic safeCall + assertTrue(source.contains("LalRuntimeHelper.toString("), + "Expected toString helper for safe nav method but got: " + source); + assertTrue(source.contains("LalRuntimeHelper.isNotEmpty("), + "Expected isNotEmpty for ExprCondition but got: " + source); } // ==================== ProcessRegistry static calls ====================
