-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Adding/fixing support for Azure SQL Database (singleton database) #6111
Conversation
Looks good, would you be able to run the plugin once with
|
@danielnelson Here you go - output requested |
I don't see any sqlserver_scheduler measurements in the output, is this expected? On Let's rename Also, optional and just something for you to consider: these new measurements don't necessarily need to be named like the source table, it might be limiting if you decide to pull in other tables in the future. Think about if there should be a higher/lower concept that they represent. |
So yeah the sqlserver_scheduler one :) I had it disabled in my conf file when ran the test. Sounds good on the waitstats, they have the same measurements. Will make the rest of changes shortly |
I decided to leave sqlserver_azuredb_waitstats as is - the reason is if you mistakenly use a dashboard from on-prem against Azure, will get incorrect results as on Azure the server is just a logical construct and so will be adding up waits from different DB's potentially in one dashboard . On-prem the servername is the starting point - hope am making sense. All other changes made |
Wouldn't having a dashboard that groups by both |
No it won't - Waits on databases aren't additive - aka for an normal SQL Server (on-prem or managened instance) you look at waits at the server level, not the database level as they are server level only so these are semantically different, for Azure SQL DB (single) you only look at a single database waits . Alternatively would have to make a change that stores the type of Database/instance in the config itself or for every measure so based on that dashboard can reflect appropriate thing as just sql_instance or not or can expose a parameter to filter for Azure SQL v/s not. The combination of sql_instance and database_name doesn't tell you the type of DB. necessarily - so will be a bigger change. If we thing would be better to add the SQLEngineType kinda tag to measures to help in future as well, let me know. |
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.
Changes look good. I don't have any Azure SQL DB instances to test against myself.
@denzilribeiro Thanks for the detailed explanation, I do prefer having them as separate measurements to avoid unwanted aggregation. I dislike laying out the data so that one generally needs to exclude certain tags, i.e.: |
Required for all PRs:
As is the SQL Plugin doesn't lend itself to Azure SQL Database.
ServerName is a logical construct on Azure SQL Database, added Database name to all collectors to be able to filter
Added Resource governance collection for Managed instance and Azure SQL Database collected if the AzureDB flag is enabled in telegraf.conf
Added waitstats collection against sys.dm_db_wait_stats if the it is Azure SQL DB instead of server level waits.
Will Publish grafana reports specific to Azure SQL DB separately.
Signed CLA.
[ X] Associated README.md updated.
[ X] Has appropriate unit tests.