1fanwang commented on PR #71561:
URL: https://github.com/apache/airflow/pull/71561#issuecomment-5284468462

   Closing this — the collision it fixes is already solvable with standard OTel 
configuration, so the extra metrics aren't earning their place.
   
   Airflow builds the meter provider with `Resource.create()` 
([otel_logger.py:465](https://github.com/apache/airflow/blob/main/shared/observability/src/airflow_shared/observability/metrics/otel_logger.py#L465)),
 which merges `OTEL_RESOURCE_ATTRIBUTES`. Give each scheduler a distinct 
`service.instance.id` and the per-scheduler samples become separate series, so 
nothing is overwritten:
   
   ```
   airflow.pool.open_slots{service.instance.id=scheduler-a, 
pool_name=default_pool} = 128
   airflow.pool.open_slots{service.instance.id=scheduler-b, 
pool_name=default_pool} = 126
   ```
   
   `min by (pool_name)` then recovers the same 126 the histogram was there to 
provide. `service.instance.id` is also what the spec maps to Prometheus' 
`instance` label, so backends already understand it — and it covers every gauge 
Airflow emits, not the five pool ones.
   
   I also had the cardinality argument backwards in the description above: 
histograms here use exponential bucket aggregation, so they cost considerably 
more stored series than N per-instance gauges, not fewer. That was the main 
justification for preferring a histogram over a per-scheduler dimension, and it 
doesn't hold.
   
   What's actually missing is documentation. Nothing in the metrics docs tells 
an HA operator that every scheduler publishes the same series, or how to 
separate them. Sending a docs PR instead.


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