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-4720: Make all sql server database metrics configurable #19111

Merged
merged 28 commits into from
Dec 2, 2024

Conversation

azhou-datadog
Copy link
Contributor

@azhou-datadog azhou-datadog commented Nov 21, 2024

What does this PR do?

  1. Make collection of all database metrics for sql server configurable
  2. Restructure database metrics in configuration file so related configurations are nested and visually grouped. This also consolidates setting the default database metric configurations in a single file, config.py, rather than spread throughout the codebase.
  3. Deprecates usage of instance_config in favor of SQLServerConfig in database_metrics.

Motivation

Allowing database metrics collection to be individually configurable allows support to identify and triage performance issues related to metric collection. The new structure also is more extensible in the future, it is now easy to add related metric collection configurations.
Prevent passing instance_config unnecessarily down to SqlserverDatabaseMetricsBase objects, when it can all be handled in the initial configuration of SQLServerConfig.

OLD CONFIGURATION STRUCTURE
include_xyz_metric : True
include_foo_metric: False
include_xyz_metric_tempdb: False
include_xyz_metric_interval: 300
etc...

NEW CONFIGURATION STRUCTURE
database_metrics
 xyz_metric
  enabled: True
  interval: 300
  enabled_tempdb: False
 foo_metric
  enabled: False
etc...

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Add the qa/skip-qa label if the PR doesn't need to be tested during QA.
  • If you need to backport this PR to another branch, you can add the backport/<branch-name> label to the PR and it will automatically open a backport PR once this one is merged

Copy link

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

Copy link

codecov bot commented Nov 21, 2024

Codecov Report

Attention: Patch coverage is 98.35165% with 3 lines in your changes missing coverage. Please review.

Project coverage is 87.36%. Comparing base (6bbd75f) to head (fbd2497).
Report is 1 commits behind head on master.

Additional details and impacted files
Flag Coverage Δ
activemq ?
cassandra ?
hive ?
hivemq ?
hudi ?
ignite ?
jboss_wildfly ?
kafka ?
nvidia_nim ?
presto ?
solr ?
sqlserver 91.16% <98.35%> (+9.27%) ⬆️

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

Copy link

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

@azhou-datadog azhou-datadog changed the title DBMON-4720 Part 1: Deprecate instance_config in database_metrics DBMON-4720: Make all sql server database metrics configurable Nov 26, 2024
@azhou-datadog azhou-datadog added this pull request to the merge queue Dec 2, 2024
Merged via the queue into master with commit f527ddf Dec 2, 2024
34 checks passed
@azhou-datadog azhou-datadog deleted the allen.zhou-DBMON4720 branch December 2, 2024 17:24
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.

3 participants