You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "vatsrahul1001 (via GitHub)" <gi...@apache.org> on 2026/05/26 05:26:48 UTC

[PR] Fix Callback.handle_event crash on OTel metrics with dict tag values [airflow]

vatsrahul1001 opened a new pull request, #67527:
URL: https://github.com/apache/airflow/pull/67527

   ## Summary
   
   Triggerer crashes on the next deadline async callback when OpenTelemetry metrics are enabled:
   
   ```
   File ".../airflow/jobs/triggerer_job_runner.py", line 659, in handle_events
       Trigger.submit_event(...)
   File ".../airflow/models/callback.py", line 234, in handle_event
       Stats.incr(**self.get_metric_info(status, self.output))
   File ".../airflow/_shared/observability/metrics/otel_logger.py", line 211, in incr
       counter.add(count, attributes=tags)
   File ".../opentelemetry/sdk/metrics/.../view_instrument_match.py", line 105
       aggr_key = frozenset(attributes.items())
   TypeError: unhashable type: 'dict'
   ```
   
   ## Root cause
   
   `Callback.get_metric_info` builds the metric tags dict directly from the callback's `result` and `self.data` (which includes `kwargs`). Both are frequently dicts — for deadline async callbacks the `result` is the user callback's return value, and `kwargs` is the captured callback kwargs.
   
   When the metrics backend is OpenTelemetry, the SDK builds the aggregation key as `frozenset(attributes.items())`, which raises `TypeError: unhashable type: 'dict'` if any value is unhashable (dict, list, set). Result: triggerer crash, stalled triggers.
   
   The bug is metrics-backend-dependent: **statsd** accepts non-primitive tag values without complaint, so OSS users running the default statsd never see it. **OpenTelemetry** backends hit it consistently — surfaced by Astronomer Astro Cloud, but affects any OSS deployment that enables `[metrics] otel_*`.
   
   Reproduces against 3.2.1 and main — not a 3.2.x regression.
   
   ## Fix
   
   Sanitize tag values to primitives before returning from `get_metric_info`. Keep `str | int | float | bool | None` as-is; JSON-stringify anything else using `default=str` so values like `datetime` fall back cleanly instead of raising.
   
   ## Test plan
   
   - [X] Adds `test_get_metric_info_dict_values_are_stringified` — exercises the deadline-async-callback shape (dict `result` + dict `kwargs`), asserts every tag value is primitive, and verifies `frozenset(tags.items())` doesn't raise.
   - [X] Updates the existing `test_get_metric_info` assertion: `kwargs` is now JSON-stringified rather than left as a dict.
   - [ ] Manual repro on an OTel-enabled deployment — runs the deadline test DAG and confirms triggerer no longer crashes.
   
   ## Reported by
   
   Astronomer Runtime team while testing the 3.2.2rc2-based Runtime image on Astro Cloud stage env. Pre-existing in Runtime 3.2-4 (3.2.1-based).
   
   ---
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes — Claude Code (Opus 4.7)
   
   Generated-by: Claude Code (Opus 4.7) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix Callback.handle_event crash on OTel metrics with dict tag values [airflow]

Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #67527:
URL: https://github.com/apache/airflow/pull/67527#discussion_r3301555953


##########
airflow-core/tests/unit/models/test_callback.py:
##########
@@ -109,13 +109,38 @@ def test_get_metric_info(self):
         metric_info = callback.get_metric_info(CallbackState.SUCCESS, "0")
 
         assert metric_info["stat"] == "deadline_alerts.callback_success"
+        # kwargs is JSON-stringified so non-primitive tag values don't crash OTel
+        # (see test_get_metric_info_dict_values_are_stringified for the regression).

Review Comment:
   Cross ref adds no value, will be dead code if `test_get_metric_info_dict_values_are_stringified` is renamed.
   
   ```suggestion
   ```



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix Callback.handle_event crash on OTel metrics with dict tag values [airflow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #67527:
URL: https://github.com/apache/airflow/pull/67527#issuecomment-4541247628

   ### Backport failed to create: v3-2-test. View the failure log <a href='https://github.com/apache/airflow/actions/runs/26436615617'> Run details </a>
   
   Note: As of [Merging PRs targeted for Airflow 3.X](https://github.com/apache/airflow/blob/main/dev/README_AIRFLOW3_DEV.md#merging-prs-targeted-for-airflow-3x)
   the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.
   
   In matter of doubt please ask in [#release-management](https://apache-airflow.slack.com/archives/C03G9H97MM2) Slack channel.
   
   <table>
       <tr>
           <th>Status</th>
           <th>Branch</th>
           <th>Result</th>
       </tr>
       <tr>
           <td>❌</td>
           <td>v3-2-test</td>
           <td><a href="https://github.com/apache/airflow/commit/597891128e9a9deb82d121f471830914ed5cb049"><img src='https://img.shields.io/badge/Commit-5978911-red' alt='Commit Link'></a></td>
       </tr>
   </table>
   
   You can attempt to backport this manually by running:
   
   ```bash
   cherry_picker 5978911 v3-2-test
   ```
   
   This should apply the commit to the v3-2-test branch and leave the commit in conflict state marking
   the files that need manual conflict resolution.
   
   After you have resolved the conflicts, you can continue the backport process by running:
   
   ```bash
   cherry_picker --continue
   ```
   
   If you don't have cherry-picker installed, see the [installation guide](https://github.com/apache/airflow/blob/main/dev/README.md#how-to-backport-pr-with-cherry-picker-cli).
   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix Callback.handle_event crash on OTel metrics with dict tag values [airflow]

Posted by "vatsrahul1001 (via GitHub)" <gi...@apache.org>.
vatsrahul1001 merged PR #67527:
URL: https://github.com/apache/airflow/pull/67527


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix Callback.handle_event crash on OTel metrics with dict tag values [airflow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #67527:
URL: https://github.com/apache/airflow/pull/67527#issuecomment-4541245148

   Hi maintainer, this PR was merged without a milestone set.
   We've automatically set the milestone to **[Airflow 3.2.3](https://github.com/apache/airflow/milestone/140)** based on: backport label targeting v3-2-test
   If this milestone is not correct, please update it to the appropriate milestone.
   
   > This comment was generated by [Milestone Tag Assistant](https://github.com/apache/airflow/blob/main/.github/workflows/milestone-tag-assistant.yml).
   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix Callback.handle_event crash on OTel metrics with dict tag values [airflow]

Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #67527:
URL: https://github.com/apache/airflow/pull/67527#discussion_r3301548857


##########
airflow-core/src/airflow/models/callback.py:
##########
@@ -161,6 +162,17 @@ def get_metric_info(self, status: CallbackState, result: Any) -> dict:
             # Remove the context (if exists) to keep the tags simple
             tags["kwargs"] = {k: v for k, v in tags["kwargs"].items() if k != "context"}
 
+        # Metric backends (statsd, OpenTelemetry) require tag values to be primitives.
+        # OTel's aggregation key is built via ``frozenset(attributes.items())``, which
+        # raises ``TypeError: unhashable type: 'dict'`` if a value is a dict/list. The
+        # callback's ``result`` (passed in from a user callback) and ``kwargs`` are both
+        # frequently dicts, so coerce any non-primitive tag value to a JSON string before
+        # returning. Using ``default=str`` so values like ``datetime`` fall back cleanly.
+        tags = {
+            k: v if isinstance(v, (str, int, float, bool)) or v is None else json.dumps(v, default=str)

Review Comment:
   Probably want to add `sort_keys=True` to have deterministic output?



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org