Skip to content
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

Consolidate query execution logs #7115

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

zachliu
Copy link
Contributor

@zachliu zachliu commented Aug 7, 2024

What type of PR is this?

  • Refactor
  • Feature
  • Bug Fix
  • New Query Runner (Data Source)
  • New Alert Destination
  • Other

Description

Redundant log entries are removed. All information (in-memory result size, query runtime, etc.) is consolidated under the _log_progress() method.

Another benefit of consolidated log entries is that when using tools such as DataDog, custom metrics can be easily generated from parsed logs:

dd

Because username and query_id are now tied with data_length and run_time by putting them in the same log entry.
Much easier to track rogue users who like to do SELECT * FROM big_table or any other inefficient queries.

How is this tested?

  • Unit tests (pytest, jest)
  • E2E Tests (Cypress)
  • Manually
  • N/A

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

@justinclift
Copy link
Member

The concept of this sounds good. I'll have to look at the implementation later on after some other tasks though, if no-one else gets to it first.

@lucasfcnunes Is this your kind of thing to review the technical details of? 😄

@lucasfcnunes
Copy link
Member

😆 I'll take a look

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.

4 participants