Fixing build for merge and move IIS check for --w3c-time-taken-milli …

…to Parser.check_format so only one warning outputted.
d19dotca · Dec 15, 2014 · 1dbec81 · 1dbec81
1 parent 04f64ac
commit 1dbec81
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 19 deletions.
diff --git a/misc/log-analytics/import_logs.py b/misc/log-analytics/import_logs.py
@@ -226,20 +226,17 @@ def check_format(self, file):
             file.seek(0)
             return
 
+        # store the header lines for a later check for IIS
+        self.header_lines = header_lines
+
         # Parse the 4th 'Fields: ' line to create the regex to use
         full_regex = []
 
         expected_fields = type(self).fields.copy() # turn custom field mapping into field => regex mapping
 
-        # if the --w3c-time-taken-millisecs option is used, make sure the time-taken field is interpreted as seconds
+        # if the --w3c-time-taken-millisecs option is used, make sure the time-taken field is interpreted as milliseconds
         if config.options.w3c_time_taken_in_millisecs:
             expected_fields['time-taken'] = '(?P<generation_time_milli>[\d.]+)'
-        else:
-            # check if we're importing IIS logs and if so, issue a warning TODO: should be done after correct format found so only one warning made
-            if 'generation_time_milli' not in expected_fields['time-taken'] and self._is_iis(header_lines):
-                logging.info("WARNING: IIS log file being parsed without --w3c-time-taken-milli option. IIS"
-                             " stores millisecond values in the time-taken field. If your logfile does this, the aforementioned"
-                             " option must be used in order to get accurate generation times.")
 
         for mapped_field_name, field_name in config.options.custom_w3c_fields.iteritems():
             expected_fields[mapped_field_name] = type(self).fields[field_name]
@@ -259,8 +256,17 @@ def check_format(self, file):
         file.seek(0)
         return self.check_format_line(first_line)
 
-    def _is_iis(self, header_lines):
-        return len([line for line in header_lines if 'internet information services' in line.lower() or 'iis' in line.lower()]) > 0
+    def check_for_iis_option(self):
+        if not config.options.w3c_time_taken_in_millisecs and self._is_time_taken_milli() and self._is_iis():
+            logging.info("WARNING: IIS log file being parsed without --w3c-time-taken-milli option. IIS"
+                         " stores millisecond values in the time-taken field. If your logfile does this, the aforementioned"
+                         " option must be used in order to get accurate generation times.")
+
+    def _is_iis(self):
+        return len([line for line in self.header_lines if 'internet information services' in line.lower() or 'iis' in line.lower()]) > 0
+
+    def _is_time_taken_milli(self):
+        return 'generation_time_milli' not in self.regex.pattern
 
 class IisFormat(W3cExtendedFormat):
 
@@ -1607,6 +1613,11 @@ def check_format(lineOrFile):
             else:
                 logging.debug('Format %s does not match', name)
 
+        # if the format is W3cExtendedFormat, check if the logs are from IIS and if so, issue a warning if the
+        # --w3c-time-taken-milli option isn't set
+        if isinstance(format, W3cExtendedFormat):
+            format.check_for_iis_option()
+
         return format
 
     @staticmethod

diff --git a/tests/PHPUnit/System/expected/test_ImportLogs__Live.getLastVisitsDetails_range.xml b/tests/PHPUnit/System/expected/test_ImportLogs__Live.getLastVisitsDetails_range.xml
@@ -1867,6 +1867,7 @@
 		<referrerUrl />
 		<referrerSearchEngineUrl />
 		<referrerSearchEngineIcon />
+		<resolution>unknown</resolution>
 		<continent>Unknown</continent>
 		<continentCode>unk</continentCode>
 		<country>Unknown</country>
@@ -1878,7 +1879,6 @@
 		<location>Unknown</location>
 		<latitude />
 		<longitude />
-		<resolution>unknown</resolution>
 		<plugins />
 		<pluginsIcons />
 		<visitLocalTime>01:13:11</visitLocalTime>
@@ -2660,6 +2660,7 @@
 		<referrerUrl />
 		<referrerSearchEngineUrl />
 		<referrerSearchEngineIcon />
+		<resolution>unknown</resolution>
 		<continent>Unknown</continent>
 		<continentCode>unk</continentCode>
 		<country>Unknown</country>
@@ -2671,7 +2672,6 @@
 		<location>Unknown</location>
 		<latitude />
 		<longitude />
-		<resolution>unknown</resolution>
 		<plugins />
 		<pluginsIcons />
 		<visitLocalTime>23:51:21</visitLocalTime>
@@ -2753,6 +2753,7 @@
 		<referrerUrl />
 		<referrerSearchEngineUrl />
 		<referrerSearchEngineIcon />
+		<resolution>unknown</resolution>
 		<continent>Unknown</continent>
 		<continentCode>unk</continentCode>
 		<country>Unknown</country>
@@ -2764,7 +2765,6 @@
 		<location>Unknown</location>
 		<latitude />
 		<longitude />
-		<resolution>unknown</resolution>
 		<plugins />
 		<pluginsIcons />
 		<visitLocalTime>23:51:20</visitLocalTime>

diff --git a/tests/PHPUnit/System/expected/test_ImportLogs__Resolution.getConfiguration_month.xml b/tests/PHPUnit/System/expected/test_ImportLogs__Resolution.getConfiguration_month.xml
@@ -44,6 +44,17 @@
 		<sum_daily_nb_uniq_visitors>2</sum_daily_nb_uniq_visitors>
 		<sum_daily_nb_users>1</sum_daily_nb_users>
 	</row>
+	<row>
+		<label>Unknown / Unknown / unknown</label>
+		<nb_visits>3</nb_visits>
+		<nb_actions>3</nb_actions>
+		<max_actions>1</max_actions>
+		<sum_visit_length>0</sum_visit_length>
+		<bounce_count>3</bounce_count>
+		<nb_visits_converted>0</nb_visits_converted>
+		<sum_daily_nb_uniq_visitors>3</sum_daily_nb_uniq_visitors>
+		<sum_daily_nb_users>0</sum_daily_nb_users>
+	</row>
 	<row>
 		<label>Android / Android Browser / unknown</label>
 		<nb_visits>2</nb_visits>
@@ -111,13 +122,13 @@
 		<sum_daily_nb_users>0</sum_daily_nb_users>
 	</row>
 	<row>
-		<label>Unknown / Unknown / unknown</label>
+		<label>Mac / Unknown / unknown</label>
 		<nb_visits>1</nb_visits>
 		<nb_actions>1</nb_actions>
 		<max_actions>1</max_actions>
 		<sum_visit_length>0</sum_visit_length>
 		<bounce_count>1</bounce_count>
-		<nb_visits_converted>0</nb_visits_converted>
+		<nb_visits_converted>1</nb_visits_converted>
 		<sum_daily_nb_uniq_visitors>1</sum_daily_nb_uniq_visitors>
 		<sum_daily_nb_users>0</sum_daily_nb_users>
 	</row>

diff --git a/tests/PHPUnit/System/expected/test_ImportLogs__Resolution.getResolution_month.xml b/tests/PHPUnit/System/expected/test_ImportLogs__Resolution.getResolution_month.xml
@@ -2,13 +2,13 @@
 <result>
 	<row>
 		<label>unknown</label>
-		<nb_visits>29</nb_visits>
-		<nb_actions>34</nb_actions>
+		<nb_visits>32</nb_visits>
+		<nb_actions>37</nb_actions>
 		<max_actions>3</max_actions>
 		<sum_visit_length>550</sum_visit_length>
-		<bounce_count>25</bounce_count>
-		<nb_visits_converted>27</nb_visits_converted>
-		<sum_daily_nb_uniq_visitors>28</sum_daily_nb_uniq_visitors>
+		<bounce_count>28</bounce_count>
+		<nb_visits_converted>28</nb_visits_converted>
+		<sum_daily_nb_uniq_visitors>31</sum_daily_nb_uniq_visitors>
 		<sum_daily_nb_users>2</sum_daily_nb_users>
 	</row>
 	<row>