fix(aiokafka): preserve partition=0 / offset=0 on consumer spans#18342
fix(aiokafka): preserve partition=0 / offset=0 on consumer spans#18342piochelepiotr wants to merge 5 commits into
Conversation
Codeowners resolved as |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2df60973c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| span.set_tag(MESSAGE_KEY, message_key) | ||
|
|
||
| span._set_attribute(PARTITION, message.partition or -1) | ||
| span._set_attribute(PARTITION, message.partition if message.partition is not None else -1) |
There was a problem hiding this comment.
This change makes single-partition getone() consumer spans emit kafka.partition=0 and first-message spans emit kafka.message_offset=0, but the existing aiokafka snapshot fixtures still expect -1 for those metrics. I checked tests/contrib/aiokafka/utils.py, where create_topic() creates NewTopic(..., 1, 1), and the test_send_and_wait_* / test_send_commit snapshots still contain "kafka.partition": -1.0 and "kafka.message_offset": -1.0, so those snapshot tests will fail unless the fixtures are regenerated or updated alongside this code change.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in df57c98 — regenerated all aiokafka snapshots so they now expect kafka.partition: 0.0 and kafka.message_offset: 0.0 for single-partition / first-message consume spans.
|
BenchmarksBenchmark execution time: 2026-06-11 21:57:59 Comparing candidate commit a20daf5 in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 384 metrics, 9 unstable metrics. scenario:span-start
|
piochelepiotr
force-pushed
the
fix/aiokafka-falsy-partition-offset
branch
2 times, most recently
from
16e93ff to
4006398
Compare
brettlangdon
left a comment
brettlangdon
left a comment
There was a problem hiding this comment.
just a question, but otherwise lgtm
The aiokafka getone handler reported `kafka.partition` and `kafka.message_offset` as -1 whenever the real value was 0, because it used `value or -1` which treats the integer 0 as falsy. Single-partition topics and the first message in any partition were therefore tagged with -1. Replace with explicit None checks so that legitimate zero values pass through unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…of using -1 Addresses reviewer feedback: -1 as a sentinel for unknown partition or offset provides no real value and is misleading. Producer spans (where partition is not yet known before broker assignment) and any span with a genuinely absent value now omit these tags entirely. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
piochelepiotr
force-pushed
the
fix/aiokafka-falsy-partition-offset
branch
from
e39743c to
0220612
Compare
…fset values After rebasing, consumer span snapshots were reset to main's state which still had the stale -1.0 sentinel values. The code correctly produces 0.0 (first message, single partition), so update snapshots to match. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
6 participants
Summary
_on_aiokafka_getone_messagesetkafka.partitionandkafka.message_offsetviavalue or -1, which collapses a legitimate0to-1. Single-partition topics (or the first message in any partition) were therefore consistently tagged with-1.is not Nonechecks so 0 passes through unchanged.Test plan
kafka.partition=-1/kafka.message_offset=-1despite real values being0.🤖 Generated with Claude Code