Clarify use of 'FCM' in app type and refactor to use 'FCM'

CHANGELOG.md

@@ -1,3 +1,19 @@
+This release deprecates the use of gcm as an app type and refactors Sygnal to use fcm as an app type exclusively. Due to these changes, some prometheus metrics names were updated to refer to Firebase Cloud Messaging (fcm) instead of the legacy Google Cloud Messaging (GCM) services. You can find a list of the changed metric names below:
+
+    sygnal_gcm_queue_time -> sygnal_fcm_queue_time
+    sygnal_gcm_request_time -> sygnal_fcm_request_time
+    sygnal_pending_gcm_requests -> sygnal_pending_fcm_requests
+    sygnal_active_gcm_requests -> sygnal_active_fcm_requests
+    sygnal_gcm_status_codes -> sygnal_fcm_status_codes
+
+Additionally, tag identifiers in OpenTracing Spans were also updated:
+
+    gcm_error -> fcm_error
+    gcm_num_devices -> fcm_num_devices
+    gcm_num_failed -> fcm_num_failed
+
+If you make use of either Prometheus (and thus Grafana, etc) metrics or use OpenTracing with Sygnal, please ensure your queries are up to date.
+
 Sygnal 0.10.1 (2021-08-16)
 ==========================
 

README.rst

@@ -63,11 +63,14 @@ apns
 
     * the 'platform' parameter which determines whether the production or sandbox APNS environment is used. Valid values are 'production' or 'sandbox'. If not provided, 'production' is used.
 
-gcm
-  This sends messages via Google/Firebase Cloud Messaging (GCM/FCM) and hence can be used
+fcm
+  This sends messages via Firebase Cloud Messaging (FCM) and hence can be used
   to deliver notifications to Android apps. It expects the 'api_key' parameter
   to contain the 'Server key', which can be acquired from Firebase Console at:
   ``https://console.firebase.google.com/project/<PROJECT NAME>/settings/cloudmessaging/``
+  Please note that the legacy key 'gcm' can be used here in place of fcm, however,
+  both keys refer to Firebase Cloud Messaging. The gcm key is deprecated and
+  will be dropped in future releases.
 
 
 Using an HTTP Proxy for outbound traffic

changelog.d/251.misc

@@ -0,0 +1 @@
+Addresses #230, refactors sygnal to use fcm as app type rather than gcm, and warns in documents about the deprecation of gcm as an app type.

docs/troubleshooting.md

@@ -120,7 +120,7 @@ If you are using Firebase for your iOS app, you will get Firebase tokens
 (looking a bit like `blahblahblah:APA91blahblahblah`… note the presence of a
 colon which is not valid base64).
 
-In this case, you need to **configure Sygnal to use a FCM (gcm) pushkin rather
+In this case, you need to **configure Sygnal to use a FCM pushkin rather
 than an APNs one, as Firebase talks to APNs on your behalf**.
 Instead of configuring Sygnal with your APNs secrets, you need to configure
 Firebase with your APNs secrets, and Sygnal with your Firebase secrets.

sygnal.yaml.sample

@@ -198,13 +198,16 @@ apps:
   #  # The default is 'production'. Uncomment to use the sandbox instance.
   #  #platform: sandbox
 
-  # This is an example GCM/FCM push configuration.
+  # This is an example FCM/GCM push configuration. Note that the gcm type is a legacy
+  # setting as we now use Firebase Cloud Messaging (fcm) exclusively. Setting gcm as the
+  # app type is still accepted but understand that under the hood we are using fcm.
+  # The gcm app type is deprecated.
   #
   #com.example.myapp.android:
-  #  type: gcm
-  #  api_key: your_api_key_for_gcm
+  #  type: fcm or gcm
+  #  api_key: your_api_key_for_fcm
   #
-  #  # This is the maximum number of connections to GCM servers at any one time
+  #  # This is the maximum number of connections to FCM servers at any one time
   #  # the default is 20.
   #  #max_connections: 20
   #

sygnal/gcmpushkin.py → sygnal/fcmpushkin.py

@@ -37,38 +37,38 @@
 from .notifications import ConcurrencyLimitedPushkin
 
 QUEUE_TIME_HISTOGRAM = Histogram(
-    "sygnal_gcm_queue_time", "Time taken waiting for a connection to GCM"
+    "sygnal_fcm_queue_time", "Time taken waiting for a connection to FCM"
 )
 
 SEND_TIME_HISTOGRAM = Histogram(
-    "sygnal_gcm_request_time", "Time taken to send HTTP request to GCM"
+    "sygnal_fcm_request_time", "Time taken to send HTTP request to FCM"
 )
 
 PENDING_REQUESTS_GAUGE = Gauge(
-    "sygnal_pending_gcm_requests", "Number of GCM requests waiting for a connection"
+    "sygnal_pending_fcm_requests", "Number of FCM requests waiting for a connection"
 )
 
 ACTIVE_REQUESTS_GAUGE = Gauge(
-    "sygnal_active_gcm_requests", "Number of GCM requests in flight"
+    "sygnal_active_fcm_requests", "Number of FCM requests in flight"
 )
 
 RESPONSE_STATUS_CODES_COUNTER = Counter(
-    "sygnal_gcm_status_codes",
-    "Number of HTTP response status codes received from GCM",
+    "sygnal_fcm_status_codes",
+    "Number of HTTP response status codes received from FCM",
     labelnames=["pushkin", "code"],
 )
 
 logger = logging.getLogger(__name__)
 
-GCM_URL = b"https://fcm.googleapis.com/fcm/send"
+FCM_URL = b"https://fcm.googleapis.com/fcm/send"
 MAX_TRIES = 3
 RETRY_DELAY_BASE = 10
 MAX_BYTES_PER_FIELD = 1024
 
 # The error codes that mean a registration ID will never
 # succeed and we should reject it upstream.
 # We include NotRegistered here too for good measure, even
-# though gcm-client 'helpfully' extracts these into a separate
+# though fcm-client 'helpfully' extracts these into a separate
 # list.
 BAD_PUSHKEY_FAILURE_CODES = [
     "MissingRegistration",
@@ -86,9 +86,9 @@
 DEFAULT_MAX_CONNECTIONS = 20
 
 
-class GcmPushkin(ConcurrencyLimitedPushkin):
+class FcmPushkin(ConcurrencyLimitedPushkin):
     """
-    Pushkin that relays notifications to Google/Firebase Cloud Messaging.
+    Pushkin that relays notifications to Firebase Cloud Messaging.
     """
 
     UNDERSTOOD_CONFIG_FIELDS = {
@@ -99,7 +99,7 @@ class GcmPushkin(ConcurrencyLimitedPushkin):
     } | ConcurrencyLimitedPushkin.UNDERSTOOD_CONFIG_FIELDS
 
     def __init__(self, name, sygnal, config):
-        super(GcmPushkin, self).__init__(name, sygnal, config)
+        super(FcmPushkin, self).__init__(name, sygnal, config)
 
         nonunderstood = set(self.cfg.keys()).difference(self.UNDERSTOOD_CONFIG_FIELDS)
         if len(nonunderstood) > 0:
@@ -177,14 +177,14 @@ async def _perform_http_request(self, body, headers):
                 with ACTIVE_REQUESTS_GAUGE.track_inprogress():
                     response = await self.http_agent.request(
                         b"POST",
-                        GCM_URL,
+                        FCM_URL,
                         headers=Headers(headers),
                         bodyProducer=body_producer,
                     )
                     response_text = (await readBody(response)).decode()
         except Exception as exception:
             raise TemporaryNotificationDispatchException(
-                "GCM request failure"
+                "FCM request failure"
             ) from exception
         finally:
             self.connection_semaphore.release()
@@ -201,7 +201,7 @@ async def _request_dispatch(self, n, log, body, headers, pushkeys, span):
             pushkin=self.name, code=response.code
         ).inc()
 
-        log.debug("GCM request took %f seconds", time.time() - poke_start_time)
+        log.debug("FCM request took %f seconds", time.time() - poke_start_time)
 
         span.set_tag(tags.HTTP_STATUS_CODE, response.code)
 
@@ -217,7 +217,7 @@ async def _request_dispatch(self, n, log, body, headers, pushkeys, span):
                 span.log_kv({"event": "gcm_retry_after", "retry_after": retry_after})
 
             raise TemporaryNotificationDispatchException(
-                "GCM server error, hopefully temporary.", custom_retry_delay=retry_after
+                "FCM server error, hopefully temporary.", custom_retry_delay=retry_after
             )
         elif response.code == 400:
             log.error(
@@ -241,7 +241,7 @@ async def _request_dispatch(self, n, log, body, headers, pushkeys, span):
             try:
                 resp_object = json_decoder.decode(response_text)
             except ValueError:
-                raise NotificationDispatchException("Invalid JSON response from GCM.")
+                raise NotificationDispatchException("Invalid JSON response from FCM.")
             if "results" not in resp_object:
                 log.error(
                     "%d from server but response contained no 'results' key: %r",
@@ -269,7 +269,7 @@ async def _request_dispatch(self, n, log, body, headers, pushkeys, span):
                     log.warning(
                         "Error for pushkey %s: %s", pushkeys[i], result["error"]
                     )
-                    span.set_tag("gcm_error", result["error"])
+                    span.set_tag("fcm_error", result["error"])
                     if result["error"] in BAD_PUSHKEY_FAILURE_CODES:
                         log.info(
                             "Reg ID %r has permanently failed with code %r: "
@@ -294,7 +294,7 @@ async def _request_dispatch(self, n, log, body, headers, pushkeys, span):
             return failed, new_pushkeys
         else:
             raise NotificationDispatchException(
-                f"Unknown GCM response code {response.code}"
+                f"Unknown FCM response code {response.code}"
             )
 
     async def _dispatch_notification_unlimited(self, n, device, context):
@@ -312,12 +312,12 @@ async def _dispatch_notification_unlimited(self, n, device, context):
         # The pushkey is kind of secret because you can use it to send push
         # to someone.
         # span_tags = {"pushkeys": pushkeys}
-        span_tags = {"gcm_num_devices": len(pushkeys)}
+        span_tags = {"fcm_num_devices": len(pushkeys)}
 
         with self.sygnal.tracer.start_span(
-            "gcm_dispatch", tags=span_tags, child_of=context.opentracing_span
+            "fcm_dispatch", tags=span_tags, child_of=context.opentracing_span
         ) as span_parent:
-            data = GcmPushkin._build_data(n, device)
+            data = FcmPushkin._build_data(n, device)
             headers = {
                 b"User-Agent": ["sygnal"],
                 b"Content-Type": ["application/json"],
@@ -343,7 +343,7 @@ async def _dispatch_notification_unlimited(self, n, device, context):
                     span_tags = {"retry_num": retry_number}
 
                     with self.sygnal.tracer.start_span(
-                        "gcm_dispatch_try", tags=span_tags, child_of=span_parent
+                        "fcm_dispatch_try", tags=span_tags, child_of=span_parent
                     ) as span:
                         new_failed, new_pushkeys = await self._request_dispatch(
                             n, log, body, headers, pushkeys, span
@@ -375,7 +375,7 @@ async def _dispatch_notification_unlimited(self, n, device, context):
             if len(pushkeys) > 0:
                 log.info("Gave up retrying reg IDs: %r", pushkeys)
             # Count the number of failed devices.
-            span_parent.set_tag("gcm_num_failed", len(failed))
+            span_parent.set_tag("fcm_num_failed", len(failed))
             return failed
 
     @staticmethod
@@ -409,7 +409,7 @@ def _build_data(n, device):
             if hasattr(n, attr):
                 data[attr] = getattr(n, attr)
                 # Truncate fields to a sensible maximum length. If the whole
-                # body is too long, GCM will reject it.
+                # body is too long, FCM will reject it.
                 if data[attr] is not None and len(data[attr]) > MAX_BYTES_PER_FIELD:
                     data[attr] = data[attr][0:MAX_BYTES_PER_FIELD]
 

sygnal/sygnal.py

@@ -151,6 +151,8 @@ async def _make_pushkin(self, app_name, app_config):
             A pushkin of the desired type.
         """
         app_type = app_config["type"]
+        if app_type == "gcm":
+            app_type = "fcm"
         if "." in app_type:
             kind_split = app_type.rsplit(".", 1)
             to_import = kind_split[0]

tests/test_concurrency_limit.py

@@ -18,13 +18,13 @@
 
 from tests.testutils import TestCase
 
-DEVICE_GCM1_EXAMPLE = {
-    "app_id": "com.example.gcm",
+DEVICE_FCM1_EXAMPLE = {
+    "app_id": "com.example.fcm",
     "pushkey": "spqrg",
     "pushkey_ts": 42,
 }
-DEVICE_GCM2_EXAMPLE = {
-    "app_id": "com.example.gcm",
+DEVICE_FCM2_EXAMPLE = {
+    "app_id": "com.example.fcm",
     "pushkey": "spqrh",
     "pushkey_ts": 42,
 }
@@ -48,7 +48,7 @@ async def _dispatch_notification_unlimited(self, n, device, context):
 class ConcurrencyLimitTestCase(TestCase):
     def config_setup(self, config):
         super(ConcurrencyLimitTestCase, self).config_setup(config)
-        config["apps"]["com.example.gcm"] = {
+        config["apps"]["com.example.fcm"] = {
             "type": "tests.test_concurrency_limit.SlowConcurrencyLimitedDummyPushkin",
             "inflight_request_limit": 1,
         }
@@ -61,7 +61,7 @@ def test_passes_under_limit_one(self):
         """
         Tests that a push notification succeeds if it is under the limit.
         """
-        resp = self._request(self._make_dummy_notification([DEVICE_GCM1_EXAMPLE]))
+        resp = self._request(self._make_dummy_notification([DEVICE_FCM1_EXAMPLE]))
 
         self.assertEqual(resp, {"rejected": []})
 
@@ -71,7 +71,7 @@ def test_passes_under_limit_multiple_no_interfere(self):
         pushkins (so do not hit a per-pushkin limit).
         """
         resp = self._request(
-            self._make_dummy_notification([DEVICE_GCM1_EXAMPLE, DEVICE_APNS_EXAMPLE])
+            self._make_dummy_notification([DEVICE_FCM1_EXAMPLE, DEVICE_APNS_EXAMPLE])
         )
 
         self.assertEqual(resp, {"rejected": []})
@@ -83,8 +83,8 @@ def test_fails_when_limit_hit(self):
         """
         resp = self._multi_requests(
             [
-                self._make_dummy_notification([DEVICE_GCM1_EXAMPLE]),
-                self._make_dummy_notification([DEVICE_GCM2_EXAMPLE]),
+                self._make_dummy_notification([DEVICE_FCM1_EXAMPLE]),
+                self._make_dummy_notification([DEVICE_FCM2_EXAMPLE]),
             ]
         )