[WMI] Switching IIS and Win Service Checks and Win32EventLog check to win32com. WMI refactor. #2136

truthbk · 2015-12-08T23:40:23Z

WMI via `win32com` for IIS, Service Checks and Win32EventLog

This PR aims to replace the older wmi module and use the WMI sampler based off win32com provided by @yann and @TheCloudlessSky in #2011 .
Because there is plenty of common code a new WinWMICheck class was introduced that is a parent to all our wmirelated checks:

System
IIS
Windows Service
Windows Event Log

Tests have been re-written to support the new current code layout and behavior, and a totally new test has been introduced for the windows event log check.

yannmh · 2015-12-11T20:33:38Z

checks/libs/wmi/sampler.py

        """
        Transform filters to a comprehensive WQL `WHERE` clause.
        """
-        def build_where_clause(fltr):
+        def build_where_clause(fltr, inclusive):


You don't change the value of inclusive across your recursive calls, so I guess you don't need to have this argument redefined in build_where_clause but only in _format_filter.

yannmh · 2015-12-14T18:16:56Z

Great changes @truthbk ! Thanks for taking care of it.
Can't wait to see it merged and drop wmi dependency once and for all.

It's a significant size PR, so i added a lot of nitpicks and questions 😄

yannmh · 2015-12-28T15:43:08Z

For a reason that I haven't identified yet, the run_check command does not work on WinWMICheck

>>> from checks import run_check
>>> run_check('wmi_check')
Traceback (most recent call last):
  File "shell.py", line 14, in shell
  File "<string>", line 1, in <module>
  File "utils\debug.pyc", line 33, in run_check
  File "checks\__init__.pyc", line 790, in check
NotImplementedError

yannmh · 2015-12-28T16:03:56Z

Looks great @truthbk.

I've been running more tests to assess that we have no regression with the 5.6.x versions in metrics, events and service checks collections.

Summary

+===================+=============================+============+============+================+
| Check             | 5.6.x / 5.7.0 check runtime | Metrics    | Event      | Service checks |
+===================+=============================+============+============+================+
| System            | ~ 1                         | OK         | N/A        | N/A            |
+-------------------+-----------------------------+------------+------------+----------------+
| WMI               | ~ 1                         | OK         | N/A        | N/A            |
+-------------------+-----------------------------+------------+------------+----------------+
| IIS               | 0.70                        | OK         | N/A        | OK             |
+-------------------+-----------------------------+------------+------------+----------------+
| Windows Service   | 80                          | N/A        | N/A        | OK             |
+-------------------+-----------------------------+------------+------------+----------------+
| Windows Log event | Not tested                  | N/A        | OK         | N/A            |
+===================+=============================+============+============+================+

Just pending evaluation of the Windows Log event check runtime vs the 5.6.x version.

truthbk · 2016-01-07T23:08:22Z

After the changes to the filtering in the last commit we might need to conduct a couple more tests to check regressions aren't present. Nothing has changed effectively, we just now place the parentheses a little differently in the WQL - we avoid adding more between AND clauses, and we isolate OR clauses with them.

truthbk · 2016-01-14T23:44:41Z

Some things break after rebase due to issue with @yannmh timeout changes: #2185 - pending fix.

yannmh · 2016-01-15T15:12:32Z

checks.d/iis.py

-        ('iis.requests.cgi', 'rate', 'TotalCGIRequests'),
-        ('iis.requests.isapi', 'rate', 'TotalISAPIExtensionRequests'),
+        ('TotalCGIRequests', 'iis.requests.cgi', 'rate'),
+        ('TotalISAPIExtensionRequests', 'iis.requests.isapi', 'rate'),
    ]
    SERVICE_CHECK = "iis.site_up"


Looks like you don't use this variable.

I've changed the service_check() calls to use it.

yannmh · 2016-01-15T15:13:37Z

@truthbk I think IIS and Windows Service checks need a quick refresh to comply well with timeouts (c.f #2185).

yannmh · 2016-01-26T19:42:49Z

setup.py

@@ -52,7 +52,6 @@
    # Modules to force-include in the exe
    include_modules = [
        # 3p
-        'wmi',


Let's keep it for now

truthbk · 2016-01-26T20:57:49Z

TODO

@yannmh:

Raise WMITimeoutException when timeout ✅
Cache connections in list instead of set ✅
Gracefully handle WMITimeoutException ✅

@truthbk:

Remove HashableConnection ✅
Get rid of done() ✅
Handle use-case where we're still sampling for service-checks (handle WMITimeoutException). ✅

@yannmh Open for discussion: 246f26a - removal of caching as it makes no sense (currently) given the reinitialization of the connections when spawning a new thread.

olivielpeau · 2016-01-26T20:58:14Z

utils/containers.py

+    Accepts nested dictionaries.
+    """
+    if isinstance(o, dict):
+        return frozenset({k:freeze(v) for k,v in o.items()}.items())


python 2.6 doesn't support dict comprehension, you should change

{k:freeze(v) for k,v in o.items()}

to

dict([(k, freeze(v)) for k, v in o.items()])

(also, you can use iteritems() instead of items() in the list comprehension)

good feedback, thanks. Fixed.

olivielpeau · 2016-02-18T23:59:11Z

tests/core/test_wmi.py

+        format_filter = sampler.WMISampler._format_filter
+
+        # Check `_format_filter` logic
+        filters = [{'Name': "Foo%"}, {'Name': "Bar*", 'Id': ('>=', "SomeId")}, {'Name': "Zulu"}]


The wildcard character in WQL WHERE clauses is only % (not *), I've confirmed that by reading through https://technet.microsoft.com/en-us/library/ee176998.aspx and manually testing on a windows VM.

In terms of BC, the only check that was supporting the wildcard was the wmi_check (see https://github.com/DataDog/dd-agent/blob/5.5.x/checks.d/wmi_check.py#L51-L55), it was actually looking for a * in the filters and replacing it with a %.

Given that the wildcard feature was broken in 5.6.x I think we could simply only allow % as a wildcard in the filters. What do you think?

Actually the wildcard was also implemented in the event_log check, but w/o the weird *-> % replacement, so I think we have another argument in favor of only allowing the % wildcard in filters :)

olivielpeau · 2016-02-19T00:19:18Z

Went through all the filtering logic, only added one comment. It's great work! (huge ❤️ for all the tests)

olivielpeau · 2016-02-19T20:36:47Z

checks.d/iis.py

+
+    def _submit_metrics(self, wmi_metrics, metrics_by_property):
+        for m in wmi_metrics:
+            if m.name == "TotalBytesTransfered":


could we keep the comment explaining why we're doing this? :)

olivielpeau · 2016-02-19T22:29:58Z

Looking great! Had one tiny nitpick.

Could you squash your commits? Then feel free to merge once the CI passes.

[wmi] Refactoring to create WinWMICheck parent class. [wmi] Adding windows check to setup.py [wmi] win32 event log to use wmi_sampler. [wmi] Updating setup.py after name change. [wmi] Lists of elements for a property will OR for any of those values. [wmi] We could also have a TypeError when parsing. [wmi] Fixing tests, classes have moved. [wmi] adding test for win32 event log. [wmi] Normalize site names, send relevant tags. [wmi] Removing wmi, refactoring types, adding message filters. [wmi] Leaving types.py for now so tests don't pull in pywintypes. [wmi] cleaning up event log. [wmi] adding further tests for WQL query generation. [wmi] fixing tests now that we've dropped the wmi dependency. [wmi] fixing WQL tests. [wmi] fixing the test mocking, removing unnecessary types.py. [wmi] moving wmi.py to wmi_check.py. [wmi] Normalize unicode to ASCII w/ unicode normalization forms. [wmi] improving WQL clause generation and tests. [wmi] fixing filters to work better with the parsed yaml. Fixing regressions. [wmi] simplifying filter logic. Handle wildcard characters, and OR/AND situations. [wmi] adding and_props for property lists we should AND around. Improving tests. Additional filter tweaks. [wmi] removing remaining inclusive references. [wmi] fixing tests with new query formatting - parentheses changed. [wmi] eventlog types, source_names and log_files are also multiple entry fields. [wmi] fix to play better with context manager. [wmi] pythoncom.CoInitialize must now be called from every thread. [wmi] instance key has to actually be unique to every instance. [wmi] fixing import typos [wmi] fixing windows imports. [iis] use SERVICE_CHECK class variable. [WMI] Switching IIS and Win Service Checks to win32com. Refactor of WMI. [wmi] Refactoring to create WinWMICheck parent class. [wmi] Adding windows check to setup.py [wmi] win32 event log to use wmi_sampler. [wmi] Updating setup.py after name change. [wmi] Lists of elements for a property will OR for any of those values. [wmi] We could also have a TypeError when parsing. [wmi] Fixing tests, classes have moved. [wmi] adding test for win32 event log. [wmi] Normalize site names, send relevant tags. [wmi] Removing wmi, refactoring types, adding message filters. [wmi] Leaving types.py for now so tests don't pull in pywintypes. [wmi] cleaning up event log. [wmi] adding further tests for WQL query generation. [wmi] fixing tests now that we've dropped the wmi dependency. [wmi] fixing WQL tests. [wmi] fixing the test mocking, removing unnecessary types.py. [wmi] moving wmi.py to wmi_check.py. [wmi] Normalize unicode to ASCII w/ unicode normalization forms. [wmi] improving WQL clause generation and tests. [wmi] fixing filters to work better with the parsed yaml. Fixing regressions. [wmi] simplifying filter logic. Handle wildcard characters, and OR/AND situations. [wmi] adding and_props for property lists we should AND around. Improving tests. Additional filter tweaks. [wmi] removing remaining inclusive references. [wmi] fixing tests with new query formatting - parentheses changed. [wmi] eventlog types, source_names and log_files are also multiple entry fields. [wmi] fix to play better with context manager. [wmi] instance key has to actually be unique to every instance. [wmi] fixing import typos [wmi] fixing windows imports. [iis] use SERVICE_CHECK class variable. [wmi] Handle TimeoutException when sample()'ing from WMISampler. [wmi] CoInitialize() in every thread. [wmi] removing caching unnecessary in timeout/threaded scenario - we always need to create one. [wmi] make freeze() python2.6 compatible. Iterate w/ iteritems() instead of items(). [wmi] keeping wmi module in package for now. [wmi] fixing tests for non-caching wmi, keeping connection state in wmi sampler class for tests/future. [wmi] fixing timeout test. [wmi] in WQL only % is a wildcard symbol, killing *

[WMI] Switching IIS and Win Service Checks and Win32EventLog check to win32com. WMI refactor.

Objects returned by the WMISampler have a dictionnary structure. Fix regression introduced with #2136 when `tag_event_id:true`.

truthbk force-pushed the jaime/wmi_iis branch 2 times, most recently from fee440c to 5cf14c6 Compare December 9, 2015 23:01

truthbk changed the title ~~[WMI] Switching IIS and Win Service Checks to win32com. Refactor of WMI.~~ [WMI] Switching IIS and Win Service Checks and Win32EventLog check to win32com. WMI refactor. Dec 9, 2015

truthbk force-pushed the jaime/wmi_iis branch 2 times, most recently from 0cab943 to 1cb7e85 Compare December 10, 2015 20:11

yannmh self-assigned this Dec 10, 2015

yannmh reviewed Dec 11, 2015
View reviewed changes

truthbk force-pushed the jaime/wmi_iis branch from d5188b1 to 6d3f83f Compare December 21, 2015 21:14

remh added this to the 5.7.0 milestone Dec 22, 2015

truthbk force-pushed the jaime/wmi_iis branch from acd0e2a to 53f6a72 Compare December 22, 2015 17:55

truthbk force-pushed the master branch from f71173e to b90766b Compare December 22, 2015 18:05

truthbk closed this Dec 22, 2015

truthbk reopened this Dec 22, 2015

truthbk force-pushed the jaime/wmi_iis branch 4 times, most recently from df85f0c to f558f2c Compare December 22, 2015 23:58

truthbk force-pushed the jaime/wmi_iis branch 2 times, most recently from 320fa3d to 0474254 Compare January 7, 2016 23:01

truthbk force-pushed the jaime/wmi_iis branch from 0474254 to 115ea87 Compare January 7, 2016 23:15

truthbk force-pushed the jaime/wmi_iis branch from 6328f21 to f29aea1 Compare January 14, 2016 23:29

yannmh reviewed Jan 15, 2016
View reviewed changes

truthbk force-pushed the jaime/wmi_iis branch 5 times, most recently from 923c823 to d1dfe50 Compare January 21, 2016 22:46

yannmh reviewed Jan 26, 2016
View reviewed changes

setup.py

@@ -52,7 +52,6 @@

# Modules to force-include in the exe

include_modules = [

# 3p

'wmi',

Copy link

Member

yannmh Jan 26, 2016

Choose a reason for hiding this comment

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

Let's keep it for now

olivielpeau reviewed Jan 26, 2016
View reviewed changes

truthbk force-pushed the jaime/wmi_iis branch 2 times, most recently from e618091 to 6030672 Compare February 18, 2016 22:37

olivielpeau reviewed Feb 18, 2016
View reviewed changes

truthbk force-pushed the jaime/wmi_iis branch 2 times, most recently from b7a8f3e to 1369657 Compare February 19, 2016 17:49

olivielpeau reviewed Feb 19, 2016
View reviewed changes

truthbk force-pushed the jaime/wmi_iis branch from 2fad063 to 5422454 Compare February 19, 2016 22:34

truthbk added a commit that referenced this pull request Feb 21, 2016

Merge pull request #2136 from DataDog/jaime/wmi_iis

9cc8e0f

[WMI] Switching IIS and Win Service Checks and Win32EventLog check to win32com. WMI refactor.

truthbk merged commit 9cc8e0f into master Feb 21, 2016

truthbk deleted the jaime/wmi_iis branch February 21, 2016 20:11

yannmh added a commit that referenced this pull request Mar 30, 2016

[win32_event_log] fix tag_event_id:true 🐛

116216b

Objects returned by the WMISampler have a dictionnary structure. Fix regression introduced with #2136 when `tag_event_id:true`.

yannmh mentioned this pull request Mar 30, 2016

[win32_event_log] fix tag_event_id:true 🐛 #2397

Merged

olivielpeau pushed a commit that referenced this pull request Mar 30, 2016

[win32_event_log] fix tag_event_id:true 🐛

37f839b

Objects returned by the WMISampler have a dictionnary structure. Fix regression introduced with #2136 when `tag_event_id:true`.

olivielpeau mentioned this pull request Jun 13, 2016

[win32_event_log] Fix small regression on msg_text selection #2591

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WMI] Switching IIS and Win Service Checks and Win32EventLog check to win32com. WMI refactor. #2136

[WMI] Switching IIS and Win Service Checks and Win32EventLog check to win32com. WMI refactor. #2136

truthbk commented Dec 8, 2015

yannmh Dec 11, 2015

yannmh commented Dec 14, 2015

yannmh commented Dec 28, 2015

yannmh commented Dec 28, 2015

truthbk commented Jan 7, 2016

truthbk commented Jan 14, 2016

yannmh Jan 15, 2016

truthbk Jan 25, 2016

yannmh commented Jan 15, 2016

yannmh Jan 26, 2016

truthbk commented Jan 26, 2016

olivielpeau Jan 26, 2016

truthbk Feb 5, 2016

olivielpeau Feb 18, 2016

olivielpeau Feb 19, 2016

olivielpeau commented Feb 19, 2016

olivielpeau Feb 19, 2016

olivielpeau commented Feb 19, 2016

[WMI] Switching IIS and Win Service Checks and Win32EventLog check to win32com. WMI refactor. #2136

[WMI] Switching IIS and Win Service Checks and Win32EventLog check to win32com. WMI refactor. #2136

Conversation

truthbk commented Dec 8, 2015

WMI via win32com for IIS, Service Checks and Win32EventLog

Choose a reason for hiding this comment

yannmh commented Dec 14, 2015

yannmh commented Dec 28, 2015

yannmh commented Dec 28, 2015

Summary

truthbk commented Jan 7, 2016

truthbk commented Jan 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yannmh commented Jan 15, 2016

Choose a reason for hiding this comment

truthbk commented Jan 26, 2016

TODO

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olivielpeau commented Feb 19, 2016

Choose a reason for hiding this comment

olivielpeau commented Feb 19, 2016

WMI via `win32com` for IIS, Service Checks and Win32EventLog