Skip to content

fix(monitor): DB-accurate max_age metrics#89

Merged
smudge merged 1 commit intoBetterment:mainfrom
smudge:monitor-perf/max-age-metrics
Feb 5, 2026
Merged

fix(monitor): DB-accurate max_age metrics#89
smudge merged 1 commit intoBetterment:mainfrom
smudge:monitor-perf/max-age-metrics

Conversation

@smudge
Copy link
Member

@smudge smudge commented Feb 5, 2026

This ensures that max_lock_age and max_age are not artificially inflated by slower query times. Previously, if the query took N seconds to perform, N seconds would be artificially added to max_age and max_lock_age. (Typically this is not a problem for sub-second queries on small-medium tables, but on large tables this can trigger occasional alert thresholds on sensitive/high-priority queues.)

With this PR, we rely on the DB's own reported UTC time at the time of the query to compute timestamp ages as of said query.

In PostgreSQL, the DB's "now" will correspond to the start of the transaction (as should the state of the rows it returns), and in other DB engines it will correspond to a clock time during execution, so there is still room for variance but it is much smaller than before.

/no-platform

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably best to view this diff with ?w=1 in the URL (to skip whitespace-only changes).


match do |block|
@expected = { event_name: expected_event_name, payload: expected_payload, value: expected_value }
if @approximately && current_adapter != 'postgresql'
Copy link
Member Author

Choose a reason for hiding this comment

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

On postgres, we can fully align Timecop with the transaction's NOW() -- on other DB engines, we need to account for slight variance in query time.


db_adapter = ENV["ADAPTER"]
gemfile = ENV["BUNDLE_GEMFILE"]
db_adapter ||= gemfile && gemfile[%r{gemfiles/(.*?)/}] && $1 # rubocop:disable Style/PerlBackrefs
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed this line entirely as we no longer rely on adapter-specific gemfiles.

effron
effron previously approved these changes Feb 5, 2026
Copy link
Contributor

@effron effron left a comment

Choose a reason for hiding this comment

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

domainLGTM! good catch

okasten
okasten previously approved these changes Feb 5, 2026
Copy link

@okasten okasten left a comment

Choose a reason for hiding this comment

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

domain lgtm platform lgtm

yay

This ensures that max_lock_age and max_age are not artificially inflated
by slower query times.

Previously, if the query took N seconds to perform, N seconds would be
artificially added to `max_age` and `max_lock_age`.

Now, we rely on the DB's own reported UTC time at the time of the query.

In PostgreSQL, this will correspond to the start of the transaction, and
in other DB engines this will correspond to the DB's clock time as the
query executes, so there is still some small variance but it is much
smaller than before.

/no-platform
Copy link
Contributor

@effron effron left a comment

Choose a reason for hiding this comment

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

domainLGTM

@smudge smudge merged commit c06c197 into Betterment:main Feb 5, 2026
25 checks passed
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.

3 participants