fix: Ensure tags values are strings #4459

szokeasaurusrex · 2025-06-12T11:47:53Z

Ensure tag values are strings before serializing an event or a transaction to an Event dictionary.

Fixes #4391

codecov · 2025-06-12T11:49:07Z

❌ Unsupported file format

Upload processing failed due to unsupported file format. Please review the parser error message:

Error deserializing json

Caused by: expected value at line 1 column 1

For more help, visit our troubleshooting guide.

❌ 476 Tests Failed:

Tests completed	Failed	Passed	Skipped
19119	476	18643	4090

View the top 3 failed test(s) by shortest run time

tests.test_api::test_init_context_manager_deprecation

Stack Traces | 0.001s run time

tests/test_api.py:229: in test_init_context_manager_deprecation
    with sentry_sdk.init():
E   NameError: name 'sentry_sdk' is not defined

tests.test_api::test_push_scope_deprecation

Stack Traces | 0.001s run time

tests/test_api.py:223: in test_push_scope_deprecation
    with push_scope():
         ^^^^^^^^^^
E   NameError: name 'push_scope' is not defined

During handling of the above exception, another exception occurred:
tests/test_api.py:222: in test_push_scope_deprecation
    with pytest.warns(DeprecationWarning):
E   Failed: DID NOT WARN. No warnings of type (<class 'DeprecationWarning'>,) were emitted.
E    Emitted warnings: [].

tests.test_api::test_push_scope_deprecation

Stack Traces | 0.001s run time

tests/test_api.py:223: in test_push_scope_deprecation
    with push_scope():
E   NameError: name 'push_scope' is not defined

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

szokeasaurusrex · 2025-06-13T12:46:35Z

Looks like tests are failing because the non-string values were being asserted in the test suites

sentrivana

Some high-level thoughts:

We seem to be stringifying the tags in multiple places. Do we need to do it in all of those? Is there a way to centralize it, e.g. move it to the end of the pipeline, e.g. in the serializer?
Are we sure we're not changing anything user-facing, especially in callbacks? (E.g. if a user is accessing tags in a before_send or similar.)

sentrivana · 2025-06-17T09:28:43Z

tests/test_scope.py

+def test_set_tag_handles_conversion_failure():
+    """Test that set_tag handles objects that fail to convert to string."""
+    scope = Scope()
+
+    # Create an object that raises an exception when str() is called
+    class BadObject:
+        def __str__(self):
+            raise Exception("Cannot convert to string")
+
+        def __repr__(self):
+            return "BadObject()"
+
+    bad_obj = BadObject()
+
+    # This should not raise an exception
+    scope.set_tag("bad_object", bad_obj)
+
+    # The tag should be set with the repr value
+    event = scope.apply_to_event({}, {})
+    tags = event.get("tags", {})
+
+    assert tags["bad_object"] == "BadObject()", "Tag should be set with repr value"
+
+
+def test_set_tags_handles_conversion_failure():
+    """Test that set_tags handles objects that fail to convert to string."""
+    scope = Scope()
+
+    # Create an object that raises an exception when str() is called
+    class BadObject:
+        def __str__(self):
+            raise Exception("Cannot convert to string")
+
+        def __repr__(self):
+            return "BadObject()"
+
+    bad_obj = BadObject()
+
+    scope.set_tags(
+        {
+            "good_tag1": "value1",
+            "bad_tag": bad_obj,
+            "good_tag2": 123,
+        }
+    )
+
+    event = scope.apply_to_event({}, {})
+    tags = event.get("tags", {})
+
+    assert tags["good_tag1"] == "value1"
+    assert tags["bad_tag"] == "BadObject()", "Tag should be set with repr value"
+    assert tags["good_tag2"] == "123"


Maybe consolidate these two test cases into one?

szokeasaurusrex · 2025-06-17T13:29:52Z

We discussed offline, but here's the summary:

Yeah, I believe we need to stringify in all of these places, as the conversion to Event is different for tags set on scope, tags set on spans, and tags set on transactions. Conversion in the serializer would not be feasible, as the serializer is unaware of the event schema, and would not know which tags need to be stringified
Unfortunately, this would break a before_send if the user was checking some non-string value. However, since the Event type passed to before_send already declares the tags as MutableMapping[str, str], the code as we have it right now is already violating this typing, so one could argue that this is a bugfix to align ourselves with the declared types

sentrivana · 2025-06-18T08:43:08Z

Re: 2: Thought about this some more. The thing about expected behavior is that it doesn't really matter if it's technically incorrect -- users come to expect it and depend on it, and we need to take that into account. Especially in this case as the event is technically part of the public API in before_send.

Another reason I'm reluctant to do this in a minor version is that in general it's hard to notice errors in a before_send. Things might just silently stop working and that's really bad UX. (There's already been a handful of issues that turned out to be problems in people's before_send that were close to impossible to diagnose as such from user point of view.)

What we can do:

Fix this on the potel-base branch so that it goes out in a major. 3.0 is hopefully just a few weeks away so people wouldn't have to wait for the fix long.
Find a way to push the serialization after before_send so that the tags are still accessible as-is in the callback, and only serialized later. Probably hacky.
Introduce a deprecation period before we make the actual change. Emit a deprecation warning if we detect a non-string tag is set. This is not great because the SDK itself is setting non-string tags and those deprecation warnings would not be actionable.

Doing this on potel-base is our best bet here. The issue has been there for years and as I understand it we've only had someone encounter it recently, so fixing this in 3.0 should be sufficient.

szokeasaurusrex · 2025-06-25T12:59:10Z

@sentrivana I guess then there's also the question of whether we should fix this at all. The use case is pretty edge-case; if your premise is correct that folks are relying on the "incorrectly typed" tag values, then making the change in the major would also be disruptive, right?

What do you think?

sentrivana · 2025-06-25T14:27:21Z

Some disruption is to be expected when upgrading to a new major version. As long as we document this properly (changelog, migration guide in repo, migration guide in docs) we're good.

Ensure tag values are strings before serializing an event or a transaction to an Event dictionary. Fixes #4391

szokeasaurusrex · 2025-06-27T12:30:01Z

@sentrivana I rebased on potel-base

szokeasaurusrex self-assigned this Jun 12, 2025

szokeasaurusrex marked this pull request as ready for review June 12, 2025 12:06

szokeasaurusrex requested a review from a team as a code owner June 12, 2025 12:06

szokeasaurusrex marked this pull request as draft June 13, 2025 12:47

szokeasaurusrex marked this pull request as ready for review June 16, 2025 13:45

sentrivana reviewed Jun 17, 2025

View reviewed changes

szokeasaurusrex marked this pull request as draft June 27, 2025 08:29

szokeasaurusrex force-pushed the cursor/prepare-pr-for-tag-value-bug-fix-81d2 branch from 16045df to 4721438 Compare June 27, 2025 11:50

szokeasaurusrex changed the base branch from master to potel-base June 27, 2025 11:50

szokeasaurusrex force-pushed the cursor/prepare-pr-for-tag-value-bug-fix-81d2 branch 2 times, most recently from a78a060 to 28eaca8 Compare June 27, 2025 12:28

fix: Ensure tags values are strings

bce0694

Ensure tag values are strings before serializing an event or a transaction to an Event dictionary. Fixes #4391

szokeasaurusrex force-pushed the cursor/prepare-pr-for-tag-value-bug-fix-81d2 branch from 28eaca8 to bce0694 Compare June 27, 2025 12:29

szokeasaurusrex marked this pull request as ready for review June 27, 2025 12:29

szokeasaurusrex requested a review from sentrivana June 27, 2025 12:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: Ensure tags values are strings #4459

fix: Ensure tags values are strings #4459

szokeasaurusrex commented Jun 12, 2025 •

edited

Loading

Uh oh!

codecov bot commented Jun 12, 2025 •

edited

Loading

Uh oh!

szokeasaurusrex commented Jun 13, 2025

Uh oh!

sentrivana left a comment

Uh oh!

sentrivana Jun 17, 2025

Uh oh!

szokeasaurusrex commented Jun 17, 2025

Uh oh!

sentrivana commented Jun 18, 2025 •

edited

Loading

Uh oh!

szokeasaurusrex commented Jun 25, 2025

Uh oh!

sentrivana commented Jun 25, 2025 •

edited

Loading

Uh oh!

szokeasaurusrex commented Jun 27, 2025

Uh oh!

Uh oh!

fix: Ensure tags values are strings #4459

Are you sure you want to change the base?

fix: Ensure tags values are strings #4459

Conversation

szokeasaurusrex commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ Unsupported file format

❌ 476 Tests Failed:

Uh oh!

szokeasaurusrex commented Jun 13, 2025

Uh oh!

sentrivana left a comment

Choose a reason for hiding this comment

Uh oh!

sentrivana Jun 17, 2025

Choose a reason for hiding this comment

Uh oh!

szokeasaurusrex commented Jun 17, 2025

Uh oh!

sentrivana commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

szokeasaurusrex commented Jun 25, 2025

Uh oh!

sentrivana commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

szokeasaurusrex commented Jun 27, 2025

Uh oh!

Uh oh!

szokeasaurusrex commented Jun 12, 2025 •

edited

Loading

codecov bot commented Jun 12, 2025 •

edited

Loading

sentrivana commented Jun 18, 2025 •

edited

Loading

sentrivana commented Jun 25, 2025 •

edited

Loading