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

feat(dyn-sampling): add new bias toggle to project details for prioritise by tx name [TET-717] #44944

Merged
merged 8 commits into from
Feb 28, 2023

Conversation

andriisoldatenko
Copy link
Contributor

@andriisoldatenko andriisoldatenko commented Feb 22, 2023

This PR adds new toggle for project details for prioritise by tx name

Tested locally:

image

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Feb 22, 2023
@andriisoldatenko andriisoldatenko changed the title feat: add new bias toggle to project details for prioritise by tx name feat(dyn-sampling): add new bias toggle to project details for prioritise by tx name [TET-717] Feb 22, 2023
@andriisoldatenko andriisoldatenko changed the title feat(dyn-sampling): add new bias toggle to project details for prioritise by tx name [TET-717] feat(dyn-sampling): add new bias toggle to project details for prioritise by tx name [TET-574] Feb 23, 2023
@andriisoldatenko andriisoldatenko changed the title feat(dyn-sampling): add new bias toggle to project details for prioritise by tx name [TET-574] feat(dyn-sampling): add new bias toggle to project details for prioritise by tx name [TET-717] Feb 23, 2023
@@ -31,6 +31,7 @@ class RuleType(Enum):
BOOST_LATEST_RELEASES_RULE = "boostLatestRelease"
IGNORE_HEALTH_CHECKS_RULE = "ignoreHealthChecks"
BOOST_KEY_TRANSACTIONS_RULE = "boostKeyTransactions"
PRIORITISE_BY_TX_NAME_RULE = "prioritiseByTxName"
Copy link
Member

Choose a reason for hiding this comment

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

shall we make this plural to be consistent?

Suggested change
PRIORITISE_BY_TX_NAME_RULE = "prioritiseByTxName"
PRIORITIZE_BY_TRANSACTION_NAMES_RULE = "prioritizeByTransactionNames"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah sure we can rename, I had a similar name but then decided to make it shorter.

@@ -1468,6 +1470,7 @@ def test_put_new_dynamic_sampling_rules_with_correct_flags(self):
},
{"id": "ignoreHealthChecks", "active": False},
{"id": "boostKeyTransactions", "active": False},
{"id": "prioritiseByTxName", "active": False},
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you missed the renaming here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in b064f9c

@@ -1351,6 +1352,7 @@ def test_get_dynamic_sampling_biases_with_previously_assigned_biases(self):
},
{"id": "ignoreHealthChecks", "active": True},
{"id": "boostKeyTransactions", "active": True},
{"id": "prioritiseByTxName", "active": False},
Copy link
Member

Choose a reason for hiding this comment

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

Also here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, let me fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andriisoldatenko andriisoldatenko requested review from iambriccardo and RaduW and removed request for iambriccardo February 27, 2023 08:34
@iambriccardo
Copy link
Member

We would also need a separate PR to update the logging system.

@andriisoldatenko
Copy link
Contributor Author

We would also need a separate PR to update the logging system.

Please create a ticket with details and I can fix it!

Copy link
Member

@priscilawebdev priscilawebdev left a comment

Choose a reason for hiding this comment

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

Checked with Andrii locally and everything looks good

@andriisoldatenko andriisoldatenko merged commit a56d65b into master Feb 28, 2023
@andriisoldatenko andriisoldatenko deleted the andrii/add-toggle-prioritise-by-tx-name branch February 28, 2023 12:24
jan-auer added a commit that referenced this pull request Feb 28, 2023
* master: (79 commits)
  feat(perf-issues): Add performance issue detection timing runner command (#44912)
  Revert "chore: Investigating org slug already set to a different value (#45134)"
  fix(hybrid-cloud): Redirect to org restoration page for customer domains (#45159)
  bug(replays): Fix 500 error when marshaling tags field (#45097)
  ref(sourcemaps): Redesign lookup of source and sourcemaps (#45032)
  chore: Investigating org slug already set to a different value (#45134)
  feat(dynamic-sampling): Implement prioritize by project bias [TET-574] (#42939)
  feat(dynamic-sampling): Add transaction name prioritize option - (#45034)
  feat(dyn-sampling): add new bias toggle to project details for prioritise by tx name [TET-717] (#44944)
  feat(admin) Add admin relay project config view [TET-509] (#45120)
  Revert "chore(assignment): Add analytics when autoassigning after a manual assignment (#45099)"
  feat(sourcemaps): Implement new tables supporting debug ids (#44572)
  ref(js): Remove usage of react-document-title (#45170)
  chore(py): Consistently name urls using `organization-` prefix (#45180)
  ref: rename acceptance required checks collector (#45156)
  chore(assignment): Add analytics when autoassigning after a manual assignment (#45099)
  feat(source-maps): Update copy for source map debug alerts (#45164)
  ref(js): Remove custom usage of DocumentTitle (#45165)
  chore(login): update the login banners (#45151)
  ref(py): Remove one more legacy project_id from Environment (#45160)
  ...
@github-actions github-actions bot locked and limited conversation to collaborators Mar 15, 2023
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.

4 participants