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

Sqlserver Input Plugin Writing Tags That Should Be Fields #7525

Closed
samhld opened this issue May 15, 2020 · 3 comments
Closed

Sqlserver Input Plugin Writing Tags That Should Be Fields #7525

samhld opened this issue May 15, 2020 · 3 comments
Labels
area/sqlserver feature request Requests for new plugin and for new features to existing plugins performance problems with decreased performance or enhancements that improve performance

Comments

@samhld
Copy link
Contributor

samhld commented May 15, 2020

Expected behavior:

These metrics (names after SQLServer:<Measurement\) should be written as Fields:

SQLServer:Buffer Manager\Buffer cache hit ratio
SQLServer:Buffer Manager\Page life expectancy
SQLServer:Buffer Node\Page life expectancy
SQLServer:Database Replica\Log Apply Pending Queue
SQLServer:Database Replica\Log Apply Ready Queue
SQLServer:Database Replica\Log Send Queue
SQLServer:Database Replica\Recovery Queue
SQLServer:Databases\Data File(s) Size (KB)
SQLServer:Databases\Log File(s) Size (KB)
SQLServer:Databases\Log File(s) Used Size (KB)
SQLServer:Databases\XTP Memory Used (KB)
SQLServer:General Statistics\Active Temp Tables
SQLServer:General Statistics\Processes blocked
SQLServer:General Statistics\Temp Tables For Destruction
SQLServer:General Statistics\User Connections
SQLServer:Memory Broker Clerks\Memory broker clerk size
SQLServer:Memory Manager\Memory Grants Pending
SQLServer:Memory Manager\Target Server Memory (KB)
SQLServer:Memory Manager\Total Server Memory (KB)
SQLServer:Resource Pool Stats\Active memory grant amount (KB)
SQLServer:Resource Pool Stats\Disk Read Bytes/sec
SQLServer:Resource Pool Stats\Disk Read IO Throttled/sec
SQLServer:Resource Pool Stats\Disk Read IO/sec
SQLServer:Resource Pool Stats\Disk Write Bytes/sec
SQLServer:Resource Pool Stats\Disk Write IO Throttled/sec
SQLServer:Resource Pool Stats\Disk Write IO/sec
SQLServer:Resource Pool Stats\Used memory (KB)
SQLServer:Transactions\Free Space in tempdb (KB)
SQLServer:Transactions\Version Store Size (KB)
SQLServer:User Settable\Query
SQLServer:Workload Group Stats\Blocked tasks
SQLServer:Workload Group Stats\CPU usage %
SQLServer:Workload Group Stats\Queued requests
SQLServer:Workload Group Stats\Requests completed/sec

Actual behavior:

The above metrics are written as Tags by default.

Additional info:

The above metrics being written as Tags means a) extremely high cardinality with metrics like Disk Write Bytes/sec or Used memory (and many others) and b) no InfluxQL operations can be done on them.

@ssoroka ssoroka added area/sqlserver feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin performance problems with decreased performance or enhancements that improve performance labels May 15, 2020
@danielnelson danielnelson added feature request Requests for new plugin and for new features to existing plugins and removed feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin labels May 15, 2020
@Trovalo
Copy link
Collaborator

Trovalo commented May 21, 2020

What do you mean exactly with The above metrics are written as Tags by default? (all those are tag values, not tag keys)

I agree with the fact that the cardinality and also the number of points will be lower if the tag "counter_name" gets pivoted and turned into a field, but the current structure does not prevent you from querying those points, it just takes more work to do so since you have to use a query that filters only one "counter_name" at a time (so you need multiple queries) or in other cases you need to include "counter_name" in the group by.

Just to add some info, the "sqlserver_performance" measurement has the following structure.

measurement sql_instance database_name object counter instance value
sqlserver_performance QDLP03:SQL2019 master MSSQL$SQL2019:Access Methods Forwarded Records/sec 0
sqlserver_performance QDLP03:SQL2019 master MSSQL$SQL2019:Resource Pool Stats Disk Write IO/sec default 0
sqlserver_performance QDLP03:SQL2019 master MSSQL$SQL2019:Resource Pool Stats Disk Write IO/sec internal 0

Where the column "value" is the only field. The telegraf measurement has the same structure of the table returned by SQL Server.

In order to obtain what is asked the pivot function can be used in the query itself but it can also be applied in telegraf using the pivot processor

@danielnelson
Copy link
Contributor

Depending on how you measure cardinality it is the same either way. You trade a unique tag value for a new field key. In the end there is the same number of series in the database.

Normally we avoid this naming style though, it can be more expensive to transmit the data due to duplication of measurements+tags+timestamp, and it doesn't give you room to grow with additional fields. The field key should contain a descriptive name of the item, so this doesn't follow best practices in that respect. We would like to always use tags and fields consistently across plugins and this doesn't meet those criteria either.

That said, this was discussed back in #3233 and there is rationale for allowing it in this case there. We can keep track of the issue in case we redesign this query in the future and would like to change the layout, but I don't think its worth the disruption to make modifications only for this reason.

@samhld
Copy link
Contributor Author

samhld commented May 21, 2020

@Trovalo That's fair. What I thought I saw (unable to reproduce as I don't have access to a SQL server, myself) was that things (what I would consider a metric) like this, Disk Read IO/sec, were being written as Tag Keys. If they're Tag Values, that resolves the issue with cardinality blowing up. The points that @danielnelson brought up on the naming scheme are both valid but if there were original reasons for following the schema it currently follows (in #3233), then probably good to close this one out!

@samhld samhld closed this as completed May 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/sqlserver feature request Requests for new plugin and for new features to existing plugins performance problems with decreased performance or enhancements that improve performance
Projects
None yet
Development

No branches or pull requests

4 participants