Skip to content

Support user databases for transaction log#4869

Merged
ritalwar merged 6 commits intoelastic:mainfrom
ritalwar:mysql_user_database_4108
Dec 22, 2022
Merged

Support user databases for transaction log#4869
ritalwar merged 6 commits intoelastic:mainfrom
ritalwar:mysql_user_database_4108

Conversation

@ritalwar
Copy link
Copy Markdown
Contributor

@ritalwar ritalwar commented Dec 19, 2022

  • Enhancement

What does this PR do?

The MSSQL Integration currently loads "transaction_log metrics" from system dbs.
This PR has updated the integration to load "transaction_log metrics" from the user dbs by providing the list of databases for which the metrics is to be collected.

Also, this PR fixes the bug in the query for fetching metrics from sys.dm_db_log_space_usage.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

How to test this PR locally

Add the microsoft_sql_server integration, add server config, enable transaction_log metrics and verify the data.
Also, for the metrics from sys.dm_db_log_space_usage table, specify the list of databases.

Related issues

Screenshots

Screenshot 2022-12-19 at 9 22 24 PM

Screenshot 2022-12-19 at 9 23 11 PM

@ritalwar ritalwar self-assigned this Dec 19, 2022
@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Dec 19, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-12-22T11:48:26.783+0000

  • Duration: 17 min 30 sec

Test stats 🧪

Test Results
Failed 0
Passed 19
Skipped 0
Total 19

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Dec 19, 2022

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (2/2) 💚
Files 100.0% (2/2) 💚
Classes 100.0% (2/2) 💚
Methods 93.333% (28/30)
Lines 100.0% (1238/1238) 💚
Conditionals 100.0% (0/0) 💚

@ritalwar ritalwar marked this pull request as ready for review December 20, 2022 08:16
@ritalwar ritalwar requested review from a team as code owners December 20, 2022 08:16
Copy link
Copy Markdown
Contributor

@lalit-satapathy lalit-satapathy left a comment

Choose a reason for hiding this comment

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

Let's not close #4108 rather keep it open if we find a future solution to automatically find all user DBs.

### transaction_log metrics

Collects system level `transaction_log` metrics information for SQL Server instance.
Collects `transaction_log` metrics information for all the system and user Dbs of SQL Server instance.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: let's be consistent with Dbs or dbs

required: true
show_user: false
show_user: true
description: Default system databases are preloaded. For custom database please add additional rows and enter the database name.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Change from "custom database" to "user defined databases"?

sql_queries:
- query: "SELECT @@servername AS server_name, @@servicename AS instance_name, name As 'database_name', database_id FROM sys.databases;"
response_format: table
- query: "SELECT @@servername AS server_name, @@servicename AS instance_name, name As 'database_name', s.database_id, total_log_size_mb, active_log_size_mb,log_backup_time,log_since_last_log_backup_mb,log_since_last_checkpoint_mb,log_recovery_size_mb from sys.databases As s CROSS APPLY sys.dm_db_log_stats(s.database_id);"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When you show all db data for log_stats and only customer provided db data for log_space_usage, there is a possibility that customer will not provided the custom database names then Kibana will only represent data for log_stats and not the log_space_usage for the selected custom database. I would recommend keeping both the stats rendered in similar way.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As per discussion with Lalit, we are providing blend of long and short term solution, where we are giving all databases metrics for log_stats and for log_space_usage we are providing short term solution i.e user can input dbs and will get the metrics for same.
This has also been updated in docs.
Additional benefit is that user will have list of all the dbs as result of 1st query and they can give the names from there to get metrics of log_space_usage.

driver: mssql
raw_data.enabled: true
databases:
{{#each databases}}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need this database iteration here ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is needed to get the list of databases to fetch user dbs metrics, upon which the query iterator will work.

Copy link
Copy Markdown
Contributor

@ShourieG ShourieG left a comment

Choose a reason for hiding this comment

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

LGTM

@muthu-mps muthu-mps self-requested a review December 22, 2022 15:03
Copy link
Copy Markdown
Contributor

@muthu-mps muthu-mps left a comment

Choose a reason for hiding this comment

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

LGTM!

@ritalwar ritalwar merged commit 91adc40 into elastic:main Dec 22, 2022
@elasticmachine
Copy link
Copy Markdown

Package microsoft_sqlserver - 1.11.0 containing this change is available at https://epr.elastic.co/search?package=microsoft_sqlserver

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.

[Microsoft Sqlserver] - Transaction Log query doesn't respect the database name as query parameter

5 participants