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

Write DomainMetrics and add FF to switch reads from calculated properties #35522

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

Conversation

nospame
Copy link
Contributor

@nospame nospame commented Dec 13, 2024

Technical Summary

SAAS-16283

Writes "calculated properties" currently stored on the Domain Elasticsearch doc to the new DomainMetrics SQL model and adds a feature flag so that reads can be switched from the ES doc to the SQL model once populated. This is part 2 of 3 for moving calculated properties into SQL, following #35415. This should be reviewable by commit.

Feature Flag

Adds the slightly wordy CALCULATED_PROPERTIES_FROM_DOMAIN_METRICS feature flag, which can be used to switch calculated properties reads over to the SQL model for a single domain, to toggle on for all domains.

Safety Assurance

Safety story

This doesn't affect existing data or the write process for calculated properties on the ES doc. Reading from the new model is controlled by a feature flag, so if there are any issues after switching over, it can easily be switched back.

Automated test coverage

Adds automated tests for identifying the correct project spaces to have their DomainMetrics updated. These are based on existing tests that look to see the right domains will get their calculated properties updated.

QA Plan

I can see an argument for QA, but not planning it at this time. I think dev testing on staging will suffice.

Migrations

  • The migrations in this code can be safely applied first independently of the code

Rollback instructions

  • This PR can be reverted after deploy with no further considerations

Labels & Review

  • Risk label is set correctly
  • The set of people pinged as reviewers is appropriate for the level of risk of the change

@dimagimon dimagimon added reindex/migration Reindex or migration will be required during or before deploy Risk: High Change affects files that have been flagged as high risk. labels Dec 13, 2024
@nospame nospame requested a review from gherceg December 13, 2024 00:46
@nospame nospame force-pushed the em/write-domain-metrics branch from 075ab84 to 42eafa5 Compare December 19, 2024 18:16
@nospame nospame marked this pull request as ready for review December 19, 2024 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reindex/migration Reindex or migration will be required during or before deploy Risk: High Change affects files that have been flagged as high risk.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants