This is an automated email from the ASF dual-hosted git repository.
wu-sheng pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/skywalking.git
The following commit(s) were added to refs/heads/master by this push:
new 137935d364 Fix: MAL `expPrefix` now applies to every metric source in
`exp`, not just the leading one. (#13875)
137935d364 is described below
commit 137935d364be83d4b4438730540856cc35a2d5c8
Author: Wan Kai <[email protected]>
AuthorDate: Fri May 15 11:03:25 2026 +0800
Fix: MAL `expPrefix` now applies to every metric source in `exp`, not just
the leading one. (#13875)
---
docs/en/changes/changes.md | 1 +
.../oap/meter/analyzer/v2/MetricConvert.java | 23 +++---
.../analyzer/v2/compiler/MALScriptParser.java | 93 ++++++++++++++++++++++
.../v2/compiler/MALClassGeneratorScopeTest.java | 61 +++++++-------
4 files changed, 141 insertions(+), 37 deletions(-)
diff --git a/docs/en/changes/changes.md b/docs/en/changes/changes.md
index d73c6077a8..20b1fb9ec5 100644
--- a/docs/en/changes/changes.md
+++ b/docs/en/changes/changes.md
@@ -188,6 +188,7 @@
* Custom `Layer`s can be declared without modifying the OAP source — via an
operator-managed `layer-extensions.yml`, inline `layerDefinitions:` block in a
MAL or LAL rule file, or a plugin extension. UI dashboard templates for new
layers are auto-discovered from the `ui-initialized-templates/` directory.
Recommended ordinal range for external layers is `>= 1000`; conflicting names
or ordinals are reported at boot.
* LAL: support full arithmetic (`+`, `-`, `*`, `/`) on numeric operands and
fix the original bug where `(tag("x") as Integer) + (tag("y") as Integer)` was
treated as string concatenation — expressions like `input_tokens +
output_tokens < 10000` produced the concatenated string `"2589115"` rather than
the integer sum `2704`, so token-threshold conditions never triggered `abort
{}`. Operand types are now inferred from explicit casts (`as Integer` / `as
Long` / `as Float` / `as Double`), ty [...]
* Fix: `avgHistogramPercentile` / `sumHistogramPercentile` meter functions
reported the smallest finite bucket boundary (e.g. `10` for OTel
`gen_ai_server_request_duration` whose `le` is rewritten from `0.01s` → `10ms`)
for every rank when no samples were observed in any bucket. The percentile
loop's `count >= roof` check matched on the first sorted bucket because both
sides were `0`. `calculate()` now short-circuits to `0` for every rank when the
windowed total is `0`.
+* Fix: MAL `expPrefix` now applies to every metric source in `exp`, not just
the leading one. Previously the prefix was spliced after the first `.`, so
secondary metrics inside arguments (e.g. the divisor in
`a.sum(['s']).safeDiv(b.sum(['s']))`) silently skipped the prefix — a rule like
envoy-ai-gateway's `request_latency_avg` (`sum / count`) would tag-rewrite only
the dividend. The injection is now AST-aware: every bare-IDENTIFIER metric
source is wrapped, while downsampling-type consta [...]
#### UI
* Add mobile menu icon and i18n labels for the iOS layer.
diff --git
a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/v2/MetricConvert.java
b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/v2/MetricConvert.java
index b3539b2975..4353339755 100644
---
a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/v2/MetricConvert.java
+++
b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/v2/MetricConvert.java
@@ -33,7 +33,7 @@ import java.util.stream.IntStream;
import java.util.stream.Stream;
import lombok.Getter;
import lombok.extern.slf4j.Slf4j;
-import org.apache.commons.lang3.StringUtils;
+import org.apache.skywalking.oap.meter.analyzer.v2.compiler.MALScriptParser;
import org.apache.skywalking.oap.meter.analyzer.v2.dsl.FilterExpression;
import org.apache.skywalking.oap.meter.analyzer.v2.dsl.SampleFamily;
import org.apache.skywalking.oap.server.core.analysis.meter.MeterSystem;
@@ -294,15 +294,18 @@ public class MetricConvert {
return new FilterExpression(filterText, "filter", yamlSource, pool,
targetClassLoader);
}
- private String formatExp(final String expPrefix, String expSuffix, String
exp) {
- String ret = exp;
- if (!Strings.isNullOrEmpty(expPrefix)) {
- ret = String.format("(%s.%s)", StringUtils.substringBefore(exp,
"."), expPrefix);
- final String after = StringUtils.substringAfter(exp, ".");
- if (!Strings.isNullOrEmpty(after)) {
- ret = String.format("(%s.%s)", ret, after);
- }
- }
+ /**
+ * Build the final MAL expression from {@code expPrefix} + {@code exp} +
{@code expSuffix}.
+ *
+ * <p>The prefix is injected at every metric source via
+ * {@link MALScriptParser#injectExpPrefix(String, String)} so secondary
+ * metric refs inside arguments (e.g. the {@code b} in
+ * {@code a.sum(['s']).safeDiv(b.sum(['s']))}) are not skipped — a naive
+ * splice-after-first-dot would only rewrite {@code a}.
+ */
+ public static String formatExp(final String expPrefix, final String
expSuffix,
+ final String exp) {
+ String ret = MALScriptParser.injectExpPrefix(exp, expPrefix);
if (!Strings.isNullOrEmpty(expSuffix)) {
ret = String.format("(%s).%s", ret, expSuffix);
}
diff --git
a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/v2/compiler/MALScriptParser.java
b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/v2/compiler/MALScriptParser.java
index e37f285456..43897451be 100644
---
a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/v2/compiler/MALScriptParser.java
+++
b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/v2/compiler/MALScriptParser.java
@@ -19,11 +19,13 @@ package
org.apache.skywalking.oap.meter.analyzer.v2.compiler;
import java.util.ArrayList;
import java.util.Collections;
+import java.util.Comparator;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.antlr.v4.runtime.BaseErrorListener;
import org.antlr.v4.runtime.ParserRuleContext;
+import org.antlr.v4.runtime.tree.ParseTree;
import org.antlr.v4.runtime.tree.TerminalNode;
import org.antlr.v4.runtime.CharStreams;
import org.antlr.v4.runtime.CommonTokenStream;
@@ -86,6 +88,97 @@ public final class MALScriptParser {
private MALScriptParser() {
}
+ /**
+ * Splice {@code expPrefix} into every metric source in {@code exp}.
+ *
+ * <p>The MAL {@code expPrefix} (e.g. {@code tag({tags -> tags.service =
...})})
+ * is a method-call snippet that must apply to every metric reference, not
+ * only the first one. A naive string splice at the first dot misses
metrics
+ * nested in arguments — e.g. the second metric in
+ * {@code a.sum(['s']).safeDiv(b.sum(['s']))}.
+ *
+ * <p>This walker parses {@code exp}, finds every {@code primary} whose
child
+ * is a bare {@code IDENTIFIER} (i.e. a metric leaf — not a function call
or
+ * parenthesised group), and replaces it with {@code (<id>.<expPrefix>)}.
+ * Downsampling-type constants ({@code SUM}, {@code AVG}, {@code LATEST},
+ * {@code SUM_PER_MIN}, {@code MAX}, {@code MIN}) look syntactically
identical
+ * to metric sources but are recognised by name and skipped — wrapping them
+ * with the prefix would turn them into bogus SampleFamily references.
+ *
+ * @param exp the MAL expression to rewrite (must parse successfully)
+ * @param expPrefix the method-call snippet to inject (e.g. {@code
tag({...})})
+ * @return the rewritten expression, or {@code exp} unchanged when
+ * {@code expPrefix} is null or empty
+ */
+ public static String injectExpPrefix(final String exp, final String
expPrefix) {
+ if (expPrefix == null || expPrefix.isEmpty()) {
+ return exp;
+ }
+ final MALLexer lexer = new MALLexer(CharStreams.fromString(exp));
+ final CommonTokenStream tokens = new CommonTokenStream(lexer);
+ final MALParser parser = new MALParser(tokens);
+ final List<String> errors = new ArrayList<>();
+ parser.removeErrorListeners();
+ parser.addErrorListener(new BaseErrorListener() {
+ @Override
+ public void syntaxError(final Recognizer<?, ?> recognizer,
+ final Object offendingSymbol,
+ final int line,
+ final int charPositionInLine,
+ final String msg,
+ final RecognitionException e) {
+ errors.add(line + ":" + charPositionInLine + " " + msg);
+ }
+ });
+ final MALParser.ExpressionContext tree = parser.expression();
+ if (!errors.isEmpty()) {
+ throw new IllegalArgumentException(
+ "MAL expression parsing failed while injecting expPrefix: "
+ + String.join("; ", errors) + " in expression: " + exp);
+ }
+ final List<int[]> ranges = new ArrayList<>();
+ collectMetricSourceRanges(tree, ranges);
+ // Splice from right to left so earlier indices remain valid.
+ ranges.sort(Comparator.comparingInt((int[] r) -> r[0]).reversed());
+ final StringBuilder sb = new StringBuilder(exp);
+ for (final int[] range : ranges) {
+ final String name = sb.substring(range[0], range[1] + 1);
+ sb.replace(range[0], range[1] + 1, "(" + name + "." + expPrefix +
")");
+ }
+ return sb.toString();
+ }
+
+ private static void collectMetricSourceRanges(final ParseTree node,
+ final List<int[]> ranges) {
+ if (node instanceof MALParser.PrimaryContext) {
+ final MALParser.PrimaryContext primary =
(MALParser.PrimaryContext) node;
+ // Three primary alternatives: bare IDENTIFIER (metric source),
+ // functionCall, and L_PAREN additiveExpression R_PAREN. Only the
+ // first is a metric ref.
+ if (primary.IDENTIFIER() != null
+ && primary.functionCall() == null
+ && primary.L_PAREN() == null) {
+ final TerminalNode id = primary.IDENTIFIER();
+ final String text = id.getText();
+ // Bare-identifier downsampling constants (SUM, AVG, LATEST,
+ // SUM_PER_MIN, MAX, MIN) parse as IDENTIFIER primaries but
+ // codegen treats them as enum values via
+ // MALCodegenHelper.isDownsamplingType — skip them so we don't
+ // turn them into bogus SampleFamily refs.
+ if (!MALCodegenHelper.isDownsamplingType(text)) {
+ ranges.add(new int[]{
+ id.getSymbol().getStartIndex(),
+ id.getSymbol().getStopIndex()
+ });
+ }
+ return;
+ }
+ }
+ for (int i = 0; i < node.getChildCount(); i++) {
+ collectMetricSourceRanges(node.getChild(i), ranges);
+ }
+ }
+
/**
* Pre-process expression to convert Groovy regex literals used as method
* arguments into string literals. E.g., {@code split(/\|/, -1)} becomes
diff --git
a/oap-server/analyzer/meter-analyzer/src/test/java/org/apache/skywalking/oap/meter/analyzer/v2/compiler/MALClassGeneratorScopeTest.java
b/oap-server/analyzer/meter-analyzer/src/test/java/org/apache/skywalking/oap/meter/analyzer/v2/compiler/MALClassGeneratorScopeTest.java
index 2c994920d7..022d4248dd 100644
---
a/oap-server/analyzer/meter-analyzer/src/test/java/org/apache/skywalking/oap/meter/analyzer/v2/compiler/MALClassGeneratorScopeTest.java
+++
b/oap-server/analyzer/meter-analyzer/src/test/java/org/apache/skywalking/oap/meter/analyzer/v2/compiler/MALClassGeneratorScopeTest.java
@@ -18,16 +18,19 @@
package org.apache.skywalking.oap.meter.analyzer.v2.compiler;
import javassist.ClassPool;
+import org.apache.skywalking.oap.meter.analyzer.v2.MetricConvert;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
/**
- * Full-expression compilation tests combining expPrefix + exp + expSuffix via
formatExp.
- * Covers endpoint, service, instance, serviceRelation scope suffixes,
- * forEach closures with if/else-if/else chains, sum() with >10 label keys,
- * and tag() closure chained with service() suffix.
+ * Full-expression compilation tests combining expPrefix + exp + expSuffix via
+ * {@link MetricConvert#formatExp(String, String, String)}. Covers endpoint,
service,
+ * instance, serviceRelation scope suffixes, forEach closures with
if/else-if/else
+ * chains, sum() with >10 label keys, and tag() closure prefix + chained
service
+ * suffix (envoy-ai-gateway shape).
*/
class MALClassGeneratorScopeTest {
@@ -38,31 +41,9 @@ class MALClassGeneratorScopeTest {
generator = new MALClassGenerator(new ClassPool(true));
}
- /**
- * Simulates MetricConvert.formatExp() to build the full expression
- * from expPrefix + exp + expSuffix.
- */
- private static String formatExp(String expPrefix, String expSuffix, String
exp) {
- String ret = exp;
- if (expPrefix != null && !expPrefix.isEmpty()) {
- int dotIdx = exp.indexOf('.');
- if (dotIdx > 0) {
- ret = String.format("(%s.%s)", exp.substring(0, dotIdx),
expPrefix);
- String after = exp.substring(dotIdx + 1);
- if (!after.isEmpty()) {
- ret = String.format("(%s.%s)", ret, after);
- }
- }
- }
- if (expSuffix != null && !expSuffix.isEmpty()) {
- ret = String.format("(%s).%s", ret, expSuffix);
- }
- return ret;
- }
-
private void compileRule(String name, String expPrefix, String expSuffix,
String exp)
throws Exception {
- final String full = formatExp(expPrefix, expSuffix, exp);
+ final String full = MetricConvert.formatExp(expPrefix, expSuffix, exp);
assertNotNull(generator.compile(name, full));
}
@@ -152,6 +133,32 @@ class MALClassGeneratorScopeTest {
}
}
+ @Test
+ void tagClosurePrefixWithServiceSuffix() throws Exception {
+ // Mirrors otel-rules/envoy-ai-gateway/gateway-service.yaml shape,
exercising
+ // the request_latency_avg rule which divides two metrics — every
metric
+ // source must receive the tag prefix, not just the leading one:
+ // filter: { tags -> tags.job_name == 'envoy-ai-gateway' }
+ // expPrefix: tag({tags -> tags.service = tags.service_name + '|'
+ tags.cluster_name})
+ // expSuffix: service(['service'], Layer.ENVOY_AI_GATEWAY)
+ // metricPrefix: meter_envoy_ai_gw
+ // request_latency_avg = (sum / count over PT1M) * 1000 (ms)
+ final String prefix = "tag({tags -> tags.service = tags.service_name +
'|' + tags.cluster_name})";
+ final String suffix = "service(['service'], Layer.ENVOY_AI_GATEWAY)";
+ final String exp =
"gen_ai_server_request_duration_sum.sum(['service_name']).increase('PT1M')"
+ +
".safeDiv(gen_ai_server_request_duration_count.sum(['service_name']).increase('PT1M'))
* 1000";
+ final String formatted = MetricConvert.formatExp(prefix, suffix, exp);
+ // Both the _sum and _count metric sources must carry the tag closure.
+ assertTrue(
+ formatted.contains("(gen_ai_server_request_duration_sum." + prefix
+ ")"),
+ "expected prefix on _sum metric, got: " + formatted);
+ assertTrue(
+ formatted.contains("(gen_ai_server_request_duration_count." +
prefix + ")"),
+ "expected prefix on _count metric (inside safeDiv), got: " +
formatted);
+
assertNotNull(generator.compile("meter_envoy_ai_gw_request_latency_avg",
formatted));
+ assertNotNull(generator.compileFilter("{ tags -> tags.job_name ==
'envoy-ai-gateway' }"));
+ }
+
@Test
void tagClosureChainedWithServiceSuffix() throws Exception {
final String suffix = "tag({tags -> tags.service = 'satellite::' +
tags.service}).service(['service'], Layer.SO11Y_SATELLITE)";
