-
Notifications
You must be signed in to change notification settings - Fork 6
sch-UID2-5853 Added logs for key bucket count in salt rotation #568
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
sch-UID2-5853 Added logs for key bucket count in salt rotation #568
Conversation
| } | ||
|
|
||
| private void logBucketFormatCount(TargetDate targetDate, SaltEntry[] preRotationSalts, SaltEntry[] postRotationSalts) { | ||
| int newKeyBucketCounter = 0, totalKeyBucketCounter = 0, totalSaltBucketCounter = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this deserves a comment - mention the migration, that the salts are the old format and we're transitioning to encryption keys. Mention that this is to monitor the migration.
| var oldSalt = preRotationSalts[i]; | ||
| var updatedSalt = postRotationSalts[i]; | ||
|
|
||
| if (updatedSalt.currentKey() != null) totalKeyBucketCounter++; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will we also do the previous salt/keys?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to monitor those - I'm not sure how helpful they would be in monitoring the migration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should start seeing the first "previous key" ~1 year after we start migration and we should stop seeing "previous salt" soon after that, at least by day 420. We can only remove the salt-related code after both all current and previous salts are gone.
| if (updatedSalt.currentKey() != null && oldSalt.currentSalt() != null) newKeyBucketCounter++; | ||
| } | ||
|
|
||
| LOGGER.info("Salt rotation bucket format: target_date={} new_key_bucket_count={} total_key_bucket_count={} total_salt_bucket_count={}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be clearer that those are formats of buckets rotated just now, not for all bucket.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are for all the buckets in snapshot, including those that haven't rotated, whereas the new_key_bucket_count are for buckets that have rotated just now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated new_key_bucket_count to migrated_key_bucket_count to hopefully make that clearer
…5853-metrics-and-dashboard-for-key-rotation
…5853-metrics-and-dashboard-for-key-rotation
…5853-metrics-and-dashboard-for-key-rotation
d3f1587
into
sch-UID2-5851-migration-to-key-rotation
After each salt rotation, logs: