Sql server - ring buffer cpu v2

[Trovalo]

Required for all PRs:

• Signed CLA.
• Associated README.md updated.
• Has appropriate unit tests.

Closes #7269

This PR adds a new measurement called "sqlserver_cpu" with the following structure:

measurement	sql_instance	sqlserver_process_cpu	system_idle_cpu	other_process_cpu
sqlserver_cpu	QDLP03:SQL2019	0	95	5

It reads the CPU data from the ring buffer, therefore it allows to get CPU data from any version of SQL Server (including express and standard).
In the source, the data are updated once every minute.

As of now, it works only on the On-prem version of SQL Server (from SQL 2008 to 2019) but I'm looking to make it work also on Azure SQL DB and managed Instance. (maybe using sys.dm_db_resource_stats or sys.server_resource_stats)

[Trovalo]

@denzilribeiro I would like to hear your opinion about it, especially for making it compatible with Azure SQL DB and Managed Instance. (I have a SQL DB, but not a Managed Instance).
Is sys.dm_db_resource_stats a good way to get those data? do you have any suggestions?

[denzilribeiro]

I would not use this for the cloud - there are better mechanisms. Even for VM's the better approach is to collect OS level metrics right?

For managed instance need to add sys.resource_stats, for SQL DB sys.dm_db_resource_stats is already being collected. I need to spend some time to make couple changes in this area add this as well.
https://docs.microsoft.com/en-us/sql/relational-databases/system-catalog-views/sys-server-resource-stats-azure-sql-database?view=azuresqldb-current . I meant to submit a PR and forgot about it had commented here - #7014

[Trovalo]

I know that getting the data directly from the VM OS is probably the best option, but if you are monitoring a remote instance of SQL Server, it is not possible to use the windows plugin to gather data. In this kind of scenario, the ring buffer data are a good alternative.

Also I'm not sure about what the windows plugin can get "out of the box", can it get Cpu data at process level? (I will check it myself)

[denzilribeiro]

Note you can also get CPU from perfmon ( workload group counters have CPU) but won't get instance level CPU from there. At some point have to make sure we are not spawning too many connections for collection :) the balance.

[Trovalo]

I already get the "CPU %" from the perfcounters measurement (in Resource Pool and Workload Groups), but sadly the Resource governor is available only in Enterprise and developer edition. Therefore, as of now, if you have a Standard or Express edition of SQL Server you can't get Cpu data using the SQL server plugin alone.

I also thought about adding this as a row in the perfcounters measurement, something like Counter = "CPU %", Instance = "_Total". But I didn't like that since it is not an actual perfcounter.

[Trovalo]

In general, I think that the SQL Server plugin alone should allow you to fully monitor your SQL Server instances, in this specific case the CPU.
Having this data in its own measurement/query allows you to be flexible, if you get the CPU from Windows perfmon then that's perfect, just disable the 'Cpu' query in order to not fetch the same info twice.

That said, I agree with you about keeping the plugin as light as possible.
A way to achieve this is the usage of custom errors in case the query is not supported on the target environment. (now it executes an "IF" and does nothing, but still connects and checks every time)

i.e. if the 'Cpu' query is used on Azure SQL DB or Managed instance we can execute

THROW 50001, 'Telegraf "Cpu" query is not supported on Azure SQL DB and Managed Instance', 1;

This might be a somehow breaking change, but it will ensure that people keep their config clean.
Let me know what you think about it

[denzilribeiro]

Can we have the IF loop simplified? Don't have to cast here as evaluation loop only within SQL.
IF SERVERPROPERTY('EngineEdition') IN (5,8)
...
else
...

[denzilribeiro]

Agree on having only targeted queries for the platform in question.When I find time will find a way to actually segregate sections, had a discussion with Daniel as well.. will write up a note before I do that. Effectively can have 3 sections in conf one for each platform ( SQL Server, Azure SQL DB, Azure SQL Managed instance) and a bit different flag rather than just AzureDB = true or false... Will work on that hopefully soon.

[Trovalo]

There is no problem in simplifying the loop by removing the variable declaration, cast and assignment. It will work as long as the comparison is "IN" or "=", but it won't if we use ">" or "<" (in this case no problem).

I did that to match and somehow standardize some changes I have in another open PR about compatibility up to SQL Server 2008 SP4, in which I used the variables to avoid repeating the SERVERPROPERTY(...) and CAST(...) combo.

About the segregation of the different platforms, I will also think about some nice ways to achieve it and then share it with you.
If you already have some ideas can you open a new feature request in order to keep the conversation there?

[Trovalo]

I've updated the readme, now everything should be ok. @denzilribeiro can you check?

[Trovalo]

Hi @denzilribeiro, any update on this?