wu-sheng commented on PR #13891:
URL: https://github.com/apache/skywalking/pull/13891#issuecomment-4627811482

   **Question: do the 7 `pool_*` e2e checks actually pass locally?**
   
   The description lists "mock e2e (30/30)" and "cluster e2e (26/26)", but I 
think the `pool_*` checks can't match their expected file as written — and I'd 
like to confirm I'm not missing something about the setup before CI runs.
   
   The pool rules keep `pool_name` as a label that is **not** an entity 
dimension:
   
   - `otel-rules/airflow/airflow-service.yaml`: 
`airflow_pool_queued_slots.sum(['cluster', 'pool_name'])` with scope 
`.service(['cluster'], Layer.AIRFLOW)`
   - `otel-rules/airflow/airflow-instance.yaml`: 
`airflow_pool_open_slots.sum(['cluster', 'host_name', 'pool_name'])` with scope 
`.instance(['cluster'], ['host_name'], Layer.AIRFLOW)`
   
   So `pool_name` survives as a **metric label**, and MQE returns 
`metric.labels: [{key: pool_name, value: default_pool}]`. But all 7 pool 
queries (3 service + 4 instance) in **both** `airflow-cases.yaml` and 
`airflow-cluster-cases.yaml` point at `expected/metrics-has-value.yml`, whose 
template asserts `metric.labels: []`.
   
   To check this rather than guess, I ran `skywalking-infra-e2e`'s own 
`verifier.Verify()` against synthetic swctl output:
   
   | Expected template | Actual result | Verdict |
   |---|---|---|
   | generic (`labels: []`) | labeled (`pool_name=default_pool`) | **MISMATCH → 
FAIL** |
   | generic (`labels: []`) | unlabeled | MATCH |
   | label-asserting (`- key: pool_name`) | labeled (`pool_name=default_pool`) 
| MATCH |
   
   This is the same single-vs-labeled split other layers already handle with a 
dedicated template — e.g. ActiveMQ 
`metrics-has-value-label-serviceinstanceid.yml`, ClickHouse 
`metrics-has-value-label.yml`, Flink 
`metrics-has-value-jobManager-node-label.yml`.
   
   Questions:
   
   1. Did the `meter_airflow_pool_*` / `meter_airflow_instance_pool_*` cases 
pass in your local `e2e run`? If so, could you share the `swctl metrics exec` 
output for one of them — does it carry a `pool_name` label?
   2. If they do carry the label, would you add an 
`expected/metrics-has-value-label-poolname.yml` (asserting `- key: pool_name` / 
`value: {{ notEmpty .value }}`) and repoint the 7 pool queries in both case 
files? Keeping `pool_name` in `.sum(...)` preserves the per-pool breakdown, so 
I'd suggest the label-template route rather than dropping the label.
   
   Happy to be corrected if there's something in the setup I've overlooked.
   


-- 
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]

Reply via email to