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

Adding Database IO Tempdb per Azure DB collection, Fixing PerfmonV2 collection for on-prem #7163

Merged
merged 3 commits into from
Mar 16, 2020

Conversation

denzilribeiro
Copy link
Contributor

@denzilribeiro denzilribeiro commented Mar 12, 2020

Required for all PRs:

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

Datababase IO changes
Today on Azure SQL DB there was a bug where left outer join from virtual file stats and database_files was only on file_id. Tempdb was exposed on virtual file stats, but not in Database files. On Azure SQL DB have to use tempdb.sys.database_files. In addition each Azure SQL DB has its own tempdb so needed to add Database_id/File ID to be able to distinguish so that reporting can be done by Logical Server + Azure DB + database_id. If you just looked at tempdb for a logical server would get multiple tempdb's belonging to different Azure SQL DB's as the logical server construct is just that a placeholder logical server not actual physical server.
Also corrected bug introduced earlier

PerfmonV2 Fix for #7164

@denzilribeiro
Copy link
Contributor Author

@Trovalo overlapped with your PR #7161 just submitted as I was in process of submitting didn't realize - can you take a look at this one and see if addresses issues as I also added additional logic to pull tempdb IO for Azure SQL so is a bit more than the bug fix

@Trovalo
Copy link
Collaborator

Trovalo commented Mar 12, 2020

I will have a look at it in a few minutes.

But I found a bigger problem while testing the whole plugin on multiple versions earlier.
While testing on SQL 2012. SQL 2019 and Azure SQL DB, I get an error

I! Invalid column name 'physical_database_name'.
E! [inputs.sqlserver] Error in plugin: mssql: Invalid column name 'physical_database_name'.

It is caused by the latest changes (January) made to the query and not yet published (in fact those are not in telegraf 1.13.4).
The field [physical_database_name] exists only in azure SQL DB and SQL Server 2019. Any other version will fail to gather the metrics due to a validation error.

I'm opening an issue now, the best solution probably is to add dynamic SQL, so it is not validated by SQL Server until it has to execute it

@denzilribeiro
Copy link
Contributor Author

I will add the dynamic SQL and resubmit shortly

@denzilribeiro
Copy link
Contributor Author

@Trovalo - actually go ahead and open an issue for that as is in a different part not IO :) misunderstood

@Trovalo
Copy link
Collaborator

Trovalo commented Mar 12, 2020

I did it few minutes ago
#7164

Copy link
Collaborator

@Trovalo Trovalo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've cloned and tested this on SQL 2012, SQL 2019 and Azure SQL DB (even if the issue was only on the last one).
It works perfectly.

@denzilribeiro
Copy link
Contributor Author

@Trovalo - one last validation - pushed commit for the Perfmon counters too

@denzilribeiro denzilribeiro changed the title Database IO - adding Tempdb per Azure DB collection Adding Database IO Tempdb per Azure DB collection, Fixing PerfmonV2 collection for on-prem Mar 12, 2020
@Trovalo
Copy link
Collaborator

Trovalo commented Mar 12, 2020

I will run the tests tomorrow morning and let you know

@danielnelson danielnelson added area/sqlserver fix pr to fix corresponding bug labels Mar 12, 2020
@danielnelson danielnelson added this to the 1.14.0 milestone Mar 12, 2020
Copy link
Collaborator

@Trovalo Trovalo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested the latest changes and everything works fine from SQL 2012 to SQL 2019 (On-Prem) and also on Azure SQL DB.

You still get an error in the 'Scheduler' query which is not compatible before SQL 2016 but there is already an issue and related PR for that. (see #7144)

@danielnelson danielnelson merged commit 0a75dea into influxdata:master Mar 16, 2020
@danielnelson danielnelson added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Mar 16, 2020
athoune pushed a commit to bearstech/telegraf that referenced this pull request Apr 17, 2020
veorlo pushed a commit to GlobalNOC/telegraf that referenced this pull request May 4, 2020
HarshitOnGitHub pushed a commit to HarshitOnGitHub/telegraf that referenced this pull request May 8, 2020
idohalevi pushed a commit to idohalevi/telegraf that referenced this pull request Sep 29, 2020
arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/sqlserver feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin fix pr to fix corresponding bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants