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

Include healthcheck results from sp_blitz #353

Open
martin-guth opened this Issue Mar 5, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@martin-guth
Copy link

martin-guth commented Mar 5, 2018

I already use sp_blitz from FirstResponderkit. Would it be possible to include a pester check in dbachecks to check for alerts of a specific priority? Then there would be just one place to go to look for something odd instead of multiple.

@SQLDBAWithABeard

This comment has been minimized.

Copy link
Collaborator

SQLDBAWithABeard commented Mar 5, 2018

It absolutely would be possible. Could you expand on what you would require please

@martin-guth

This comment has been minimized.

Copy link

martin-guth commented Mar 6, 2018

Cool. Well sp_blitz returns a table with different columns.
Here is an example:
grafik

I would suggest the following columns to be relevant for filtering in a check:

  • Priority: Numbers >= 200 are informational and could be ignored by default
  • FindingsGroup: Contains values like 'Performance', 'Reliability' and so on. It could make sense to provide a config value for whitelisting these values. An example would be "Performance, Reliability" to only filter entries of these two groups.
  • Finding: Contains a description of the actual potential problem like "Active Tables Without Clustered Indexes". It could make sense to blacklist some of these entries as well.
  • DatabaseName: Contains the name of the database...maybe you would like to ignore results for some databases. This property would be similar like the existing property policy.invaliddbowner.excludedb

I guess it gets really complicated if someone would like to combine filters like ignore finding "Active Tables Without Clustered Indexes" but just for database xyz.

The error message could be something like:

Expected $null, because sp_blitz finding could relate to a potential problem but got "The [xyz] database has heaps - tables without a clustered index - that are being actively queried.".
The text in qoutes then comes from the column Details in the sp_blitz result.

One potential downside of integrating sp_blitz would be that some checks are already existent in dbachecks as native checks (for example check for database owner or backups) and thus could lead to duplicate error message or at worst inconsistent error messages where dbachecks considers a different approach as best practice than sp_blitz does. However I would value the use of having more checks and benefitting from sp_blitz health checks as well much higher than the inconvenience of duplicate checks or different approaches. In the end I believe each tool has its right to exist on its own and together they could offer more value.

@michalporeba michalporeba self-assigned this Mar 10, 2018

@michalporeba

This comment has been minimized.

Copy link
Collaborator

michalporeba commented Mar 10, 2018

I've been thinking about it myself. I will have a look at it today.

@michalporeba

This comment has been minimized.

Copy link
Collaborator

michalporeba commented Mar 10, 2018

I have started the work already. IF you are interested it is here
https://github.com/michalporeba/dbachecks/tree/frk

My plan is to implement checks using sp_blitz first in a new checks file, just to keep them separate until they 'mature' a bit. Once we are happy with how they are tested and how they work I think the next step will be to figure out how to include them in appropriate categories, and perhaps based on configuration and whether the first responder kit is installed or not, to use them in stead of the original ones rather than duplicate the checks.

Main reason I took this approach is because a lot of those checks will be eventually in the Database.Tests.ps1 file and I'm already working (waiting for comments) on making big changes to it. So this temporary approach with a new file FirstResponderKit.Tests.ps1 helps me separate the work and avoid blocking. Still, even when we move most of the new checks to the correct files, there still will be few that don't fit anywhere else but in the FirstResponderKit.Tests.ps1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment