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

[DBMON-3050] Collect dynamic relation metrics for autodiscovered databases #16076

Merged
merged 2 commits into from
Oct 25, 2023

Conversation

lu-zhengda
Copy link
Contributor

@lu-zhengda lu-zhengda commented Oct 24, 2023

What does this PR do?

This PR fixes issue relates to not collecting dynamic relation metrics for auto-discovered databases. One side effect of this issue is that the table_size of auto-discovered tables are not populated in DBM Schema tab.

After fix:
image
image

Motivation

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Changelog entries must be created for modifications to shipped code
  • Add the qa/skip-qa label if the PR doesn't need to be tested during QA.

@ghost ghost added the integration/postgres label Oct 24, 2023
@codecov
Copy link

codecov bot commented Oct 24, 2023

Codecov Report

Merging #16076 (e9ff849) into master (d93e894) will increase coverage by 0.00%.
Report is 7 commits behind head on master.
The diff coverage is 92.59%.

Flag Coverage Δ
confluent_platform ?
hudi ?
postgres 91.92% <92.59%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@github-actions
Copy link

github-actions bot commented Oct 24, 2023

Test Results

     12 files       12 suites   16m 12s ⏱️
   239 tests    237 ✔️   2 💤 0
1 440 runs  1 385 ✔️ 55 💤 0

Results for commit e9ff849.

♻️ This comment has been updated with latest results.

@ghost ghost added the documentation label Oct 24, 2023
@lu-zhengda lu-zhengda changed the title Collect dynamic relation metrics for autodiscovered databases [DBMON-3050] Collect dynamic relation metrics for autodiscovered databases Oct 25, 2023
@lu-zhengda lu-zhengda marked this pull request as ready for review October 25, 2023 00:07
@lu-zhengda lu-zhengda requested review from a team as code owners October 25, 2023 00:07
def execute_query_raw(self, query):
with self.db() as conn:
def execute_query_raw(self, query, db):
with db() as conn:
Copy link
Contributor

Choose a reason for hiding this comment

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

nice, i think we can do something similar in sqlserver to get rid of this stuff where we have to collect per database && have some weird logic around how we do that. Instead, we can use this same path to migrate the query executor in the sqlserver integration to run these queries across all databases. Just a thought !

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I also kinda want to "simplify" the way we switch db within the same connection for sqlserver. Ideally it's abstracted from metrics collection

@lu-zhengda lu-zhengda merged commit b9f2d72 into master Oct 25, 2023
47 checks passed
@lu-zhengda lu-zhengda deleted the zhengda.lu/dynamic-relation-autodiscovery branch October 25, 2023 19:08
alexandre-normand added a commit that referenced this pull request Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants