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 432d55f2a7 Fix MQE top_n raising a raw storage IO exception for an
invalid attribute (#13940)
432d55f2a7 is described below
commit 432d55f2a7de58d797a382e7df823edfdb745358
Author: 吴晟 Wu Sheng <[email protected]>
AuthorDate: Tue Jul 7 20:51:47 2026 +0800
Fix MQE top_n raising a raw storage IO exception for an invalid attribute
(#13940)
An MQE top_n(metric, N, order, attrX='value') whose attribute is not a
queryable
column of the metric reached the storage engine with a tag the measure does
not
define and failed there, surfacing the opaque "Internal IO exception, query
metrics error.". Attribute columns (attr0..attrN) exist only on decorated
metrics
(service_*/endpoint_*/kubernetes_service_*, set to the layer via OAL
.decorator())
and the MAL meter base; relations and database/cache/mq access carry none.
MQEVisitor now validates each attribute key against the metric's registered
queryable columns before the storage call and raises
IllegalExpressionException
(naming the attribute and metric), which the existing catch turns into a
descriptive MQE error instead of the opaque storage exception.
---
docs/en/changes/changes.md | 1 +
.../oap/query/graphql/mqe/rt/MQEVisitor.java | 55 ++++++++-
.../graphql/mqe/rt/MQEVisitorAttributeTest.java | 136 +++++++++++++++++++++
3 files changed, 191 insertions(+), 1 deletion(-)
diff --git a/docs/en/changes/changes.md b/docs/en/changes/changes.md
index 950d00b557..15839723ec 100644
--- a/docs/en/changes/changes.md
+++ b/docs/en/changes/changes.md
@@ -332,6 +332,7 @@
* Fix: the v2 MAL compiler now resolves custom layers referenced as
`Layer.NAME` in an expression. A custom layer declared through a
`layerDefinitions:` block (or `layer-extensions.yml` / the `LayerExtension`
SPI) has no generated `Layer.*` static field, so `service(['svc'],
Layer.IOT_FLEET)` previously failed code generation because `Layer` has no
`IOT_FLEET` field. The compiler now lowers every `Layer.NAME` static-field
reference to a runtime `Layer.nameOf("NAME")` registry lookup, so [...]
* Fix Envoy ALS rendering for the LAL live-debugger and the persisted log
`content`: an Istio metadata-exchange peer in
`common_properties.filter_state_objects` (legacy Wasm `wasm.*_peer` =
`Any{BytesValue}` wrapping a FlatBuffer, or modern `*_peer` = `Any{Struct}`) is
now decoded into the readable peer metadata (pod / namespace / labels) instead
of an opaque `jsonformat-failed` envelope or base64. The serialization is
hardened so a single un-printable field can no longer blank the whole [...]
* Surface the effective BanyanDB configuration (`bydb.yml` / `bydb-topn.yml`)
in the `/debugging/config/dump` admin API. Because the BanyanDB config moved to
a separate file in 10.2.0, a BanyanDB deployment previously showed an empty
`storage.banyandb` block in the dump; its post-environment-resolution values
are now merged into the same response under `storage.banyandb.*` (TopN rules
under `storage.banyandb.topN.*`), masked by the same secret-keyword list, via a
generic `ConfigDumpExten [...]
+* Fix: an MQE `top_n(metric, N, order, attrX='value')` query whose attribute
is not a column of the target metric now returns a descriptive MQE error
instead of a raw storage `IOException` surfaced as `Internal IO exception,
query metrics error.`. Attribute columns (`attr0..attrN`) exist only on
decorated metrics (`service_*` / `endpoint_*` / `kubernetes_service_*`, set to
the layer name via OAL `.decorator(...)`) and the MAL meter base; metrics such
as relations or database / cache / mq [...]
#### UI
* Add Airflow layer dashboards and menu i18n under Workflow Scheduler in
Horizon UI (SWIP-7).
diff --git
a/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/mqe/rt/MQEVisitor.java
b/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/mqe/rt/MQEVisitor.java
index 9fedf928fa..e176af6293 100644
---
a/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/mqe/rt/MQEVisitor.java
+++
b/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/mqe/rt/MQEVisitor.java
@@ -24,6 +24,8 @@ import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
import lombok.extern.slf4j.Slf4j;
import org.apache.skywalking.mqe.rt.exception.IllegalExpressionException;
import org.apache.skywalking.mqe.rt.grammar.MQEParser;
@@ -52,6 +54,9 @@ import
org.apache.skywalking.oap.server.core.query.type.debugging.DebuggingSpan;
import
org.apache.skywalking.oap.server.core.query.type.debugging.DebuggingTraceContext;
import org.apache.skywalking.oap.server.core.storage.annotation.Column;
import
org.apache.skywalking.oap.server.core.storage.annotation.ValueColumnMetadata;
+import org.apache.skywalking.oap.server.core.storage.model.IModelManager;
+import org.apache.skywalking.oap.server.core.storage.model.Model;
+import org.apache.skywalking.oap.server.core.storage.model.ModelColumn;
import org.apache.skywalking.oap.server.library.module.ModuleManager;
import org.apache.skywalking.mqe.rt.MQEVisitorBase;
import org.apache.skywalking.oap.server.core.query.mqe.ExpressionResult;
@@ -69,6 +74,7 @@ public class MQEVisitor extends MQEVisitorBase {
private MetricsQueryService metricsQueryService;
private AggregationQueryService aggregationQueryService;
private RecordQueryService recordQueryService;
+ private IModelManager modelManager;
public MQEVisitor(final ModuleManager moduleManager,
final Entity entity,
@@ -97,6 +103,15 @@ public class MQEVisitor extends MQEVisitorBase {
return aggregationQueryService;
}
+ private IModelManager getModelManager() {
+ if (modelManager == null) {
+ this.modelManager = moduleManager.find(CoreModule.NAME)
+ .provider()
+ .getService(IModelManager.class);
+ }
+ return modelManager;
+ }
+
private RecordQueryService getRecordQueryService() {
if (recordQueryService == null) {
this.recordQueryService = moduleManager.find(CoreModule.NAME)
@@ -220,11 +235,49 @@ public class MQEVisitor extends MQEVisitorBase {
}
}
+ /**
+ * Reject a top_n attribute condition whose key is not a queryable column
of the metric, before the
+ * request reaches storage. Attribute columns (attr0..attrN) exist only on
decorated OAL metrics and
+ * the MAL meter base; metrics such as relations or database/cache/mq
access carry none, so passing the
+ * condition to the storage engine surfaces a raw backend exception
instead of a query result. Failing
+ * here turns that into a descriptive MQE error via {@link
#getErrorResult(String)}.
+ *
+ * @param metricName the queried metric name
+ * @param attrConditions the attribute conditions parsed from the top_n
function
+ * @throws IllegalExpressionException if an attribute key is not a
queryable column of the metric
+ */
+ void checkAttributes(String metricName,
+ List<AttrCondition> attrConditions) throws
IllegalExpressionException {
+ if (attrConditions == null || attrConditions.isEmpty()) {
+ return;
+ }
+ Optional<Model> model = getModelManager().allModels().stream()
+ .filter(m ->
m.getName().equals(metricName))
+ .findFirst();
+ if (model.isEmpty()) {
+ // The metric existence is already validated via
ValueColumnMetadata; a missing model here is
+ // unexpected, so leave the condition untouched rather than reject
a possibly-valid query.
+ return;
+ }
+ Set<String> queryableColumns = model.get().getColumns().stream()
+ .filter(ModelColumn::shouldIndex)
+ .map(column ->
column.getColumnName().getName())
+ .collect(Collectors.toSet());
+ for (AttrCondition attr : attrConditions) {
+ if (!queryableColumns.contains(attr.getKey())) {
+ throw new IllegalExpressionException(
+ "The attribute [" + attr.getKey() + "] is not a queryable
column of metric [" + metricName
+ + "], so it can not be used as a top_n condition.");
+ }
+ }
+ }
+
private void querySortMetrics(String metricName,
int topN,
Order order,
List<AttrCondition> attrConditions,
- ExpressionResult result) throws IOException {
+ ExpressionResult result) throws IOException,
IllegalExpressionException {
+ checkAttributes(metricName, attrConditions);
TopNCondition topNCondition = new TopNCondition();
topNCondition.setName(metricName);
topNCondition.setTopN(topN);
diff --git
a/oap-server/server-query-plugin/query-graphql-plugin/src/test/java/org/apache/skywalking/oap/query/graphql/mqe/rt/MQEVisitorAttributeTest.java
b/oap-server/server-query-plugin/query-graphql-plugin/src/test/java/org/apache/skywalking/oap/query/graphql/mqe/rt/MQEVisitorAttributeTest.java
new file mode 100644
index 0000000000..f94b3f4004
--- /dev/null
+++
b/oap-server/server-query-plugin/query-graphql-plugin/src/test/java/org/apache/skywalking/oap/query/graphql/mqe/rt/MQEVisitorAttributeTest.java
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.oap.query.graphql.mqe.rt;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import org.apache.skywalking.mqe.rt.exception.IllegalExpressionException;
+import org.apache.skywalking.oap.server.core.CoreModule;
+import org.apache.skywalking.oap.server.core.query.enumeration.Step;
+import org.apache.skywalking.oap.server.core.query.input.AttrCondition;
+import org.apache.skywalking.oap.server.core.query.input.Duration;
+import org.apache.skywalking.oap.server.core.query.input.Entity;
+import org.apache.skywalking.oap.server.core.storage.model.ColumnName;
+import org.apache.skywalking.oap.server.core.storage.model.IModelManager;
+import org.apache.skywalking.oap.server.core.storage.model.Model;
+import org.apache.skywalking.oap.server.core.storage.model.ModelColumn;
+import org.apache.skywalking.oap.server.library.module.ModuleManager;
+import org.apache.skywalking.oap.server.library.module.ModuleProviderHolder;
+import org.apache.skywalking.oap.server.library.module.ModuleServiceHolder;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.mockito.junit.jupiter.MockitoSettings;
+import org.mockito.quality.Strictness;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Guards the top_n attribute pre-check in {@link MQEVisitor#checkAttributes}:
an attribute condition whose
+ * key is not a queryable column of the metric must be rejected with a
descriptive {@link
+ * IllegalExpressionException} before the request reaches storage, instead of
letting the backend surface a
+ * raw IO exception. Metrics with no attribute columns (relations,
database/cache/mq access) are the case
+ * that used to fail.
+ */
+@ExtendWith(MockitoExtension.class)
+@MockitoSettings(strictness = Strictness.LENIENT)
+public class MQEVisitorAttributeTest {
+ @Mock
+ private ModuleManager moduleManager;
+ @Mock
+ private IModelManager modelManager;
+
+ private MQEVisitor visitor;
+
+ @BeforeEach
+ public void setup() {
+ final ModuleProviderHolder providerHolder =
mock(ModuleProviderHolder.class);
+ final ModuleServiceHolder serviceHolder =
mock(ModuleServiceHolder.class);
+ when(moduleManager.find(CoreModule.NAME)).thenReturn(providerHolder);
+ when(providerHolder.provider()).thenReturn(serviceHolder);
+
when(serviceHolder.getService(IModelManager.class)).thenReturn(modelManager);
+ // Build the model mocks before stubbing allModels(); nesting when()
calls confuses Mockito.
+ final List<Model> models = Arrays.asList(
+ // endpoint_cpm is decorated, so its measure carries the attr0 tag.
+ modelWithColumns("endpoint_cpm", "entity_id", "service_id",
"attr0"),
+ // a relation metric carries no attribute columns at all.
+ modelWithColumns("service_relation_client_cpm", "entity_id",
"source_service_id", "dest_service_id"));
+ when(modelManager.allModels()).thenReturn(models);
+
+ final Duration duration = mock(Duration.class);
+ when(duration.getStep()).thenReturn(Step.MINUTE);
+ visitor = new MQEVisitor(moduleManager, mock(Entity.class), duration);
+ }
+
+ @Test
+ public void rejectsAttributeMissingFromMetricSchema() {
+ final IllegalExpressionException ex =
assertThrows(IllegalExpressionException.class, () ->
+ visitor.checkAttributes(
+ "service_relation_client_cpm",
+ Collections.singletonList(new AttrCondition("attr0",
"VIRTUAL_DATABASE", true))));
+ assertTrue(ex.getMessage().contains("attr0"));
+ assertTrue(ex.getMessage().contains("service_relation_client_cpm"));
+ }
+
+ @Test
+ public void rejectsNotEqualsAttributeMissingFromMetricSchema() {
+ // the same holds for `attr0 != x`, since the check is on the
attribute key.
+ assertThrows(IllegalExpressionException.class, () ->
+ visitor.checkAttributes(
+ "service_relation_client_cpm",
+ Collections.singletonList(new AttrCondition("attr0", "MESH",
false))));
+ }
+
+ @Test
+ public void allowsAttributePresentInMetricSchema() {
+ assertDoesNotThrow(() ->
+ visitor.checkAttributes(
+ "endpoint_cpm",
+ Collections.singletonList(new AttrCondition("attr0",
"GENERAL", true))));
+ }
+
+ @Test
+ public void allowsEmptyAttributes() {
+ assertDoesNotThrow(() ->
visitor.checkAttributes("service_relation_client_cpm",
Collections.emptyList()));
+ }
+
+ private static Model modelWithColumns(String name, String... columnNames) {
+ final Model model = mock(Model.class);
+ when(model.getName()).thenReturn(name);
+ final List<ModelColumn> columns = new ArrayList<>(columnNames.length);
+ for (final String columnName : columnNames) {
+ final ColumnName wrappedName = mock(ColumnName.class);
+ when(wrappedName.getName()).thenReturn(columnName);
+ final ModelColumn column = mock(ModelColumn.class);
+ when(column.getColumnName()).thenReturn(wrappedName);
+ when(column.shouldIndex()).thenReturn(true);
+ columns.add(column);
+ }
+ when(model.getColumns()).thenReturn(columns);
+ return model;
+ }
+}
