Skip to content

chore(aci): manually add spans for delayed workflow processing #91908

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 1 commit into from
May 19, 2025

Conversation

cathteng
Copy link
Member

Like for delayed rule processing #91764, we should track the performance of delayed workflow processing. We need to manually add spans though.

@cathteng cathteng requested a review from a team as a code owner May 19, 2025 22:20
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label May 19, 2025
Copy link
Contributor

@saponifi3d saponifi3d left a comment

Choose a reason for hiding this comment

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

generally lgtm 🎉

Comment on lines +517 to +529
project = fetch_project(project_id)
if not project:
return

workflow_event_dcg_data = fetch_group_to_event_data(project_id, Workflow, batch_key)
workflow_event_dcg_data = fetch_group_to_event_data(project_id, Workflow, batch_key)

# Get mappings from DataConditionGroups to other info
dcg_to_groups, trigger_group_to_dcg_model = get_dcg_group_workflow_detector_data(
workflow_event_dcg_data
)
dcg_to_workflow = trigger_group_to_dcg_model[DataConditionHandler.Group.WORKFLOW_TRIGGER].copy()
dcg_to_workflow.update(trigger_group_to_dcg_model[DataConditionHandler.Group.ACTION_FILTER])
# Get mappings from DataConditionGroups to other info
dcg_to_groups, trigger_group_to_dcg_model = get_dcg_group_workflow_detector_data(
workflow_event_dcg_data
)
dcg_to_workflow = trigger_group_to_dcg_model[
DataConditionHandler.Group.WORKFLOW_TRIGGER
].copy()
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should this be moved into a method to make it a little easier to read? perhaps naming it prepare_data? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

hmm yeah i can

Copy link
Member Author

Choose a reason for hiding this comment

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

err well there's the early return if there's no project

Copy link
Member Author

Choose a reason for hiding this comment

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

the return type starts looking a little crazy if i put it all into a single function, so just going to keep it separate for now

Copy link

codecov bot commented May 19, 2025

Codecov Report

Attention: Patch coverage is 4.16667% with 23 lines in your changes missing coverage. Please review.

⚠️ 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 1:157237 in /home/runner/work/sentry/sentry/.artifacts/pytest.junit.xml
Files with missing lines Patch % Lines
...try/workflow_engine/processors/delayed_workflow.py 4.16% 23 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #91908      +/-   ##
==========================================
- Coverage   87.63%   87.62%   -0.01%     
==========================================
  Files       10356    10356              
  Lines      587133   587141       +8     
  Branches    22585    22585              
==========================================
+ Hits       514506   514510       +4     
- Misses      72199    72203       +4     
  Partials      428      428              

@cathteng cathteng merged commit 07e915f into master May 19, 2025
61 checks passed
@cathteng cathteng deleted the cathy/aci/instrument-delayed-workflow branch May 19, 2025 22:41
jan-auer added a commit that referenced this pull request May 20, 2025
* master: (58 commits)
  link: cleanup link (#91687)
  ref: create project_id index for organizationonboardingtask (#91918)
  storybook: smaller last edited (#91875)
  issues: fix chonk stacktrace alignment (#91891)
  alert: drop custom alert (#91892)
  insights: fix bar height (#91895)
  ref(span-buffer): Move max-memory-percentage to right CLI (#91924)
  ref(js): Factor button functionality (#91763)
  tests(resolve_groups): Clean up the tests (#91779)
  ref(span-buffer): Add backpressure (#91707)
  fix(nextjs-insights): project id is not passed to explore link (#91920)
  fix(crons): Floor seconds / microsecond on recorded dateClock (#91890)
  fix(uptime): Fix bug with the uptime_checks dataset in the events endpoint (#91824)
  ref: add state-only migration to reflect existing indexes in prod (#91901)
  ref: remove unnecssary metaclass (#91906)
  fix(stats): use data category title name (#91913)
  feat(issues): Add success messages to some actions (#91899)
  test(taskworker): Lower exec time (#91907)
  chore(aci): manually add spans for delayed workflow processing (#91908)
  chore(aci): remove uses of WorkflowFireHistory rollout columns (#91904)
  ...
Copy link

sentry-io bot commented May 30, 2025

Suspect Issues

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

  • ‼️ KeyError: 6078021132 sentry.workflow_engine.processors.delayed_workflow View Issue

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

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.

2 participants