Skip to content

chore(ACI): Add 1:1 metrics for dual processing #91840

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ceorourke
Copy link
Member

In order for us to determine whether we are firing metric alerts at the same rate in workflow engine, add org id to the existing metric alert fire/resolve metrics and add a new metric to process workflows that doesn't count the number of actions being fired. We'll have to be careful to not add too many orgs to this flag as that'd result in a high cardinality of tags in Datadog.

@ceorourke ceorourke requested review from a team as code owners May 16, 2025 21:40
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label May 16, 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.

might be nice to include a test for these as well 😅

Comment on lines 315 to 323
if features.has("organizations:workflow-engine-metric-alert-dual-processing-logs"):
# in order to check if workflow engine is firing 1:1 with the old system, we must only count once rather than each action
metrics.incr(
"workflow_engine.process_workflows.fired_actions",
tags={
"detector_type": detector.type,
"organization_id": detector.project.organization_id,
},
)
Copy link
Contributor

@saponifi3d saponifi3d May 16, 2025

Choose a reason for hiding this comment

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

thoughts on pulling out the org_id and setting the tag on every metric? something like:

organization_id = None
if features.has("organizations:workflow-engine-metric-alert-dual-processing-logs"):
    organization_id = detector.project.organization_id
    
metrics.incr(
    "workflow_engine.process_workflows.triggered_actions",
    amount=len(actions),
    tags={"detector_type": detector.type, "organization_id": organization_id},
)

Copy link
Member Author

Choose a reason for hiding this comment

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

I am making this a separate metric because I don't want to have the amount being set to the number of actions as that's not how we track it in the existing system.

Copy link
Contributor

@saponifi3d saponifi3d May 16, 2025

Choose a reason for hiding this comment

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

sorry for the confusion, the action one is just meant to illustrate an existing metric decorated with the organization id.

mostly just thinking that we could just add the organization id to all the existing metrics if we pull out the check for the feature and assign the org id to a separate variable. (very similar to the comment that dan had left)

Copy link
Member Author

Choose a reason for hiding this comment

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

🤔 all the other metrics in this function pass an amount that's possibly larger than 1, and I only ever want to count a metric issue firing or resolving once otherwise the metrics might not line up

Copy link

codecov bot commented May 16, 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 1:157001 in /home/runner/work/sentry/sentry/.artifacts/pytest.junit.xml

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