You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by wu...@apache.org on 2026/03/03 12:56:12 UTC
(skywalking) 27/27: Improve LAL generated code readability for extraLogType proto getter chains
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 119441cce52e219273e96f241ebf13da72edb4a6
Author: Wu Sheng <wu...@foxmail.com>
AuthorDate: Tue Mar 3 20:55:29 2026 +0800
Improve LAL generated code readability for extraLogType proto getter chains
Cache the extraLog cast in a _p local variable and break safe-nav chains
into sequential _tN locals instead of deeply nested ternaries. Repeated
access to the same chain prefix reuses existing variables (dedup).
Co-Authored-By: Claude Opus 4.6 <no...@anthropic.com>
---
.../analyzer/v2/compiler/LALClassGenerator.java | 268 ++++++++++++++++-----
.../v2/compiler/LALClassGeneratorTest.java | 23 +-
2 files changed, 223 insertions(+), 68 deletions(-)
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 a721d8e973..1c2268f02d 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
@@ -95,6 +95,15 @@ public final class LALClassGenerator {
String lastNullChecks;
String lastRawChain;
+ // Per-method proto field variable caching (NONE + extraLogType only).
+ // Maps chain key ("response", "response.responseCode") to variable name ("_t0", "_t1").
+ // Enables dedup: the same chain accessed multiple times reuses the same variable.
+ final Map<String, String> protoVars = new HashMap<>();
+ final List<String[]> protoLvtVars = new ArrayList<>();
+ final StringBuilder protoVarDecls = new StringBuilder();
+ int protoVarCounter;
+ boolean usedProtoAccess;
+
GenCtx(final ParserType parserType, final Class<?> extraLogType) {
this.parserType = parserType;
this.extraLogType = extraLogType;
@@ -110,6 +119,36 @@ public final class LALClassGenerator {
lastNullChecks = null;
lastRawChain = null;
}
+
+ void resetProtoVars() {
+ protoVars.clear();
+ protoLvtVars.clear();
+ protoVarDecls.setLength(0);
+ protoVarCounter = 0;
+ usedProtoAccess = false;
+ }
+
+ Object[] saveProtoVarState() {
+ return new Object[]{
+ new HashMap<>(protoVars),
+ new ArrayList<>(protoLvtVars),
+ protoVarDecls.toString(),
+ protoVarCounter,
+ usedProtoAccess
+ };
+ }
+
+ @SuppressWarnings("unchecked")
+ void restoreProtoVarState(final Object[] state) {
+ protoVars.clear();
+ protoVars.putAll((Map<String, String>) state[0]);
+ protoLvtVars.clear();
+ protoLvtVars.addAll((List<String[]>) state[1]);
+ protoVarDecls.setLength(0);
+ protoVarDecls.append((String) state[2]);
+ protoVarCounter = (Integer) state[3];
+ usedProtoAccess = (Boolean) state[4];
+ }
}
public LALClassGenerator() {
@@ -279,11 +318,19 @@ public final class LALClassGenerator {
final javassist.CtMethod execMethod = CtNewMethod.make(executeBody, ctClass);
ctClass.addMethod(execMethod);
- addLocalVariableTable(execMethod, className, new String[][]{
- {"filterSpec", "L" + FILTER_SPEC.replace('.', '/') + ";"},
- {"ctx", "L" + EXEC_CTX.replace('.', '/') + ";"},
- {"h", "L" + H.replace('.', '/') + ";"}
- });
+
+ // Build LVT for execute(): params + h + optional _p and proto vars
+ final List<String[]> execLvt = new ArrayList<>();
+ execLvt.add(new String[]{"filterSpec", "L" + FILTER_SPEC.replace('.', '/') + ";"});
+ execLvt.add(new String[]{"ctx", "L" + EXEC_CTX.replace('.', '/') + ";"});
+ execLvt.add(new String[]{"h", "L" + H.replace('.', '/') + ";"});
+ if (genCtx.usedProtoAccess && genCtx.extraLogType != null) {
+ execLvt.add(new String[]{"_p",
+ "L" + genCtx.extraLogType.getName().replace('.', '/') + ";"});
+ execLvt.addAll(genCtx.protoLvtVars);
+ }
+ addLocalVariableTable(execMethod, className,
+ execLvt.toArray(new String[0][]));
writeClassFile(ctClass);
@@ -313,15 +360,28 @@ public final class LALClassGenerator {
private String generateExecuteMethod(final LALScriptModel model,
final GenCtx genCtx) {
+ genCtx.resetProtoVars();
+
+ // Generate body first so proto var declarations are collected
+ final StringBuilder bodyContent = new StringBuilder();
+ for (final LALScriptModel.FilterStatement stmt : model.getStatements()) {
+ generateFilterStatement(bodyContent, stmt, genCtx);
+ }
+
final StringBuilder sb = new StringBuilder();
sb.append("public void execute(").append(FILTER_SPEC)
.append(" filterSpec, ").append(EXEC_CTX).append(" ctx) {\n");
sb.append(" ").append(H).append(" h = new ").append(H).append("(ctx);\n");
- for (final LALScriptModel.FilterStatement stmt : model.getStatements()) {
- generateFilterStatement(sb, stmt, genCtx);
+ // Insert _p + proto var declarations if any proto field access was used
+ if (genCtx.usedProtoAccess && genCtx.extraLogType != null) {
+ final String elTypeName = genCtx.extraLogType.getName();
+ sb.append(" ").append(elTypeName).append(" _p = (")
+ .append(elTypeName).append(") h.ctx().extraLog();\n");
+ sb.append(genCtx.protoVarDecls);
}
+ sb.append(bodyContent);
sb.append("}\n");
return sb.toString();
}
@@ -382,21 +442,41 @@ public final class LALClassGenerator {
final LALScriptModel.ExtractorBlock block,
final GenCtx genCtx) {
final String methodName = genCtx.nextMethodName("extractor");
- final StringBuilder body = new StringBuilder();
- body.append("private void ").append(methodName).append("(")
- .append(EXTRACTOR_SPEC).append(" _e, ").append(H).append(" h) {\n");
+ final Object[] savedState = genCtx.saveProtoVarState();
+ genCtx.resetProtoVars();
+ // Generate body first to collect proto var declarations
+ final StringBuilder bodyContent = new StringBuilder();
final List<LALScriptModel.FilterStatement> extractorStmts = new ArrayList<>();
for (final LALScriptModel.ExtractorStatement es : block.getStatements()) {
extractorStmts.add((LALScriptModel.FilterStatement) es);
}
- generateExtractorBody(body, extractorStmts, genCtx);
+ generateExtractorBody(bodyContent, extractorStmts, genCtx);
+
+ // Assemble method with declarations before body
+ final StringBuilder body = new StringBuilder();
+ body.append("private void ").append(methodName).append("(")
+ .append(EXTRACTOR_SPEC).append(" _e, ").append(H).append(" h) {\n");
+
+ final List<String[]> lvtVars = new ArrayList<>();
+ lvtVars.add(new String[]{"_e", "L" + EXTRACTOR_SPEC.replace('.', '/') + ";"});
+ lvtVars.add(new String[]{"h", "L" + H.replace('.', '/') + ";"});
+
+ if (genCtx.usedProtoAccess && genCtx.extraLogType != null) {
+ final String elTypeName = genCtx.extraLogType.getName();
+ body.append(" ").append(elTypeName).append(" _p = (")
+ .append(elTypeName).append(") h.ctx().extraLog();\n");
+ body.append(genCtx.protoVarDecls);
+ lvtVars.add(new String[]{"_p", "L" + elTypeName.replace('.', '/') + ";"});
+ lvtVars.addAll(genCtx.protoLvtVars);
+ }
+ body.append(bodyContent);
body.append("}\n");
- genCtx.privateMethods.add(new PrivateMethod(body.toString(), new String[][]{
- {"_e", "L" + EXTRACTOR_SPEC.replace('.', '/') + ";"},
- {"h", "L" + H.replace('.', '/') + ";"}
- }));
+ genCtx.privateMethods.add(new PrivateMethod(
+ body.toString(), lvtVars.toArray(new String[0][])));
+
+ genCtx.restoreProtoVarState(savedState);
sb.append(" if (!ctx.shouldAbort()) {\n");
sb.append(" ").append(methodName).append("(filterSpec.extractor(), h);\n");
@@ -674,21 +754,41 @@ public final class LALClassGenerator {
final LALScriptModel.SinkBlock sink,
final GenCtx genCtx) {
final String methodName = genCtx.nextMethodName("sink");
- final StringBuilder body = new StringBuilder();
- body.append("private void ").append(methodName).append("(")
- .append(FILTER_SPEC).append(" _f, ").append(H).append(" h) {\n");
+ final Object[] savedState = genCtx.saveProtoVarState();
+ genCtx.resetProtoVars();
+ // Generate body first to collect proto var declarations
+ final StringBuilder bodyContent = new StringBuilder();
final List<LALScriptModel.FilterStatement> sinkStmts = new ArrayList<>();
for (final LALScriptModel.SinkStatement ss : sink.getStatements()) {
sinkStmts.add((LALScriptModel.FilterStatement) ss);
}
- generateSinkBody(body, sinkStmts, genCtx);
+ generateSinkBody(bodyContent, sinkStmts, genCtx);
+ // Assemble method with declarations before body
+ final StringBuilder body = new StringBuilder();
+ body.append("private void ").append(methodName).append("(")
+ .append(FILTER_SPEC).append(" _f, ").append(H).append(" h) {\n");
+
+ final List<String[]> lvtVars = new ArrayList<>();
+ lvtVars.add(new String[]{"_f", "L" + FILTER_SPEC.replace('.', '/') + ";"});
+ lvtVars.add(new String[]{"h", "L" + H.replace('.', '/') + ";"});
+
+ if (genCtx.usedProtoAccess && genCtx.extraLogType != null) {
+ final String elTypeName = genCtx.extraLogType.getName();
+ body.append(" ").append(elTypeName).append(" _p = (")
+ .append(elTypeName).append(") h.ctx().extraLog();\n");
+ body.append(genCtx.protoVarDecls);
+ lvtVars.add(new String[]{"_p", "L" + elTypeName.replace('.', '/') + ";"});
+ lvtVars.addAll(genCtx.protoLvtVars);
+ }
+
+ body.append(bodyContent);
body.append("}\n");
- genCtx.privateMethods.add(new PrivateMethod(body.toString(), new String[][]{
- {"_f", "L" + FILTER_SPEC.replace('.', '/') + ";"},
- {"h", "L" + H.replace('.', '/') + ";"}
- }));
+ genCtx.privateMethods.add(new PrivateMethod(
+ body.toString(), lvtVars.toArray(new String[0][])));
+
+ genCtx.restoreProtoVarState(savedState);
sb.append(" if (!ctx.shouldAbort()) {\n");
sb.append(" ").append(methodName).append("(filterSpec, h);\n");
@@ -1246,67 +1346,117 @@ public final class LALClassGenerator {
return call.toString();
}
+ /**
+ * Generates proto getter chain using local variables for readability and dedup.
+ *
+ * <p>Instead of deeply nested ternaries like:
+ * <pre>
+ * ((Type) h.ctx().extraLog()).getA() == null ? null
+ * : ((Type) h.ctx().extraLog()).getA().getB()
+ * </pre>
+ * generates sequential local variable declarations in {@code genCtx.protoVarDecls}:
+ * <pre>
+ * TypeA _t0 = _p == null ? null : _p.getA();
+ * TypeB _t1 = _t0 == null ? null : _t0.getB();
+ * </pre>
+ * and returns a simple variable reference ({@code _t1}).
+ *
+ * <p>Repeated access to the same chain prefix reuses existing variables (dedup).
+ */
private String generateExtraLogAccess(
final List<LALScriptModel.FieldSegment> fieldSegments,
final Class<?> extraLogType,
final GenCtx genCtx) {
+ genCtx.usedProtoAccess = true;
+
if (fieldSegments.isEmpty()) {
- return "h.ctx().extraLog()";
+ return "_p";
}
final String typeName = extraLogType.getName();
- final StringBuilder chainSb = new StringBuilder();
- chainSb.append("((").append(typeName)
- .append(") h.ctx().extraLog())");
-
- final List<String> nullChecks = new ArrayList<>();
+ final StringBuilder chainKey = new StringBuilder();
+ String prevVar = "_p";
Class<?> currentType = extraLogType;
- boolean lastIsPrimitive = false;
- String lastPrimitiveType = null;
+ boolean prevCanBeNull = true;
- for (final LALScriptModel.FieldSegment seg : fieldSegments) {
+ for (int i = 0; i < fieldSegments.size(); i++) {
+ final LALScriptModel.FieldSegment seg = fieldSegments.get(i);
final String field = seg.getName();
final String getterName = "get" + Character.toUpperCase(field.charAt(0))
+ field.substring(1);
- // ?. means: check the chain so far for null before calling this getter
- if (seg.isSafeNav()) {
- nullChecks.add(chainSb.toString() + " == null");
- }
-
+ final java.lang.reflect.Method getter;
try {
- final java.lang.reflect.Method getter =
- currentType.getMethod(getterName);
- chainSb.append(".").append(getterName).append("()");
- currentType = getter.getReturnType();
- lastIsPrimitive = currentType.isPrimitive();
- if (lastIsPrimitive) {
- lastPrimitiveType = boxTypeName(currentType);
- }
+ getter = currentType.getMethod(getterName);
} catch (NoSuchMethodException e) {
throw new IllegalArgumentException(
"Cannot resolve getter " + currentType.getSimpleName()
+ "." + getterName + "() for extraLogType "
+ typeName + ". Check the field path in the LAL rule.");
}
- }
+ final Class<?> returnType = getter.getReturnType();
- // Store metadata for callers that can optimize (e.g., numeric comparisons)
- genCtx.lastResolvedType = currentType;
- genCtx.lastRawChain = chainSb.toString();
- genCtx.lastNullChecks = nullChecks.isEmpty()
- ? null : String.join(" || ", nullChecks);
+ if (chainKey.length() > 0) {
+ chainKey.append(".");
+ }
+ chainKey.append(field);
+ final String key = chainKey.toString();
+ final boolean isLast = i == fieldSegments.size() - 1;
+
+ // Primitive final segment: return inline expression, no variable
+ if (isLast && returnType.isPrimitive()) {
+ final String rawAccess = prevVar + "." + getterName + "()";
+ genCtx.lastResolvedType = returnType;
+ genCtx.lastRawChain = rawAccess;
+ final String boxName = boxTypeName(returnType);
+ if (seg.isSafeNav() && prevCanBeNull) {
+ genCtx.lastNullChecks = prevVar + " == null";
+ return "(" + prevVar + " == null ? null : "
+ + boxName + ".valueOf(" + rawAccess + "))";
+ } else {
+ genCtx.lastNullChecks = null;
+ return boxName + ".valueOf(" + rawAccess + ")";
+ }
+ }
- // Default return: boxed + null-checked (safe for all callers)
- String result = chainSb.toString();
- if (lastIsPrimitive && lastPrimitiveType != null) {
- result = lastPrimitiveType + ".valueOf(" + result + ")";
- }
+ // Reuse existing variable (dedup)
+ final String existingVar = genCtx.protoVars.get(key);
+ if (existingVar != null) {
+ prevVar = existingVar;
+ currentType = returnType;
+ prevCanBeNull = true;
+ continue;
+ }
- if (!nullChecks.isEmpty()) {
- return "(" + genCtx.lastNullChecks + " ? null : " + result + ")";
+ // Create new local variable declaration
+ final String newVar = "_t" + genCtx.protoVarCounter++;
+ final String returnTypeName = returnType.getName();
+ if (seg.isSafeNav() && prevCanBeNull) {
+ genCtx.protoVarDecls.append(" ").append(returnTypeName)
+ .append(" ").append(newVar).append(" = ")
+ .append(prevVar).append(" == null ? null : ")
+ .append(prevVar).append(".").append(getterName).append("();\n");
+ prevCanBeNull = true;
+ } else {
+ genCtx.protoVarDecls.append(" ").append(returnTypeName)
+ .append(" ").append(newVar).append(" = ")
+ .append(prevVar).append(".").append(getterName).append("();\n");
+ prevCanBeNull = !returnType.isPrimitive();
+ }
+ genCtx.protoVars.put(key, newVar);
+ genCtx.protoLvtVars.add(new String[]{
+ newVar, "L" + returnTypeName.replace('.', '/') + ";"
+ });
+
+ prevVar = newVar;
+ currentType = returnType;
}
- return result;
+
+ // Non-primitive final result — null checks are in declarations
+ genCtx.lastResolvedType = currentType;
+ genCtx.lastRawChain = prevVar;
+ genCtx.lastNullChecks = null;
+ return prevVar;
}
private static String boxTypeName(final Class<?> primitiveType) {
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 e039e85564..83367ea9dc 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
@@ -537,19 +537,24 @@ class LALClassGeneratorTest {
final String source = generator.generateSource(dsl);
final String fqcn =
"io.envoyproxy.envoy.data.accesslog.v3.HTTPAccessLogEntry";
- // Direct getter chains, not getAt
+ // Proto field access uses _p local variable, not inline cast
assertTrue(source.contains(
- "((" + fqcn + ") h.ctx().extraLog()).getResponse()"),
- "Expected direct getter for parsed.response but got: " + source);
- assertTrue(source.contains(
- "((" + fqcn + ") h.ctx().extraLog()).getCommonProperties()"),
- "Expected direct getter for parsed.commonProperties but got: " + source);
+ fqcn + " _p = (" + fqcn + ") h.ctx().extraLog()"),
+ "Expected _p local variable for extraLog cast but got: " + source);
+ // Intermediate fields cached in _tN local variables
+ assertTrue(source.contains("_p.getResponse()"),
+ "Expected _p.getResponse() via cached variable but got: " + source);
+ assertTrue(source.contains("_p.getCommonProperties()"),
+ "Expected _p.getCommonProperties() via cached variable but got: " + source);
assertFalse(source.contains("getAt"),
"Should NOT contain getAt calls but got: " + source);
- // Safe navigation: null checks with == null
- assertTrue(source.contains("== null"),
+ // Safe navigation: null checks with == null on local variables
+ assertTrue(source.contains("_p == null ? null :"),
"Expected null checks for ?. safe navigation but got: " + source);
- // Numeric comparison: direct primitive, no h.toLong() boxing
+ // Dedup: _tN variables declared once and reused
+ assertTrue(source.contains("_t0") && source.contains("_t1"),
+ "Expected _tN local variables for chain dedup but got: " + source);
+ // Numeric comparison: direct primitive via _tN variable, no h.toLong()
assertTrue(source.contains(".getValue() < 400"),
"Expected direct primitive comparison without boxing but got: " + source);
assertFalse(source.contains("h.toLong"),