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

Clarify the enabled property in database_metrics configuration descriptions for sqlserver #19288

Merged
merged 11 commits into from
Dec 21, 2024

Conversation

azhou-datadog
Copy link
Contributor

@azhou-datadog azhou-datadog commented Dec 19, 2024

What does this PR do?

Add the enabled property to description of database_metrics configuration descriptions for sqlserver

Motivation

Realized thanks to help from @willstrickfaden and Faizan Hussain in the support channel, that the conf.yaml.example does not sufficiently surface the properties available for the database_metrics configurations. Most critically, it is not clear that in order to enable an metric, the format must look something like this (with the enabled property set to true):

...
database_metrics:
  master_files_metrics:
    enabled: true
  index_usage_metrics:
    enabled: true
  db_fragmentation_metrics:
    enabled: true
...

This PR explicitly states each of the properties available for each database_metric configuration, making this more clear.

Please note, the include_xyz_metrics: true configuration format will still continue to work in future agent releases as well, in order to maintain backwards compatibility of conf.yaml files.

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 changelog type changed or removed was used in this Pull Request, so the next release will bump major version. Please make sure this is a breaking change, or use the fixed or added type instead.

Copy link

The changelog type changed or removed was used in this Pull Request, so the next release will bump major version. Please make sure this is a breaking change, or use the fixed or added type instead.

lu-zhengda
lu-zhengda previously approved these changes Dec 19, 2024
Copy link

codecov bot commented Dec 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.98%. Comparing base (32f8ae9) to head (1e3bbd5).
Report is 4 commits behind head on master.

Additional details and impacted files
Flag Coverage Δ
sqlserver 91.15% <ø> (+9.51%) ⬆️

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

@azhou-datadog azhou-datadog changed the title Clarify the enabled property in database_metrics configuration descriptions for sqlserver DRAFT: Clarify the enabled property in database_metrics configuration descriptions for sqlserver Dec 19, 2024
…b.com:DataDog/integrations-core into allen.zhou/update_sqlserver_spec_descriptions
@azhou-datadog azhou-datadog added the qa/skip-qa Automatically skip this PR for the next QA label Dec 20, 2024
@azhou-datadog azhou-datadog changed the title DRAFT: Clarify the enabled property in database_metrics configuration descriptions for sqlserver Clarify the enabled property in database_metrics configuration descriptions for sqlserver Dec 20, 2024
@azhou-datadog azhou-datadog added this pull request to the merge queue Dec 21, 2024
Merged via the queue into master with commit aa9bd4e Dec 21, 2024
41 of 42 checks passed
@azhou-datadog azhou-datadog deleted the allen.zhou/update_sqlserver_spec_descriptions branch December 21, 2024 15:14
github-actions bot pushed a commit to yogesh-s-modak/integrations-core that referenced this pull request Dec 23, 2024
…ptions for sqlserver (DataDog#19288)

* Clarify the enabled property in database_metrics configuration

* changelog

* database_metrics don't require dbm

* Delete sqlserver/changelog.d/19288.changed

* changelog

* Sync configs

* Delete sqlserver/changelog.d/19288.added

* Review comments

* sync instance change aa9bd4e
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