Only log read only queries if they take more than 100ms#2672
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 326c729342
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // If we're not inside a transaction, this doesn't get added to our normal transaction timing. | ||
| if (!_insideTransaction) { | ||
| // This log line will happen a lot. Let's log only when it takes more than 100ms to complete. | ||
| if (!_insideTransaction && _readElapsed > 100'000) { |
There was a problem hiding this comment.
For autocommit reads, _readElapsed accumulates across calls and is only reset when an explicit transaction starts, while the logged totalTransactionElapsed value is the current timeSpent. Once enough fast reads outside a transaction cumulatively exceed 100ms, every later read on that handle will still emit this noisy log even if it only took a few microseconds, so the new filter does not actually limit logging to slow auto-transactions; this condition should gate on timeSpent instead.
Useful? React with 👍 / 👎.
| // If we're not inside a transaction, this doesn't get added to our normal transaction timing. | ||
| if (!_insideTransaction) { | ||
| // This log line will happen a lot. Let's log only when it takes more than 100ms to complete. | ||
| if (!_insideTransaction && _readElapsed > 100'000) { |
There was a problem hiding this comment.
I am seeing a lot of
Read with auto-transaction timing info ~~ totalTransactionElapsed: '0' command: 'OpenApp'
Maybe we should update the if to also check timeSpent?
3 participants
Details
Filter log so we only log transactions that took more than 100ms
Fixed Issues
Fixes https://github.com/Expensify/Expensify/issues/655406
Tests
N/A
Internal Testing Reminder: when changing bedrock, please compile auth against your new changes