wu-sheng commented on code in PR #13737:
URL: https://github.com/apache/skywalking/pull/13737#discussion_r2912349313
##########
docs/en/concepts-and-designs/lal.md:
##########
@@ -145,6 +145,89 @@ filter {
Extractors aim to extract metadata from the logs. The metadata can be a
service name, a service instance name, an
endpoint name, or even a trace ID, all of which can be associated with the
existing traces and metrics.
+#### Local variables (`def`)
+
+You can use `def` to declare local variables in the extractor (or at the
filter level). This is useful when an
+expression is reused multiple times, or when you want to break a long chain
into readable steps.
+
+The syntax is:
+
+```
+def variableName = expression
+def variableName = expression as TypeName
+```
+
+The variable type is inferred from the initializer expression at compile time.
`def` is not limited to JSON — it works
+with any value access expression whose type is resolvable on the classpath,
including protobuf getter chains,
+`log.*` fields, and Gson JSON method chains. Subsequent method calls on the
variable are validated at compile time
+against the inferred type.
Review Comment:
Correct observation. Filter-level def lives in execute(), extractor/sink are
separate private methods with their own localVars scope. This is intentional —
filter-level def is useful for conditions before extractor/sink blocks, not for
passing variables into them. Will clarify the docs.
##########
oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/rt/LalRuntimeHelper.java:
##########
@@ -180,6 +188,115 @@ public String tagValue(final String key) {
return "";
}
+ // ==================== JSON conversion (for def variables)
====================
+ //
+ // toJsonObject: converts to JsonObject. Overloaded for Struct, String,
Object.
+ // toJsonArray: converts to JsonArray. Overloaded for String, Object.
+ //
+ // Primary use case: extract JWT claims from envoy ALS filter_metadata:
+ // def jwt = toJson(parsed?.commonProperties?.metadata
+ // ?.filterMetadataMap?.get("envoy.filters.http.jwt_authn"))
+ // tag 'email':
jwt?.getAsJsonObject("payload")?.get("email")?.getAsString()
+
+ /**
+ * Converts a protobuf {@link Struct} to a Gson {@link JsonObject}.
+ * Recursively converts nested Struct/Value/ListValue to Gson types.
+ */
+ public JsonObject toJsonObject(final Struct struct) {
+ if (struct == null) {
+ return null;
+ }
+ return structToJsonObject(struct);
+ }
+
+ /**
+ * Parses a JSON string into a {@link JsonObject}.
+ */
+ public JsonObject toJsonObject(final String s) {
+ if (s == null || s.isEmpty()) {
+ return null;
+ }
+ final JsonElement el = JsonParser.parseString(s);
+ return el.isJsonObject() ? el.getAsJsonObject() : null;
+ }
+
+ /**
+ * Fallback for {@code Map.get()} erasure — dispatches to typed overloads.
+ */
+ public JsonObject toJsonObject(final Object obj) {
+ if (obj == null) {
+ return null;
+ }
+ if (obj instanceof Struct) {
+ return toJsonObject((Struct) obj);
+ }
+ if (obj instanceof String) {
+ return toJsonObject((String) obj);
+ }
+ return null;
+ }
Review Comment:
Valid — same as the docs comment. The Map case is missing in the
toJsonObject(Object) dispatcher. Will add Map handling.
##########
oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALExpressionExecutionTest.java:
##########
@@ -318,6 +330,30 @@ private void executeAndAssert(
}
}
+ /**
+ * Reads a field from the output builder if present, otherwise falls back
+ * to the proto value. The output builder (e.g. LogBuilder) stores
+ * extracted fields that haven't been flushed to LogData proto yet.
+ */
+ private static String getOutputOrProto(final Object outputObj,
+ final String fieldName,
+ final String protoValue) {
+ if (outputObj == null) {
+ return protoValue;
+ }
+ try {
+ final Field f = outputObj.getClass().getDeclaredField(fieldName);
+ f.setAccessible(true);
+ final Object val = f.get(outputObj);
+ if (val != null) {
+ return val.toString();
+ }
+ } catch (NoSuchFieldException | IllegalAccessException ignored) {
+ // Field not on this output type — fall back to proto
+ }
+ return protoValue;
+ }
Review Comment:
The test helper works correctly for the current output types. The field
names (service, serviceInstance, endpoint, layer) match LogBuilder which is the
default outputType. Custom outputTypes that use different setter names would
need their own test assertions — this test focuses on the standard LAL path.
##########
oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALBlockCodegen.java:
##########
@@ -1140,6 +1172,283 @@ static String generateExtraLogAccess(
return prevVar;
}
+ // ==================== Def statement codegen ====================
+
+ static void generateDefStatement(final StringBuilder sb,
+ final LALScriptModel.DefStatement def,
+ final LALClassGenerator.GenCtx genCtx) {
+ final LALScriptModel.ValueAccess init = def.getInitializer();
+ final String varName = def.getVarName();
+ final String javaVar = "_d" + genCtx.localVarCounter++;
+
+ // Determine type and generate initializer expression
+ Class<?> resolvedType;
+ final StringBuilder initExpr = new StringBuilder();
+
+ if (init.getFunctionCallName() != null
+ && BUILTIN_FUNCTIONS.containsKey(init.getFunctionCallName())) {
+ // Built-in function: toJson(...), toJsonArray(...)
+ final Object[] info =
BUILTIN_FUNCTIONS.get(init.getFunctionCallName());
+ final String helperMethod = (String) info[0];
+ resolvedType = (Class<?>) info[1];
+
+ initExpr.append(helperMethod).append("(");
+ if (!init.getFunctionCallArgs().isEmpty()) {
+ generateValueAccess(initExpr,
+ init.getFunctionCallArgs().get(0).getValue(), genCtx);
+ } else {
+ initExpr.append("null");
+ }
+ initExpr.append(")");
+ } else {
Review Comment:
Valid minor improvement. Will add arity check — toJson/toJsonArray should
require exactly 1 argument.
##########
oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALBlockCodegen.java:
##########
@@ -1140,6 +1172,283 @@ static String generateExtraLogAccess(
return prevVar;
}
+ // ==================== Def statement codegen ====================
+
+ static void generateDefStatement(final StringBuilder sb,
+ final LALScriptModel.DefStatement def,
+ final LALClassGenerator.GenCtx genCtx) {
+ final LALScriptModel.ValueAccess init = def.getInitializer();
+ final String varName = def.getVarName();
+ final String javaVar = "_d" + genCtx.localVarCounter++;
+
+ // Determine type and generate initializer expression
+ Class<?> resolvedType;
+ final StringBuilder initExpr = new StringBuilder();
+
+ if (init.getFunctionCallName() != null
+ && BUILTIN_FUNCTIONS.containsKey(init.getFunctionCallName())) {
+ // Built-in function: toJson(...), toJsonArray(...)
+ final Object[] info =
BUILTIN_FUNCTIONS.get(init.getFunctionCallName());
+ final String helperMethod = (String) info[0];
+ resolvedType = (Class<?>) info[1];
+
+ initExpr.append(helperMethod).append("(");
+ if (!init.getFunctionCallArgs().isEmpty()) {
+ generateValueAccess(initExpr,
+ init.getFunctionCallArgs().get(0).getValue(), genCtx);
+ } else {
+ initExpr.append("null");
+ }
+ initExpr.append(")");
+ } else {
+ // General value access — type inferred from lastResolvedType
+ generateValueAccess(initExpr, init, genCtx);
+ resolvedType = genCtx.lastResolvedType != null
+ ? genCtx.lastResolvedType : Object.class;
+ // Box primitive types for local variable declarations
+ if (resolvedType.isPrimitive()) {
+ final String boxName =
LALCodegenHelper.boxTypeName(resolvedType);
+ if (boxName != null) {
+ try {
+ resolvedType = Class.forName("java.lang." + boxName);
+ } catch (ClassNotFoundException ignored) {
+ // keep primitive
+ }
+ }
+ }
+ }
+
+ // Apply explicit type cast if specified (e.g., "as
com.example.MyType")
+ final String castType = def.getCastType();
+ if (castType != null && !castType.isEmpty()) {
+ // Resolve the cast type — primitive wrapper names are handled,
+ // anything else is treated as a FQCN
+ final Class<?> castClass = resolveDefCastType(castType);
+ if (castClass != null) {
+ resolvedType = castClass;
+ }
+ }
+
+ // Register in local vars for later reference
+ genCtx.localVars.put(varName,
+ new LALClassGenerator.LocalVarInfo(javaVar, resolvedType));
+
+ // Emit declaration (placed at method top via localVarDecls)
+ genCtx.localVarDecls.append(" ").append(resolvedType.getName())
+ .append(" ").append(javaVar).append(";\n");
+ genCtx.localVarLvtVars.add(new String[]{
+ javaVar, "L" + resolvedType.getName().replace('.', '/') + ";"
+ });
+
+ // Emit assignment in body (at the point where def appears)
+ sb.append(" ").append(javaVar).append(" = ");
+ if (castType != null && !castType.isEmpty()) {
+ sb.append("(").append(resolvedType.getName()).append(") ");
+ }
+ sb.append(initExpr).append(";\n");
+ }
+
+ /**
+ * Resolves a cast type string to a {@link Class}.
+ * Handles the four built-in type names ({@code String}, {@code Long},
+ * {@code Integer}, {@code Boolean}) and fully qualified class names.
+ */
+ private static Class<?> resolveDefCastType(final String castType) {
+ switch (castType) {
+ case "String":
+ return String.class;
+ case "Long":
+ return Long.class;
+ case "Integer":
+ return Integer.class;
+ case "Boolean":
+ return Boolean.class;
+ default:
+ try {
+ return Class.forName(castType);
+ } catch (ClassNotFoundException e) {
+ throw new IllegalArgumentException(
+ "def cast type not found on classpath: " + castType,
e);
+ }
+ }
+ }
+
+ // ==================== Def variable chain codegen ====================
+
+ /**
+ * Generates typed method-chain access on a def variable.
+ * Uses reflection to resolve each method/field call and track types.
+ *
+ * @param sb output buffer
+ * @param localVar the def variable info (java var name + resolved type)
+ * @param chain the chain segments after the variable name
+ * @param genCtx codegen context
+ */
+ static void generateDefVarChain(
+ final StringBuilder sb,
+ final LALClassGenerator.LocalVarInfo localVar,
+ final List<LALScriptModel.ValueAccessSegment> chain,
+ final LALClassGenerator.GenCtx genCtx) {
+ if (chain.isEmpty()) {
+ sb.append(localVar.javaVarName);
+ genCtx.lastResolvedType = localVar.resolvedType;
+ return;
+ }
+
+ String prevExpr = localVar.javaVarName;
+ Class<?> currentType = localVar.resolvedType;
+ boolean canBeNull = true;
+
+ for (int i = 0; i < chain.size(); i++) {
+ final LALScriptModel.ValueAccessSegment seg = chain.get(i);
+ final boolean isLast = i == chain.size() - 1;
+
+ if (seg instanceof LALScriptModel.MethodSegment) {
+ final LALScriptModel.MethodSegment ms =
+ (LALScriptModel.MethodSegment) seg;
+ final String methodName = ms.getName();
+
+ // Resolve method on currentType via reflection
+ final java.lang.reflect.Method method =
+ resolveMethod(currentType, methodName, ms.getArguments());
+ if (method == null) {
+ throw new IllegalArgumentException(
+ "Cannot resolve method " + currentType.getSimpleName()
+ + "." + methodName + "() in def variable chain");
+ }
+ final Class<?> returnType = method.getReturnType();
+ final String args = generateMethodArgs(ms.getArguments());
+
+ if (ms.isSafeNav() && canBeNull) {
+ if (isLast && returnType.isPrimitive()) {
+ // Primitive return with null guard
+ final String boxName =
+ LALCodegenHelper.boxTypeName(returnType);
+ prevExpr = "(" + prevExpr + " == null ? null : "
+ + boxName + ".valueOf(" + prevExpr + "."
+ + methodName + "(" + args + ")))";
+ currentType = returnType;
+ } else {
+ prevExpr = "(" + prevExpr + " == null ? null : "
+ + prevExpr + "." + methodName + "(" + args + "))";
+ currentType = returnType;
+ canBeNull = true;
+ }
+ } else {
+ prevExpr = prevExpr + "." + methodName + "(" + args + ")";
+ currentType = returnType;
+ canBeNull = !returnType.isPrimitive();
+ }
Review Comment:
Not an issue in practice. The prevExpr for def var chains starts as a simple
variable name (e.g. `_config`) and each safe-nav step wraps it in one ternary.
The chain depth in real LAL scripts is typically 2-4 levels. The generated code
is readable and JIT-friendly. Introducing temp variables would add complexity
for negligible benefit.
##########
oap-server/analyzer/log-analyzer/CLAUDE.md:
##########
@@ -105,6 +110,68 @@ All generated methods include a `LocalVariableTable`
attribute for debugger/deco
LVT entries are added via `PrivateMethod` inner class which carries both
source code and variable descriptors.
+## Local Variables (`def`)
+
+The `def` keyword declares local variables in the extractor (or filter level).
The grammar rule:
+
+```
+defStatement: DEF IDENTIFIER ASSIGN valueAccess typeCast? ;
+```
+
+The optional `typeCast` supports built-in types (`String`, `Long`, `Integer`,
`Boolean`) and
+fully qualified class names (`as com.example.MyType`). The FQCN is resolved
via `Class.forName()`
+at compile time. If not found, compilation fails with
`IllegalArgumentException`.
+
+### Type inference
+
+The variable type is inferred from the initializer expression:
+
+| Initializer | Inferred type | Generated code |
+|---|---|---|
+| `toJson(expr)` | `JsonObject` | `h.toJsonObject(expr)` |
+| `toJsonArray(expr)` | `JsonArray` | `h.toJsonArray(expr)` |
+| General value access | Last resolved type via reflection | Standard value
access codegen |
+
+Built-in functions are registered in `BUILTIN_FUNCTIONS` map in
`LALBlockCodegen`.
+
+### Chained def variables
+
+A `def` variable can be initialized from another `def` variable's method chain:
+
+```
+def jwt =
toJson(parsed?.commonProperties?.metadata?.filterMetadataMap?.get("envoy.filters.http.jwt_authn"))
+def payload = jwt?.getAsJsonObject("payload")
+tag 'email': payload?.get("email")?.getAsString()
+```
+
+The general value access path in `generateDefStatement()` recognizes `jwt` as
a def variable,
+delegates to `generateDefVarChain()` which uses reflection to resolve the
chain, and
+`genCtx.lastResolvedType` captures the resolved type (`JsonObject` in this
case).
+
+### Runtime helpers
+
+`LalRuntimeHelper` provides `toJsonObject()` and `toJsonArray()` overloads:
+
+| Method | Input type | Conversion |
+|---|---|---|
+| `toJsonObject(String)` | JSON string |
`JsonParser.parseString().getAsJsonObject()` |
+| `toJsonObject(Map)` | Map (from JSON/YAML parser) | Recursive Gson
conversion |
+| `toJsonObject(Struct)` | Protobuf `Struct` | Recursive field conversion
preserving nested structures |
+| `toJsonObject(Object)` | Any (fallback) | Delegates to above based on
runtime type |
+| `toJsonArray(String)` | JSON array string |
`JsonParser.parseString().getAsJsonArray()` |
+| `toJsonArray(Object)` | Any (fallback) | String fallback |
+
Review Comment:
Valid — same root cause as the other Map comments. Will add Map support in
toJsonObject(Object) and update docs to match.
--
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]
