Fix MQE top_n raising a raw storage IO exception for an invalid attribute condition

[wu-sheng]

Fix top_n(metric, N, order, attrX='value') returning a raw storage IOException when the attribute is not a column of the metric

• Add a unit test to verify that the fix works.
• Explain briefly why the bug exists and how to fix it.

Why the bug exists. An MQE top_n(...) query accepts attribute conditions (e.g. attr0='VIRTUAL_DATABASE', attr0!='MESH'). These attr0..attrN columns exist only on decorated metrics — service_* / endpoint_* / kubernetes_service_*, where OAL .decorator("ServiceDecorator"/"EndpointDecorator"/"K8SServiceDecorator") sets attr0 to the entity's layer — plus the MAL Meter base class. Metrics whose source declares no attribute columns (service/instance/endpoint relations, and database_access_* / cache_* / mq_* virtual-access metrics) carry no attr0 tag at all.

When such an attribute condition is passed for one of those metrics, nothing validated it before the storage call: MQEVisitor.querySortMetrics → AggregationQueryService.sortMetrics → the storage DAO builds a query filtering on a tag the measure does not define, and the backend fails. On BanyanDB the BanyanDBException is wrapped as IOException, which MQEVisitor flattens into the opaque, non-actionable "Internal IO exception, query metrics error." — indistinguishable from a genuine I/O fault. A well-formed MQE should never surface a storage I/O exception.

How it's fixed. MQEVisitor.checkAttributes(...) now runs before the storage call and validates each attribute key against the metric's registered queryable columns (via IModelManager). If the attribute is not a column of the metric it raises IllegalExpressionException naming the attribute and the metric, which the existing catch in visitMetric turns into a descriptive ExpressionResult error (the same graceful path already used for "metric does not exist", "TopN value must be > 0", etc.). Decorated metrics and empty attribute lists are unaffected; the check is on the attribute key, so = and != conditions are both covered.

The guard lives only in MQEVisitor because MQE is the sole caller that populates TopNCondition.attributes (PromQL's buildTopNCondition never sets them; the GraphQL TopNCondition input does not expose attributes).

Unit test: MQEVisitorAttributeTest (4 cases — reject =/!= on a no-attr metric, allow a valid attribute, allow empty attributes).

• Update the CHANGES log.

[Copilot]

Pull request overview

This pull request adds an MQE-side validation guard for top_n(..., attrX='value') so invalid attribute conditions (attributes not present as queryable columns on the target metric) fail fast with a descriptive MQE error rather than propagating a backend IOException.

Changes:

• Add MQEVisitor.checkAttributes(...) and invoke it before executing top_n storage queries to validate attribute keys against the metric model schema.
• Add unit tests covering rejecting =/!= for no-attribute metrics, allowing valid attributes, and allowing empty attribute lists.
• Document the fix in the CHANGES log.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/mqe/rt/MQEVisitor.java	Adds schema-based attribute validation prior to top_n storage execution to prevent raw backend IO errors.
oap-server/server-query-plugin/query-graphql-plugin/src/test/java/org/apache/skywalking/oap/query/graphql/mqe/rt/MQEVisitorAttributeTest.java	Adds unit tests verifying the new attribute validation behavior.
docs/en/changes/changes.md	Records the MQE top_n attribute validation fix in release notes.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.