1119110 - Add server-side metrics for sync1.1 migration tracking

Status: RESOLVED FIXED

(Cloud Services Graveyard :: Server: Sync, defect)

Description

[Ryan Kelly [:rfkelly]]

Attachment: sync11-migration-metrics.diff

This is a follow-up to Bug 1110013 in which we added logic to send end-of-life headers in sync1.1  We also want to log a bunch of counters so that we can track e.g. how many requests actually see a header, and how many take action to proceed with the migration.

Fortunately we already have metlog talking to statds on these machines, so it's a simple matter of sending additional counters.  This patch sends the following, which I hope are mostly self-explanatory:

  * syncstorage.migration.eol_header_considered
  * syncstorage.migration.eol_header_sent
  * syncstorage.migration.sentinel_read
  * syncstorage.migration.sentinel_write
  * syncstorage.migration.client_delete

Two subtleties worth pointing out:

  * "eol_header_considered" counts all requests by eligible devices, so before general release this will only count FF37+ desktop devices

  * "client_delete" only counts deletes where the request would also see an EOL header; this makes it more likely that the delete is due to the client cleaning up after a migration

Comment 1

[Tarek Ziadé (:tarek)]

Comment on attachment 8545680 [details] [diff] [review]
sync11-migration-metrics.diff

LGTM, just a few nitpicks in the diff below


>--- a/syncstorage/wsgiapp.py
>+++ b/syncstorage/wsgiapp.py
>@@ -61,16 +61,25 @@ except ImportError:
> #
> # During staged rollout of EOL headers, we only want to show them to
> # desktop machines on FF37 or later.  Matchin 37/38/39 should take us
> # through until this is on release, at which point we can disable
> # this check.
> 
> USER_AGENT_WITH_EOL_SUPPORT = re.compile("Firefox/3[789]\..*\.desktop")
> 
>+# Metlog counter names for use in tracking migration metrics.
>+
>+CTR_EOL_PREFIX = "syncstorage.migration."
>+CTR_EOL_HEADER_CONSIDERED = CTR_EOL_PREFIX + "eol_header_considered"
>+CTR_EOL_HEADER_SENT = CTR_EOL_PREFIX + "eol_header_sent"
>+CTR_EOL_SENTINEL_READ = CTR_EOL_PREFIX + "sentinel_read"
>+CTR_EOL_SENTINEL_WRITE = CTR_EOL_PREFIX + "sentinel_write"
>+CTR_EOL_CLIENT_DELETE = CTR_EOL_PREFIX + "client_delete"
>+
> 
> def _url(url):
>     for pattern, replacer in (('_API_', '{api:1.0|1|1.1}'),
>                               ('_COLLECTION_',
>                                '{collection:[a-zA-Z0-9._-]+}'),
>                               ('_USERNAME_',
>                                '{username:[a-zA-Z0-9._-]+}'),
>                               ('_ITEM_',
>@@ -258,54 +267,83 @@ class StorageServerApp(SyncServerApp):
> 
>     def _dispatch_request_with_match(self, request, match):
>         # Manage staged rollout of end-of-life headers.
>         # We need to do it in this method rather than _before_call(),
>         # because the logic needs access to the numeric userid.
> 
>         userid = request.user['userid']
> 
>+        # We're going to test the EOL headers while the corresponding
>+        # client code is still riding the trains.  Until general release, it's
>+        # only safe to show them to sufficiently-recent desktop devices.
>+
>+        if self.eol_general_release:
>+            is_eligible_user_agent = True
>+        else:
>+            user_agent = request.headers.get("User-Agent", "")

is_eligible_user_agent = USER_AGENT_WITH_EOL_SUPPORT.match(user_agent) is not None


>+            if USER_AGENT_WITH_EOL_SUPPORT.match(user_agent) is not None:
>+                is_eligible_user_agent = True
>+            else:
>+                is_eligible_user_agent = False
>+
>         # Show the EOL headers to the configured percentage of users,
>         # based on the low-order bits of their userid.  This is a simple
>         # way to ensure that users will consistently see headers on or off.
> 
>-        should_see_eol_headers = (userid % 100 < self.eol_rollout_percent)

Parenthesis preferred for py >2.5


         should_see_eol_headers = (is_eligible_user_agent and 
                                   userid % 100 < self.eol_rollout_percent)


>+        should_see_eol_headers = is_eligible_user_agent and \
>+                                 userid % 100 < self.eol_rollout_percent
> 
>-        # We're going to test the EOL headers while the corresponding
>-        # client code is still riding the trains.  Until general release, it's
>-        # only safe to show them to sufficiently-recent desktop devices with
>-        # no other clients attached to the account.
>+        # Before general release, we only want to show the headers to users
>+        # with a single device connected.  Sadly, the only way to check this
>+        # is by doing a massive layering violation and peeking at the DB.
> 
>         if should_see_eol_headers and not self.eol_general_release:
>-            user_agent = request.headers.get("User-Agent", "")
>-            if not USER_AGENT_WITH_EOL_SUPPORT.match(user_agent):
>+            storage = self.get_storage(request)
>+            clients = storage.get_items(userid, "clients", limit=2)

if there's not more else, then a shrter version is:

should_see_eol_headers = len(clients) <= 1


>+            if len(clients) > 1:
>                 should_see_eol_headers = False
>-            else:
>-                # Check in the DB whether this user has multiple devices.
>-                # It's a massive layering violation, but gets the job done!
>-                storage = self.get_storage(request)
>-                clients = storage.get_items(userid, "clients", limit=2)
>-                if len(clients) > 1:
>-                    should_see_eol_headers = False
> 
>         # If we ever make the eol_rollout_percent smaller, it will mean that
>         # some users stop seeing EOL headers where previously they were seeing
>         # them.  This could interfere with in-progress migrations, so we need
>         # to check whether those users have already started the process, and
>         # keep sending headers if so.
> 
>         if not should_see_eol_headers:
>             if self.eol_rollout_percent_hwm > self.eol_rollout_percent:
>                 if userid % 100 < self.eol_rollout_percent_hwm:
>                     # Check in the DB whether this user has started migrating.
>                     # It's a massive layering violation, but gets the job done!
>                     storage = self.get_storage(request)
>                     if storage.get_item(userid, "meta", "fxa_credentials"):
>                         should_see_eol_headers = True
> 
>+        # Log some metrics to help track progress and performance of migration:
>+        #  * count of requests for which we considered sending the header
>+        #  * count of requests for which we actually sent the header
>+        #  * count of reads and writes to the migration sentinel
>+        #  * count of client record cleanups by users who received a header
>+
>+        if is_eligible_user_agent:
>+            self.logger.incr(CTR_EOL_HEADER_CONSIDERED)
>+        if should_see_eol_headers:
>+            self.logger.incr(CTR_EOL_HEADER_SENT)
>+        if request.method == "GET":
>+            if match.get("collection") == "meta":
>+                if match.get("item") == "fxa_credentials":
>+                    self.logger.incr(CTR_EOL_SENTINEL_READ)
>+        elif request.method in ("PUT", "POST"):
>+            if match.get("collection") == "meta":
>+                if match.get("item") == "fxa_credentials":
>+                    self.logger.incr(CTR_EOL_SENTINEL_WRITE)
>+        elif request.method == "DELETE":
>+            if should_see_eol_headers:
>+                self.logger.incr(CTR_EOL_CLIENT_DELETE)
>+
>         # Continue with request dispatch, ensuring that we set the appropriate
>         # headers on both success and failure responses.
> 
>         supercls = super(StorageServerApp, self)
>         try:
>             response = supercls._dispatch_request_with_match(request, match)
>         except HTTPException, response:
>             if should_see_eol_headers:

Comment 2

[Ryan Kelly [:rfkelly]]

http://hg.mozilla.org/services/server-storage/rev/df48228512dc