Issue 395 - Filter hits statistics backend

sitescripts/filterhits/common.py

Index: sitescripts/filterhits/common.py
diff --git a/sitescripts/filterhits/common.py b/sitescripts/filterhits/common.py
index 8c3a651cd032c8e108de370141fbecd11000cf7e..1e807339f0b79f861531289ca0f472fb5e798f6e 100644
--- a/sitescripts/filterhits/common.py
+++ b/sitescripts/filterhits/common.py
@@ -1,7 +1,7 @@
 # coding: utf-8
 
 # This file is part of the Adblock Plus web scripts,
-# Copyright (C) 2006-2014 Eyeo GmbH
+# Copyright (C) 2006-2015 Eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
@@ -15,66 +15,34 @@
 # You should have received a copy of the GNU General Public License
 # along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
 
-from datetime import datetime
-import json, tempfile, os
+from errno import EEXIST
+import json, tempfile, time, os
 
 def showError(message, start_response, status="400 Processing Error"):
   start_response(status, [("Content-Type", "text/plain; charset=utf-8")])
   return [message.encode("utf-8")]
 
-def valid_filter_hit(filter_hit):
-  return (
-    isinstance(filter_hit, dict) and
-    "thirdParty" in filter_hit and
-    isinstance(filter_hit["thirdParty"], dict) and
-    "firstParty" in filter_hit and
-    isinstance(filter_hit["firstParty"], dict) and
-    "subscriptions" in filter_hit and
-    isinstance(filter_hit["subscriptions"], (list, tuple))
-  )
-
-def valid_log_data(data):
-  """
-    This returns True if the filterhits data passed is structured
-    roughly OK. Used as a quick check, it's not comprehensive.
-  """
-  return (
-    isinstance(data, dict) and
-    "version" in data and
-    "timeSincePush" in data and
-    "addonName" in data and
-    "addonVersion" in data and
-    "application" in data and
-    "applicationVersion" in data and
-    "platform" in data and
-    "platformVersion" in data and
-    "filters" in data and
-    isinstance(data["filters"], dict) and
-    (not len(data["filters"]) or
-     valid_filter_hit(data["filters"].itervalues().next()))
-  )
-
-def datetime_to_timestamp(dt):
-  return int((dt - datetime(1970, 1, 1)).total_seconds())
-
 def log_filterhits(data, basepath, query_string):
   """
-    This logs the provided filterhits data as JSON to a file named after
-    the current timestamp in a directory named after the current date.
+  This logs the provided filterhits data as JSON to a file named after
+  the current timestamp in a directory named after the current date.
   """
-  now = datetime.now()
+  now = time.gmtime()

[Sebastian Noack, 2015/02/17 14:59:17]

For reference, this will be UTC. I don't have a strong opinion whether UTC or
local timezone is more appropriate for local filenames.

[Wladimir Palant, 2015/02/17 15:12:21]

Local timezone is always a bad idea - already because of daylight saving time,
there is one hour each year where the same "timestamp" can refer to two
different points in time. Not that it should matter much as the "local" time for
all our servers is UTC, for that very reason.

[Sebastian Noack, 2015/02/17 15:19:44]

I generally agree. Though I wasn't entirely sure in this particular case, since
from a sysadmin perspective it might be confusing to have files with names based
on the time in a different time zone. Log files on UNIX machines usually use the
local timezone. But I don't mind.
Mind leap seconds? So as long as you don't run your server with GPS time, you
have that issue anyway. ;P

 
-  dir_name = now.strftime("%Y-%m-%d")
+  dir_name = time.strftime("%Y-%m-%d", now)
   path = os.path.join(basepath, dir_name)
-  if not os.path.exists(path):
+  try:
     os.makedirs(path)
+  except OSError as e:
+    if e.errno != EEXIST:
+      raise
 
   with tempfile.NamedTemporaryFile(
-    prefix = str(datetime_to_timestamp(now)) + "-",
+    prefix = str(int(time.mktime(now))) + "-",
     suffix = ".log",
     dir = path,
     delete = False
   ) as f:
-    f.write("[%s] \"%s\" %s\n" % (now.strftime('%d/%b/%Y:%H:%M:%S %z'),
+    f.write("[%s] \"%s\" %s\n" % (time.strftime('%d/%b/%Y:%H:%M:%S', now),
                                   query_string, json.dumps(data)))
     return f.name