Non textual field string data in sqlserver plugin with SqlRequests query

[danielnelson]

Relevant telegraf.conf:

[[inputs.sqlserver]]
  servers = ["..."]
  query_version = 2

[[outputs.file]]
  files = ["tmp"]

System info:

Telegraf 1.12.5

Steps to reproduce:

1. Run input plugin without excluding the SqlRequests query.
2. Check output file.

Expected behavior:

All string fields are valid UTF-8 text.

Actual behavior:

The query_hash and query_plan_hash appear to be binary data.

Additional info:

Small selection of example output:
input-filter.txt

[danielnelson]

@denzilribeiro @m82labs On this one it seems like we should just remove these two fields, can you weight in?

[denzilribeiro]

We can remove those, will be harder to correlate to actual Query plans in the database if we do. I didn't realize couldn't have binary data sorry

[danielnelson]

Maybe it would be best if we had these fields base64 encoded or hashed to a UUID?

[Trovalo]

if the problem depends only on the datatype the below solution may work, if it's related to GO or some sort string escape/processing problem maybe the "Second solution" section will solve it

First Solution
those two columns are of type "binary" in SQL Server, a simple CONVERT should do the trick,

#what the query does now:
	, r.query_hash
	, r.query_plan_hash

#what should it do
	,CONVERT(varchar(100),[query_hash],1) as [query_hash]
	,CONVERT(varchar(100),[query_plan_hash],1) as [query_plan_hash]

Second Solution
This is more an approximation since it truncates the 0x in front of the string, but is better than nothing:
CONVERT(varchar(100), column, 1) will keep the 0x in front of the string
CONVERT(varchar(100), column, 2) will remove the 0x in front of the string

personally I think that those fields are useful, especially on a production system like an ERP, where you don't have custom queries, therefore the query_hash is always the same and allows you to identify a single query (and then get it's text by querying the database using the hash). the same is valid for the execution plan, which may change sometimes but not that often.

a small addition:
right now those columns are fields and not tags, but I think that those will be more useful as tags since you may want to group by query or filter just one query.

[danielnelson]

I think it would make sense to do this in the query, the particular way we encode the data isn't of great importance it just needs to be valid utf-8, either a hex string (with or without 0x) or base64 would be fine for me.

On the subject of field vs tags, we should consider the cardinality. Is the relationship between statement_text, query_hash, and query_plan_hash always 1:1 (within the limits the hash length)?

[denzilribeiro]

query_hash to statement_text is 1:1. query_hash to query_plan_hash is 1:M
Given all new versions have query_store, I think getting query_hash is good enough as can correlate to a plan in query_store, can remove query_plan_hash totally.