-
Notifications
You must be signed in to change notification settings - Fork 468
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
Support user databases for transaction log #4869
Conversation
🌐 Coverage report
|
There was a problem hiding this 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.
@@ -75,7 +75,8 @@ See: [Instructions about each performance counter metrics](https://docs.microsof | |||
|
|||
### 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. |
There was a problem hiding this comment.
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
@@ -17,7 +17,7 @@ streams: | |||
title: Databases | |||
multi: true | |||
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. |
There was a problem hiding this comment.
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"?
# Collect the transaction logs from the system database | ||
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);" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
@@ -5,15 +5,19 @@ hosts: | |||
period: {{period}} | |||
driver: mssql | |||
raw_data.enabled: true | |||
databases: | |||
{{#each databases}} |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Package microsoft_sqlserver - 1.11.0 containing this change is available at https://epr.elastic.co/search?package=microsoft_sqlserver |
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
changelog.yml
file.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