Fix wrong timestamp for DBM beta feature #9024
Conversation
ghost
added
the
djova
force-pushed
the
djova/postgres-statement-samples-fix-timestamp-bug
branch
2 times, most recently
from
edffc9c
to
9ea8f7e
Compare
| @@ -22,6 +23,8 @@ | |||
|
|
|||
| VALID_EXPLAIN_STATEMENTS = frozenset({'select', 'table', 'delete', 'insert', 'replace', 'update'}) | |||
|
|
|||
| unix_epoch = datetime.datetime(1970, 1, 1, tzinfo=datetime.timezone.utc) | |||
There was a problem hiding this comment.
I don't think we can always assume UTC here. Won't this depend on the configured TZ of the database?
djova
force-pushed
the
djova/postgres-statement-samples-fix-timestamp-bug
branch
from
9ea8f7e
to
a8badb7
Compare
The timestamp derived from the last `state_change` was wrong because it didn't take into account the timezone, causing some events to be dropped during intake. Update to use internal helpers that derive the unix timestamp correctly.
djova
force-pushed
the
djova/postgres-statement-samples-fix-timestamp-bug
branch
from
a8badb7
to
2c9d9a4
Compare
| # If the transaction is idle then we have a more specific "end time" than the current time at | ||
| # which we're collecting this event. Can only be done if the datetime has a timezone. | ||
| if row['state_change'].tzinfo: | ||
| event['timestamp'] = get_timestamp(row['state_change']) * 1000 |
There was a problem hiding this comment.
Just want to make sure I have all of the cases down:
row['state_change']is timezone-aware. Everything will work as expected.row['state_change']is not timezone-aware, the agent is running on the database host.row['state_change']is not timezone-aware, the agent is running off-host.
As long as case #1 is guaranteed, we won't have any problems. #2 should also be fine (unless the database is using a different timezone than the host it's running on).
#3 would mean we default to using the agent timestamp. But because all events are aligned on UTC, we will always be close to the actual DB timestamp? Is this correct?
There was a problem hiding this comment.
According to the docs this is a timestamp with timezone so the timezone info should always be there. It would only be missing if there's a bug in psycopg or if the database host is misconfigured somehow.
#2 should also be fine (unless the database is using a different timezone than the host it's running on).
Yes in that case we use now() as the timestamp, which is still fine because if we're doing one collection per second we should be off by at most one second.
#3 would mean we default to using the agent timestamp. But because all events are aligned on UTC, we will always be close to the actual DB timestamp? Is this correct?
Yeah, same reasoning as #2, falling back to now() should still be within one second of the actual event time, unless the agent was down for a while then woke up and saw some stale connection that had finished its last query a long time ago.
ofek
changed the title
What does this PR do?
The timestamp derived from the last
state_changewas wrong because it didn't take into account the timezone, causing some events to be dropped during intake. Now we do an explicit conversion to a unix timestamp from the UTC Unix epoch date.Motivation
Fix wrong timestamps.
Review checklist (to be filled by reviewers)
changelog/andintegration/labels attached