Browse Source

feat(relay): Add support for collecting relay analytics (#20653)

* add support for collecting relay analytics
Radu Woinaroski 4 years ago
parent
commit
0d9178e836

+ 1 - 1
migrations_lockfile.txt

@@ -10,7 +10,7 @@ auth: 0008_alter_user_username_max_length
 contenttypes: 0002_remove_content_type_name
 jira_ac: 0001_initial
 nodestore: 0001_initial
-sentry: 0101_backfill_file_type_on_event_attachment
+sentry: 0102_collect_relay_analytics
 sessions: 0001_initial
 sites: 0002_alter_domain_unique
 social_auth: 0001_initial

+ 1 - 1
requirements-base.txt

@@ -54,7 +54,7 @@ requests[security]>=2.20.0,<2.21.0
 rfc3339-validator==0.1.2
 rfc3986-validator==0.1.1
 # [end] jsonschema format validators
-sentry-relay>=0.6.0,<0.7.0
+sentry-relay>=0.7.0,<0.8.0
 sentry-sdk>=0.16.4,<0.17.0
 simplejson>=3.11.0,<3.12.0
 six>=1.11.0,<1.12.0

+ 10 - 3
src/sentry/api/endpoints/relay_register.py

@@ -10,13 +10,12 @@ from django.utils import timezone
 
 from sentry import options
 from sentry.utils import json
-from sentry.models import Relay
+from sentry.models import Relay, RelayUsage
 from sentry.auth.system import is_internal_ip
 from sentry.api.base import Endpoint
 from sentry.api.serializers import serialize
 from sentry.relay.utils import get_header_relay_id, get_header_relay_signature
 
-
 from sentry_relay import (
     create_register_challenge,
     validate_register_response,
@@ -168,6 +167,7 @@ class RelayRegisterResponseEndpoint(Endpoint):
             )
 
         relay_id = six.text_type(validated["relay_id"])
+        version = six.text_type(validated["version"])
         public_key = validated["public_key"]
 
         if relay_id != get_header_relay_id(request):
@@ -184,8 +184,15 @@ class RelayRegisterResponseEndpoint(Endpoint):
                 relay_id=relay_id, public_key=public_key, is_internal=is_internal
             )
         else:
-            relay.last_seen = timezone.now()
             relay.is_internal = is_internal
             relay.save()
 
+        try:
+            relay_usage = RelayUsage.objects.get(relay_id=relay_id, version=version)
+        except RelayUsage.DoesNotExist:
+            RelayUsage.objects.create(relay_id=relay_id, version=version)
+        else:
+            relay_usage.last_seen = timezone.now()
+            relay_usage.save()
+
         return Response(serialize({"relay_id": relay.relay_id}))

+ 66 - 0
src/sentry/migrations/0102_collect_relay_analytics.py

@@ -0,0 +1,66 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.29 on 2020-09-16 08:42
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.utils.timezone
+import sentry.db.models.fields.bounded
+
+
+class Migration(migrations.Migration):
+    # This flag is used to mark that a migration shouldn't be automatically run in
+    # production. We set this to True for operations that we think are risky and want
+    # someone from ops to run manually and monitor.
+    # General advice is that if in doubt, mark your migration as `is_dangerous`.
+    # Some things you should always mark as dangerous:
+    # - Large data migrations. Typically we want these to be run manually by ops so that
+    #   they can be monitored. Since data migrations will now hold a transaction open
+    #   this is even more important.
+    # - Adding columns to highly active tables, even ones that are NULL.
+    is_dangerous = False
+
+    # This flag is used to decide whether to run this migration in a transaction or not.
+    # By default we prefer to run in a transaction, but for migrations where you want
+    # to `CREATE INDEX CONCURRENTLY` this needs to be set to False. Typically you'll
+    # want to create an index concurrently when adding one to an existing table.
+    atomic = True
+
+
+    dependencies = [
+        ('sentry', '0101_backfill_file_type_on_event_attachment'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='RelayUsage',
+            fields=[
+                ('id', sentry.db.models.fields.bounded.BoundedBigAutoField(primary_key=True, serialize=False)),
+                ('relay_id', models.CharField(max_length=64)),
+                ('version', models.CharField(default=b'0.0.1', max_length=32)),
+                ('first_seen', models.DateTimeField(default=django.utils.timezone.now)),
+                ('last_seen', models.DateTimeField(default=django.utils.timezone.now)),
+            ],
+            options={
+                'db_table': 'sentry_relayusage',
+            },
+        ),
+        migrations.AlterField(
+            model_name='relay',
+            name='first_seen',
+            field=models.DateTimeField(default=None, null=True),
+        ),
+        migrations.AlterField(
+            model_name='relay',
+            name='is_internal',
+            field=models.NullBooleanField(default=None),
+        ),
+        migrations.AlterField(
+            model_name='relay',
+            name='last_seen',
+            field=models.DateTimeField(default=None, null=True),
+        ),
+        migrations.AlterUniqueTogether(
+            name='relayusage',
+            unique_together=set([('relay_id', 'version')]),
+        ),
+    ]

+ 19 - 3
src/sentry/models/relay.py

@@ -10,14 +10,30 @@ from django.utils.functional import cached_property
 from sentry_relay import PublicKey
 
 
+class RelayUsage(Model):
+    __core__ = True
+
+    relay_id = models.CharField(max_length=64)
+    version = models.CharField(max_length=32, default="0.0.1")
+    first_seen = models.DateTimeField(default=timezone.now)
+    last_seen = models.DateTimeField(default=timezone.now)
+
+    class Meta:
+        unique_together = (("relay_id", "version"),)
+        app_label = "sentry"
+        db_table = "sentry_relayusage"
+
+
 class Relay(Model):
     __core__ = True
 
     relay_id = models.CharField(max_length=64, unique=True)
     public_key = models.CharField(max_length=200)
-    first_seen = models.DateTimeField(default=timezone.now)
-    last_seen = models.DateTimeField(default=timezone.now)
-    is_internal = models.BooleanField(default=False)
+    # not used, functionality replaced by RelayUsage
+    first_seen = models.DateTimeField(default=None, null=True)
+    # not used, functionality replaced by RelayUsage
+    last_seen = models.DateTimeField(default=None, null=True)
+    is_internal = models.NullBooleanField(default=None)
 
     class Meta:
         app_label = "sentry"

+ 171 - 1
tests/sentry/api/endpoints/test_relay_register.py

@@ -6,9 +6,10 @@ from uuid import uuid4
 
 from django.conf import settings
 from django.core.urlresolvers import reverse
+from django.utils import timezone
 
 from sentry.utils import json
-from sentry.models import Relay
+from sentry.models import Relay, RelayUsage
 from sentry.testutils import APITestCase
 
 from sentry_relay import generate_key_pair
@@ -28,6 +29,58 @@ class RelayRegisterTest(APITestCase):
 
         self.path = reverse("sentry-api-0-relay-register-challenge")
 
+    def add_internal_key(self, public_key):
+        if public_key not in settings.SENTRY_RELAY_WHITELIST_PK:
+            settings.SENTRY_RELAY_WHITELIST_PK.append(six.text_type(self.public_key))
+
+    def register_relay(self, key_pair, version, relay_id):
+
+        private_key = key_pair[0]
+        public_key = key_pair[1]
+
+        data = {"public_key": six.text_type(public_key), "relay_id": relay_id, "version": version}
+
+        raw_json, signature = private_key.pack(data)
+
+        resp = self.client.post(
+            self.path,
+            data=raw_json,
+            content_type="application/json",
+            HTTP_X_SENTRY_RELAY_ID=relay_id,
+            HTTP_X_SENTRY_RELAY_SIGNATURE=signature,
+        )
+
+        assert resp.status_code == 200, resp.content
+        result = json.loads(resp.content)
+
+        raw_json, signature = self.private_key.pack(result)
+
+        self.client.post(
+            reverse("sentry-api-0-relay-register-response"),
+            data=raw_json,
+            content_type="application/json",
+            HTTP_X_SENTRY_RELAY_ID=relay_id,
+            HTTP_X_SENTRY_RELAY_SIGNATURE=signature,
+        )
+
+        data = {
+            "token": six.text_type(result.get("token")),
+            "relay_id": relay_id,
+            "version": version,
+        }
+
+        raw_json, signature = private_key.pack(data)
+
+        resp = self.client.post(
+            reverse("sentry-api-0-relay-register-response"),
+            data=raw_json,
+            content_type="application/json",
+            HTTP_X_SENTRY_RELAY_ID=relay_id,
+            HTTP_X_SENTRY_RELAY_SIGNATURE=signature,
+        )
+
+        assert resp.status_code == 200, resp.content
+
     def test_valid_register(self):
         data = {"public_key": six.text_type(self.public_key), "relay_id": self.relay_id}
 
@@ -400,3 +453,120 @@ class RelayRegisterTest(APITestCase):
     def test_valid_register_response_twice(self):
         self.test_valid_register_response()
         self.test_valid_register_response()
+
+    def test_old_relays_can_register(self):
+        """
+        Test that an old Relay that does not send version information
+        in the challenge response is still able to register.
+        """
+        data = {
+            "public_key": six.text_type(self.public_key),
+            "relay_id": self.relay_id,
+            "version": "1.0.0",
+        }
+
+        raw_json, signature = self.private_key.pack(data)
+
+        resp = self.client.post(
+            self.path,
+            data=raw_json,
+            content_type="application/json",
+            HTTP_X_SENTRY_RELAY_ID=self.relay_id,
+            HTTP_X_SENTRY_RELAY_SIGNATURE=signature,
+        )
+
+        assert resp.status_code == 200, resp.content
+        result = json.loads(resp.content)
+
+        raw_json, signature = self.private_key.pack(result)
+
+        self.client.post(
+            reverse("sentry-api-0-relay-register-response"),
+            data=raw_json,
+            content_type="application/json",
+            HTTP_X_SENTRY_RELAY_ID=self.relay_id,
+            HTTP_X_SENTRY_RELAY_SIGNATURE=signature,
+        )
+
+        data = {"token": six.text_type(result.get("token")), "relay_id": self.relay_id}
+
+        raw_json, signature = self.private_key.pack(data)
+
+        resp = self.client.post(
+            reverse("sentry-api-0-relay-register-response"),
+            data=raw_json,
+            content_type="application/json",
+            HTTP_X_SENTRY_RELAY_ID=self.relay_id,
+            HTTP_X_SENTRY_RELAY_SIGNATURE=signature,
+        )
+
+        assert resp.status_code == 200, resp.content
+
+    def test_multiple_relay_versions_tracked(self):
+        """
+        Test that updating the relay version would properly be
+        reflected in the relay analytics. Also that tests that
+        multiple relays
+        """
+        key_pair = generate_key_pair()
+        relay_id = six.text_type(uuid4())
+        before_registration = timezone.now()
+        self.register_relay(key_pair, "1.1.1", relay_id)
+        after_first_relay = timezone.now()
+        self.register_relay(key_pair, "2.2.2", relay_id)
+        after_second_relay = timezone.now()
+
+        v1 = Relay.objects.get(relay_id=relay_id)
+        assert v1 is not None
+
+        rv1 = RelayUsage.objects.get(relay_id=relay_id, version="1.1.1")
+        assert rv1 is not None
+        rv2 = RelayUsage.objects.get(relay_id=relay_id, version="2.2.2")
+        assert rv2 is not None
+
+        assert rv1.first_seen > before_registration
+        assert rv1.last_seen > before_registration
+        assert rv1.first_seen < after_first_relay
+        assert rv1.last_seen < after_first_relay
+
+        assert rv2.first_seen > after_first_relay
+        assert rv2.last_seen > after_first_relay
+        assert rv2.first_seen < after_second_relay
+        assert rv2.last_seen < after_second_relay
+
+    def test_relay_usage_is_updated_at_registration(self):
+        """
+        Tests that during registration the proper relay usage information
+        is updated
+        """
+
+        key_pair = generate_key_pair()
+        relay_id = six.text_type(uuid4())
+        before_registration = timezone.now()
+        # register one relay
+        self.register_relay(key_pair, "1.1.1", relay_id)
+        after_first_relay = timezone.now()
+        # register another one that should not be updated after this
+        self.register_relay(key_pair, "2.2.2", relay_id)
+        after_second_relay = timezone.now()
+        # re register the first one in order to update the last used time
+        self.register_relay(key_pair, "1.1.1", relay_id)
+        after_re_register = timezone.now()
+
+        rv1 = RelayUsage.objects.get(relay_id=relay_id, version="1.1.1")
+        assert rv1 is not None
+        rv2 = RelayUsage.objects.get(relay_id=relay_id, version="2.2.2")
+        assert rv2 is not None
+
+        # check first seen is not modified by re register
+        assert rv1.first_seen > before_registration
+        assert rv1.first_seen < after_first_relay
+        # check last seen shows the time at re-registration
+        assert rv1.last_seen > after_second_relay
+        assert rv1.last_seen < after_re_register
+
+        # check version 2.2.2 is not affected by version 1.1.1
+        assert rv2.first_seen > after_first_relay
+        assert rv2.last_seen > after_first_relay
+        assert rv2.first_seen < after_second_relay
+        assert rv2.last_seen < after_second_relay