Skip to content

Adding timestamp to KmsLogger#11933

Merged
mww59 merged 3 commits intomainfrom
mww59/add-timestamp-to-kms-log
Mar 4, 2025
Merged

Adding timestamp to KmsLogger#11933
mww59 merged 3 commits intomainfrom
mww59/add-timestamp-to-kms-log

Conversation

@mww59
Copy link
Contributor

@mww59 mww59 commented Feb 28, 2025

🛠 Summary of changes

Adds a timestamp to kms.log. Currently, when logs from kms.log are ingested into Cloudwatch, the cloudwatch-agent uses the current time as the timestamp. This can be skewed by up to 150s from when the kms event was actually logged.

Before (CloudTrail event time: 2025-02-28T13:17:19Z):

2025-02-28T13:17:23.854Z | {"kms":{"action":"decrypt","encryption_context":...

After (CloudTrail event time: 2025-02-28T14:17:54Z):

2025-02-28T14:17:54.248Z | {"kms":{"timestamp":"2025-02-28T14:17:54.248Z","action":"decrypt","encryption_context": ...

@mww59 mww59 requested a review from mitchellhenke February 28, 2025 16:32
@mww59 mww59 self-assigned this Feb 28, 2025
Comment on lines +5 to +8
def self.log(action, key_id:, context: nil, log_context: nil)
output = {
kms: {
timestamp: Time.zone.now,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def self.log(action, key_id:, context: nil, log_context: nil)
output = {
kms: {
timestamp: Time.zone.now,
def self.log(action:, timestamp:, key_id:, context: nil, log_context: nil)
output = {
kms: {
timestamp: timestamp,

I wonder if it might be preferable to pass the time as an argument? Would have some implications around the tests, but I suspect they will require freezing time.

@mww59
Copy link
Contributor Author

mww59 commented Mar 4, 2025

Confirming logs after updates:

2025-03-04T14:23:57.003-05:00 {"kms":{"timestamp":"2025-03-04T19:23:57.003Z","action":"decrypt","encryption_context":{"context":"password-digest"

@mww59 mww59 merged commit 5ed882f into main Mar 4, 2025
2 checks passed
@mww59 mww59 deleted the mww59/add-timestamp-to-kms-log branch March 4, 2025 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants