wu-sheng opened a new pull request, #13940:
URL: https://github.com/apache/skywalking/pull/13940
### Fix `top_n(metric, N, order, attrX='value')` returning a raw storage
`IOException` when the attribute is not a column of the metric
- [x] Add a unit test to verify that the fix works.
- [x] 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).
- [x] Update the [`CHANGES`
log](https://github.com/apache/skywalking/blob/master/docs/en/changes/changes.md).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_017caW5gdjPCYqWprgA6csjJ
--
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]
