Skip to content

Dogfood Python SDK 3.0.0 alpha #91614

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 23 commits into from

Conversation

antonpirker
Copy link
Member

We will soon release a new major version of the Sentry Python SDK: 3.0.0.

This PR prepares the sentry code base to use the new major release and update the code to make it work with the new API.

We will then gradually deploy this to some nodes of Sentry and monitor if any problems arise.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label May 14, 2025
vgrozdanic and others added 2 commits May 14, 2025 14:07
PoC of monkey patching sentry_sdk in runtime
Copy link

codecov bot commented May 19, 2025

⚠️ Parser warning

The parser emitted a warning. Please review your JUnit XML file:

Warning while parsing testcase attributes: Limit of string is 1000 chars, for name, we got 2083 at 11446:140 in /home/runner/work/sentry/sentry/.artifacts/pytest.junit.xml

❌ 7623 Tests Failed:

Tests completed Failed Passed Skipped
25937 7623 18314 304
View the top 3 failed test(s) by shortest run time
tests.sentry.integrations.vsts.test_issues.VstsIssueFormTest::test_default_project_error_on_default_project
Stack Traces | 0.012s run time
#x1B[1m#x1B[31mtests/conftest.py#x1B[0m:146: in check_leaked_responses_mocks
    raise AssertionError(
#x1B[1m#x1B[31mE   AssertionError: `responses` were leaked outside of the test context:#x1B[0m
#x1B[1m#x1B[31mE   - <Response(url='https://fabrikam-fiber-inc.visualstudio..../workitemtypes/Bug/states' status=200 content_type='application/json' headers='null')>#x1B[0m
#x1B[1m#x1B[31mE   - <Response(url='https://fabrikam-fiber-inc.visualstudio.com/_apis/projects' status=200 content_type='application/json' headers='null')>#x1B[0m
#x1B[1m#x1B[31mE   (make sure to use `@responses.activate` or `with responses.mock:`)#x1B[0m
tests.sentry.integrations.vsts.test_issues.VstsIssueFormTest__InMonolithMode::test_get_create_issue_config_error_on_get_projects
Stack Traces | 0.012s run time
#x1B[1m#x1B[31mtests/conftest.py#x1B[0m:146: in check_leaked_responses_mocks
    raise AssertionError(
#x1B[1m#x1B[31mE   AssertionError: `responses` were leaked outside of the test context:#x1B[0m
#x1B[1m#x1B[31mE   - <Response(url='https://fabrikam-fiber-inc.visualstudio..../workitemtypes/Bug/states' status=200 content_type='application/json' headers='null')>#x1B[0m
#x1B[1m#x1B[31mE   - <Response(url='https://fabrikam-fiber-inc.visualstudio.com/_apis/projects' status=200 content_type='application/json' headers='null')>#x1B[0m
#x1B[1m#x1B[31mE   (make sure to use `@responses.activate` or `with responses.mock:`)#x1B[0m
tests.sentry.integrations.vsts.test_issues.VstsIssueFormTest__InMonolithMode::test_default_project_and_category
Stack Traces | 0.014s run time
#x1B[1m#x1B[31mtests/conftest.py#x1B[0m:146: in check_leaked_responses_mocks
    raise AssertionError(
#x1B[1m#x1B[31mE   AssertionError: `responses` were leaked outside of the test context:#x1B[0m
#x1B[1m#x1B[31mE   - <Response(url='https://fabrikam-fiber-inc.visualstudio..../workitemtypes/Bug/states' status=200 content_type='application/json' headers='null')>#x1B[0m
#x1B[1m#x1B[31mE   - <Response(url='https://fabrikam-fiber-inc.visualstudio.com/_apis/projects' status=200 content_type='application/json' headers='null')>#x1B[0m
#x1B[1m#x1B[31mE   (make sure to use `@responses.activate` or `with responses.mock:`)#x1B[0m

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

Copy link
Member

@asottile-sentry asottile-sentry left a comment

Choose a reason for hiding this comment

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

this seems like a really bad idea. why does this need special treatment? do we not expect s4s and canary to catch issues?

Comment on lines +747 to +748
if in_random_rollout("sentry-sdk.use-python-sdk-alpha"):
redirect_import("sentry_sdk", "sentry_sdk_alpha")
Copy link
Member

Choose a reason for hiding this comment

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

options can't / shouldn't be used at import time (as they require a database call)

Copy link
Member

Choose a reason for hiding this comment

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

not to mention they won't be reflected upon update -- as they will be read exactly once during startup and then never again

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, noted. I will try something.

@antonpirker antonpirker changed the title Dogfood python sdk 3.0.0 alpha Dogfood Python SDK 3.0.0 alpha May 20, 2025
@antonpirker
Copy link
Member Author

this seems like a really bad idea. why does this need special treatment? do we not expect s4s and canary to catch issues?

Because we use now OpenTelemetry under the hood it might happen that:

  • collected data (like span.tags or span.breadcrumbs and so on) is different to the old implementation in 2.x. (Our manual tests do not show this, but we just want to be extra sure)
  • Be have no data yet on cpu/mem usage change, this is why we want to release this gradually to catch those issue.

Can s4s or canary detect such issues?

@antonpirker
Copy link
Member Author

@asottile-sentry can you help to figure out a way to gradually roll out the alpha sdk? (we have to packages on pypi (sentry_sdk and sentry_sdk_alpha) so we can install both versions of the SDK in sentry. No we need to figure how to switch between the two at import time (based on an option or feature flag, or something else we can change without needing to do a full deploy.

@asottile-sentry
Copy link
Member

this seems like a really bad idea. why does this need special treatment? do we not expect s4s and canary to catch issues?

Because we use now OpenTelemetry under the hood it might happen that:

  • collected data (like span.tags or span.breadcrumbs and so on) is different to the old implementation in 2.x. (Our manual tests do not show this, but we just want to be extra sure)
  • Be have no data yet on cpu/mem usage change, this is why we want to release this gradually to catch those issue.

Can s4s or canary detect such issues?

I don't see why not -- should be pretty easy to see a cpu / memory regression (it'll likely result in a crashloop) -- as for tags I'd just manually look at a few during the soak period (~5 minutes) and if you're not happy pause and revert.

@asottile-sentry
Copy link
Member

this seems like a really bad idea. why does this need special treatment? do we not expect s4s and canary to catch issues?

Because we use now OpenTelemetry under the hood it might happen that:

  • collected data (like span.tags or span.breadcrumbs and so on) is different to the old implementation in 2.x. (Our manual tests do not show this, but we just want to be extra sure)
  • Be have no data yet on cpu/mem usage change, this is why we want to release this gradually to catch those issue.

Can s4s or canary detect such issues?

I don't see why not -- should be pretty easy to see a cpu / memory regression (it'll likely result in a crashloop) -- as for tags I'd just manually look at a few during the soak period (~5 minutes) and if you're not happy pause and revert.

we routinely upgrade packages, the interpreter, django, etc. without special treatment -- I don't see why the sdk would need extra special care comparatively

@antonpirker
Copy link
Member Author

Ok, thanks for all the input @asottile-sentry !

We will not move on with this and will do the regular deployment for testing the SDK alpha.

I will close this in favor of #92011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants