Skip to content
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

Add DataConditions for workflow_engine #77448

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

Conversation

saponifi3d
Copy link
Contributor

@saponifi3d saponifi3d commented Sep 12, 2024

Description

📜 Technical spec

PR Changes

  • Adds DataConditions
  • Connects DataCondition with a 1 to many relationship for DataConditionGroups the Group model allows us to define ANY and ALL, and group the conditions for evaluation.

@github-actions github-actions bot added Scope: Frontend Automatically applied to PRs that change frontend components Scope: Backend Automatically applied to PRs that change backend components labels Sep 12, 2024
Copy link
Contributor

🚨 Warning: This pull request contains Frontend and Backend changes!

It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently.

Have questions? Please ask in the #discuss-dev-infra channel.

Copy link
Contributor

This PR has a migration; here is the generated SQL for src/sentry/workflow_engine/migrations/0005_datacondition.py ()

--
-- Create model DataCondition
--
CREATE TABLE "workflow_engine_datacondition" ("id" bigint NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "date_updated" timestamp with time zone NOT NULL, "date_added" timestamp with time zone NOT NULL, "condition" varchar(200) NOT NULL, "threshold" double precision NOT NULL, "condition_result" jsonb NOT NULL, "type" varchar(200) NOT NULL, "detectors_id" bigint NOT NULL, "workflow_action_id" bigint NOT NULL);
ALTER TABLE "workflow_engine_datacondition" ADD CONSTRAINT "workflow_engine_data_detectors_id_d9f45fc7_fk_workflow_" FOREIGN KEY ("detectors_id") REFERENCES "workflow_engine_detector" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "workflow_engine_datacondition" VALIDATE CONSTRAINT "workflow_engine_data_detectors_id_d9f45fc7_fk_workflow_";
ALTER TABLE "workflow_engine_datacondition" ADD CONSTRAINT "workflow_engine_data_workflow_action_id_7d1dcfde_fk_workflow_" FOREIGN KEY ("workflow_action_id") REFERENCES "workflow_engine_workflowaction" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "workflow_engine_datacondition" VALIDATE CONSTRAINT "workflow_engine_data_workflow_action_id_7d1dcfde_fk_workflow_";
CREATE INDEX CONCURRENTLY "workflow_engine_datacondition_detectors_id_d9f45fc7" ON "workflow_engine_datacondition" ("detectors_id");
CREATE INDEX CONCURRENTLY "workflow_engine_datacondition_workflow_action_id_7d1dcfde" ON "workflow_engine_datacondition" ("workflow_action_id");

Copy link

codecov bot commented Sep 12, 2024

❌ 1467 Tests Failed:

Tests completed Failed Passed Skipped
1467 1467 0 0
View the top 3 failed tests by shortest run time
 tests.acceptance.sentry_plugins.test_victorops
Stack Traces | 0s run time
No failure message available
 tests.sentry.sentry_apps.models.test_sentryappinstallationtoken
Stack Traces | 0s run time
No failure message available
 tests.sentry.testutils.helpers.test_features
Stack Traces | 0s run time
No failure message available

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

Copy link
Contributor

This PR has a migration; here is the generated SQL for src/sentry/workflow_engine/migrations/0006_data_condition.py ()

--
-- Create model DataCondition
--
CREATE TABLE "workflow_engine_datacondition" ("id" bigint NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "date_updated" timestamp with time zone NOT NULL, "date_added" timestamp with time zone NOT NULL, "condition" varchar(200) NOT NULL, "threshold" double precision NOT NULL, "condition_result" jsonb NOT NULL, "type" varchar(200) NOT NULL, "detector_id" bigint NOT NULL, "workflow_action_id" bigint NOT NULL);
ALTER TABLE "workflow_engine_datacondition" ADD CONSTRAINT "workflow_engine_data_detector_id_c0979477_fk_workflow_" FOREIGN KEY ("detector_id") REFERENCES "workflow_engine_detector" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "workflow_engine_datacondition" VALIDATE CONSTRAINT "workflow_engine_data_detector_id_c0979477_fk_workflow_";
ALTER TABLE "workflow_engine_datacondition" ADD CONSTRAINT "workflow_engine_data_workflow_action_id_7d1dcfde_fk_workflow_" FOREIGN KEY ("workflow_action_id") REFERENCES "workflow_engine_workflowaction" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "workflow_engine_datacondition" VALIDATE CONSTRAINT "workflow_engine_data_workflow_action_id_7d1dcfde_fk_workflow_";
CREATE INDEX CONCURRENTLY "workflow_engine_datacondition_detector_id_c0979477" ON "workflow_engine_datacondition" ("detector_id");
CREATE INDEX CONCURRENTLY "workflow_engine_datacondition_workflow_action_id_7d1dcfde" ON "workflow_engine_datacondition" ("workflow_action_id");

Copy link
Contributor

github-actions bot commented Sep 16, 2024

This PR has a migration; here is the generated SQL for src/sentry/workflow_engine/migrations/0006_data_conditions.py ()

--
-- Create model DataConditionGroup
--
CREATE TABLE "workflow_engine_dataconditiongroup" ("id" bigint NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "date_updated" timestamp with time zone NOT NULL, "date_added" timestamp with time zone NOT NULL, "logic_type" varchar(200) NOT NULL, "detector_id" bigint NULL, "workflow_action_id" bigint NULL);
ALTER TABLE "workflow_engine_dataconditiongroup" ADD CONSTRAINT "workflow_engine_data_detector_id_5b8d9491_fk_workflow_" FOREIGN KEY ("detector_id") REFERENCES "workflow_engine_detector" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "workflow_engine_dataconditiongroup" VALIDATE CONSTRAINT "workflow_engine_data_detector_id_5b8d9491_fk_workflow_";
ALTER TABLE "workflow_engine_dataconditiongroup" ADD CONSTRAINT "workflow_engine_data_workflow_action_id_600a68ea_fk_workflow_" FOREIGN KEY ("workflow_action_id") REFERENCES "workflow_engine_workflowaction" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "workflow_engine_dataconditiongroup" VALIDATE CONSTRAINT "workflow_engine_data_workflow_action_id_600a68ea_fk_workflow_";
CREATE INDEX CONCURRENTLY "workflow_engine_dataconditiongroup_detector_id_5b8d9491" ON "workflow_engine_dataconditiongroup" ("detector_id");
CREATE INDEX CONCURRENTLY "workflow_engine_dataconditiongroup_workflow_action_id_600a68ea" ON "workflow_engine_dataconditiongroup" ("workflow_action_id");
--
-- Create model DataCondition
--
CREATE TABLE "workflow_engine_datacondition" ("id" bigint NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "date_updated" timestamp with time zone NOT NULL, "date_added" timestamp with time zone NOT NULL, "condition" varchar(200) NOT NULL, "threshold" double precision NOT NULL, "condition_result" jsonb NOT NULL, "type" varchar(200) NOT NULL, "condition_group_id" bigint NOT NULL);
ALTER TABLE "workflow_engine_datacondition" ADD CONSTRAINT "workflow_engine_data_condition_group_id_122daf08_fk_workflow_" FOREIGN KEY ("condition_group_id") REFERENCES "workflow_engine_dataconditiongroup" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "workflow_engine_datacondition" VALIDATE CONSTRAINT "workflow_engine_data_condition_group_id_122daf08_fk_workflow_";
CREATE INDEX CONCURRENTLY "workflow_engine_datacondition_condition_group_id_122daf08" ON "workflow_engine_datacondition" ("condition_group_id");

Copy link
Member

@markstory markstory left a comment

Choose a reason for hiding this comment

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

Schema looks good to me.


condition = models.CharField(max_length=200)
threshold = models.FloatField()
condition_result = models.JSONField()
Copy link
Member

Choose a reason for hiding this comment

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

How are we planning to handle the schema for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This schema would just be a primitive value; either a string for the detector or boolean for the workflow actions. i can imagine some other scenarios that having this be flexible as a json primitive - but it should never be a compex object / array etc.

is there a good way to enforce that? (off the top of my head, we could have a pre-save signal that validates it's a primitive or throws an exception? 🤔)

@region_silo_model
class DataConditionGroup(DefaultFieldsModel):
"""
A data condition is a way to specify a condition that must be met for a workflow to execute.
Copy link
Member

Choose a reason for hiding this comment

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

A data group is a way to specify a group of conditions that must be met for a workflow action to execute

Comment on lines 26 to 28
workflow_action = models.ForeignKey(
WorkflowAction, on_delete=models.CASCADE, blank=True, null=True
)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we allow multiple workflow actions to be linked here? So this group of conditions fires this group of actions?

…ated a new model called data condition group which will also store logic gates for the data conditions
- Changed workflow_action to be action, and more generic. will use this
  to emit events on detectors or send notifications in workflows
- Created lookup tables; detector_workflow and
  workflow_data_condition_group
- Updated detector and workflow models to represent new design
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 Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants