Skip to content

ref(event_manager): Inline signals into _record_transaction_info #86604

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

Merged
merged 6 commits into from
Mar 10, 2025

Conversation

jan-auer
Copy link
Member

@jan-auer jan-auer commented Mar 7, 2025

Centralizes all transaction-related side effects in _record_transaction_info
so it can be moved into the new spans consumer. This follows up to prior work by
@JoshFerge.

  • Two signals were emitted directly in event_manager.save. The underlying
    functions are now called directly and the calls have been moved in.
  • We no longer connect the record_* functions that were called directly to
    their old signals.

Ref https://github.com/getsentry/streaming-planning/issues/67

@jan-auer jan-auer requested a review from a team as a code owner March 7, 2025 16:04
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 7, 2025
@jan-auer jan-auer requested review from untitaker and JoshFerge March 7, 2025 16:05
Copy link

codecov bot commented Mar 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #86604       +/-   ##
===========================================
+ Coverage   45.37%   87.80%   +42.42%     
===========================================
  Files        9759     9774       +15     
  Lines      552916   553326      +410     
  Branches    21693    21576      -117     
===========================================
+ Hits       250889   485822   +234933     
+ Misses     301632    67133   -234499     
+ Partials      395      371       -24     

@jan-auer
Copy link
Member Author

Turns out we do need FeatureAdoption outside of the Sentry codebase; I'm going to restore the function now.

* master: (57 commits)
  ref(getting-started): Change code to always update loader script when products changes (#86583)
  ref(spans): Detect performance issues directly in segments consumer (#86595)
  ref(getting-started): Update copies, replacing 'Sentry dashboard' with 'Sentry Issues' (#86672)
  ref(product-selection): Update code to not strip url (#86582)
  ref(quick-start): Replace 'record' with 'create_or_update' in 'record_new_project' (#86663)
  ref(assemble): Remove old `find_missing_chunks` method (#86588)
  fix(views):Exclude newly added views from last visited update as well (#86653)
  feat(workflow_engine): Only execute enabled Detectors (#86652)
  fix(autofix): Github links, remove seer branding, and UI cleanup (#86640)
  fix(seer-issues-patch) More parsing of functions for Python (#86558)
  ref(ui): Remove sentry.eot (#86649)
  feat(alerts): Restrict uptime/crons overview buttons for alerts:write (#86436)
  chore(deps): bump axios from 1.7.7 to 1.8.2 (#86642)
  deps(ui): Upgrade prettier (#86634)
  deps(ui): Upgrade eslint, biome (#86630)
  fix(shared-views): Fix default view passing to last visited endpoint (#86632)
  feat(billing): update copy for payg disabled CTA (#86143)
  ref(ui): Remove usage of withOrganization from organizationAuthList (#86554)
  fix(crons): Fix disabled state of disable button (#86637)
  feat(ui): Replace OrganizationAuth DeprecatedAsyncComponent (#86556)
  ...
@jan-auer jan-auer merged commit f30e2e0 into master Mar 10, 2025
49 checks passed
@jan-auer jan-auer deleted the ref/em-simplify-signals branch March 10, 2025 14:02
Copy link

sentry-io bot commented Mar 10, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ AttributeError: 'NoneType' object has no attribute 'keys' sentry.tasks.post_process.post_process_group View Issue

Did you find this useful? React with a 👍 or 👎

DominikB2014 pushed a commit that referenced this pull request Mar 14, 2025
…pan (#87123)

Fixes a regression introduced in #86604 that resulted in a captured
`TypeError` when recording the first insight span for a project.

We switched from using signals, which are prohibited in
`EventManager.save`, to directly calling the onboarding receiver
function. In this specific instance, we missed that the function was
decorated, and the import resulted in `None`. For an unknown reason, the
resulting `TypeError` did not show up in Sentry.

Projects that have not recorded these flags since the regression was
rolled out will be flagged when they ingest the next span of the
corresponding insights module.
@github-actions github-actions bot locked and limited conversation to collaborators Mar 26, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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