Repository navigation
fix: drop None attributes from OTel span events - #190
Merged
Merged
Conversation
OpenTelemetry SDKs before 1.45 reject None attribute values with a warning, so a retry.giving_up after a raised NetworkError logged a warning and lost last_status. The span event now leaves out None values; the log record keeps them. Closes #171
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
_emit_eventnow leaves outNone-valued attributes from the OTel span event. The log record still gets every attribute,Noneincluded.Why
OpenTelemetry SDKs before 1.45 reject
Noneattribute values. I reproduced it with an in-memory exporter, a recording span, and a syncRetry(max_attempts=2)that gives up on a raisedConnectErrorthroughMockTransport. On SDK 1.20.0, 1.38.0 and 1.44.0 every give-up loggedand the recorded
retry.giving_upevent had nolast_status. SDK 1.45.1 acceptsNoneand keeps it without a warning. Our floor isopentelemetry-api>=1.20, so most of the supported range is affected. With this change, every SDK version records the same event (without theNonekeys) and none of them log the warning. On 1.45+ that meanslast_status: Noneno longer shows up on the span event, so a missing key now means "no value".Events that can carry
None:retry.giving_up(reason="max_attempts"):last_statuswhen the last attempt raised instead of returning a response.retry.budget_refused:last_status, same case.circuit.rejected:retry_afterwhen the circuit is half-open with a probe in flight.bulkhead.rejecteddeclaresacquire_timeout: float | None, but it is only emitted when a bounded acquire times out, so it is neverNonein practice. Theretry_after_exceeds_max_delaygive-up always has a response.The test follows the existing mock-span pattern in
tests/test_observability.py, so noopentelemetry-sdkdev dependency was added.Performance
_emit_eventruns only when a resilience event fires, not on every request. The filter sits inside the OTel branch, so log-only installs don't pay for it. A dict comprehension over a six-key attribute dict costs about 0.6 to 0.7 us. Timing the whole_emit_eventagainst a recording SDK span came out at 28 to 44 us per call before and after, and the run-to-run noise was bigger than the difference. On SDK 1.44 theNonecase got faster, from 42 to 54 us down to about 30 us, because the SDK warning is no longer logged.Closes #171