Skip to content

Conversation

antonpirker
Copy link
Contributor

@antonpirker antonpirker commented Aug 4, 2025

This allows users to set multiple span.data attributes at once.
In 3.x this then will become set_attributes (plural).

@antonpirker antonpirker requested a review from a team as a code owner August 4, 2025 14:51
cursor[bot]

This comment was marked as outdated.

Copy link
Member

@szokeasaurusrex szokeasaurusrex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This API is a bit confusing, imo, as it is unclear what should happen if a user calls set_data with a dict as the first parameter, and something else as the second parameter.

I would prefer we instead introduce a separate function to enable setting data as a dict, rather than supporting two distinct signatures for set_data.

Although if others are okay with having set_data support both ways, we can go with it – just sharing my two cents here. In that case, though, I would define both APIs with @overload.

Copy link

codecov bot commented Aug 4, 2025

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.07%. Comparing base (84adbb7) to head (dfe6c76).
⚠️ Report is 3 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
sentry_sdk/tracing.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@ ## master #4666 +/- ## ========================================== - Coverage 85.09% 85.07% -0.03%  ========================================== Files 156 156 Lines 15797 15801 +4 Branches 2671 2671 ========================================== Hits 13442 13442 - Misses 1578 1585 +7  + Partials 777 774 -3 
Files with missing lines Coverage Δ
sentry_sdk/tracing.py 86.36% <75.00%> (-0.10%) ⬇️

... and 4 files with indirect coverage changes

@antonpirker antonpirker changed the title Let set_data accept a dict. Add update_data to Span. Aug 4, 2025
@antonpirker
Copy link
Contributor Author

True @szokeasaurusrex i have now changed it to have a update_data() function instead. Similar to pythons own dict.update().

cursor[bot]

This comment was marked as outdated.

Copy link
Member

@szokeasaurusrex szokeasaurusrex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the Cursor bug bot might have found something. Besides that, looks good

Copy link
Member

@szokeasaurusrex szokeasaurusrex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks! (but pls see comment before merge)

@antonpirker antonpirker enabled auto-merge (squash) August 5, 2025 08:49
@antonpirker antonpirker merged commit 0d569d2 into master Aug 5, 2025
137 of 138 checks passed
@antonpirker antonpirker deleted the antonpirker/set-data-dict branch August 5, 2025 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants