Merge branch 'develop' into feature/issue-774-metrics-mw-expire-keys

go/apps/bulk_message/tests/test_vumi_app.py

@@ -212,26 +212,6 @@ def test_process_command_send_message_in_reply_to(self):
         self.assertEqual(sent_msg['content'], 'foo')
         self.assertEqual(sent_msg['in_reply_to'], msg['message_id'])
 
-    @inlineCallbacks
-    def test_collect_metrics(self):
-        conv = yield self.create_conversation()
-        yield self.start_conversation(conv)
-
-        yield self.msg_helper.make_stored_outbound(conv, "out 1")
-        yield self.msg_helper.make_stored_outbound(conv, "out 2")
-        yield self.msg_helper.make_stored_inbound(conv, "in 2")
-
-        yield self.dispatch_command(
-            'collect_metrics', conversation_key=conv.key,
-            user_account_key=self.user_account.key)
-
-        prefix = "campaigns.test-0-user.conversations.%s" % conv.key
-
-        self.assertEqual(
-            self.get_published_metrics(self.app),
-            [("%s.messages_sent" % prefix, 2),
-             ("%s.messages_received" % prefix, 1)])
-
     @inlineCallbacks
     def test_reconcile_cache(self):
         conv = yield self.create_conversation()

go/apps/bulk_message/vumi_app.py

@@ -113,11 +113,6 @@ def handle_event(self, event):
                                 message['message_id'])
             yield self.window_manager.remove_key(window_id, flight_key)
 
-    @inlineCallbacks
-    def collect_metrics(self, user_api, conversation_key):
-        conv = yield user_api.get_wrapped_conversation(conversation_key)
-        yield self.collect_message_metrics(conv)
-
     @inlineCallbacks
     def process_command_initial_action_hack(self, user_account_key,
                                             conversation_key, **kwargs):

go/apps/sequential_send/tests/test_vumi_app.py

@@ -283,18 +283,3 @@ def test_sends(self):
                        key=lambda m: m['to_addr'])
         self.assertEqual(msg['content'], 'bar')
         self.assertEqual(msg['to_addr'], contact3.msisdn)
-
-    @inlineCallbacks
-    def test_collect_metrics(self):
-        conv = yield self.create_conversation()
-        yield self.start_conversation(conv)
-        yield self.dispatch_command(
-            'collect_metrics', conversation_key=conv.key,
-            user_account_key=self.user_account.key)
-
-        prefix = "campaigns.test-0-user.conversations.%s" % conv.key
-
-        self.assertEqual(
-            self.get_published_metrics(self.app),
-            [("%s.messages_sent" % prefix, 0),
-             ("%s.messages_received" % prefix, 0)])

go/apps/sequential_send/vumi_app.py

@@ -187,8 +187,3 @@ def process_command_stop(self, user_account_key, conversation_key):
         log.debug("Unscheduling conversation: %s" % (conversation_key,))
         yield self.redis.srem('scheduled_conversations', json.dumps(
                 [user_account_key, conversation_key]))
-
-    @inlineCallbacks
-    def collect_metrics(self, user_api, conversation_key):
-        conv = yield user_api.get_wrapped_conversation(conversation_key)
-        yield self.collect_message_metrics(conv)

go/apps/subscription/definition.py

@@ -1,5 +1,23 @@
 from go.vumitools.conversation.definition import ConversationDefinitionBase
+from go.apps.subscription.metrics import SubscribedMetric, UnsubscribedMetric
 
 
 class ConversationDefinition(ConversationDefinitionBase):
     conversation_type = u'subscription'
+
+    def get_metrics(self):
+        metrics = super(ConversationDefinition, self).get_metrics()
+
+        campaign_names = sorted(set([
+            h['campaign_name']
+            for h in self.conv.get_config().get('handlers', [])]))
+
+        metrics.extend([
+            SubscribedMetric(self.conv, campaign_name)
+            for campaign_name in campaign_names])
+
+        metrics.extend([
+            UnsubscribedMetric(self.conv, campaign_name)
+            for campaign_name in campaign_names])
+
+        return metrics

go/apps/subscription/metrics.py

@@ -0,0 +1,32 @@
+from go.vumitools.metrics import ConversationMetric
+
+
+class SubscriptionMetric(ConversationMetric):
+    CONTACT_LOOKUP_KEY = None
+
+    def __init__(self, conv, campaign_name):
+        self.campaign_name = campaign_name
+        metric_name = self.make_metric_name(campaign_name)
+        super(SubscriptionMetric, self).__init__(conv, metric_name)
+
+    @classmethod
+    def make_metric_name(cls, campaign_name):
+        return "%s.%s" % (campaign_name, cls.METRIC_NAME)
+
+    def get_value(self, user_api):
+        contacts = user_api.contact_store.contacts
+        search = contacts.raw_search(
+            "subscription-%s:%s" %
+            (self.campaign_name, self.CONTACT_LOOKUP_KEY))
+
+        return search.get_count()
+
+
+class SubscribedMetric(SubscriptionMetric):
+    METRIC_NAME = 'subscribed'
+    CONTACT_LOOKUP_KEY = 'subscribed'
+
+
+class UnsubscribedMetric(SubscriptionMetric):
+    METRIC_NAME = 'unsubscribed'
+    CONTACT_LOOKUP_KEY = 'unsubscribed'

go/apps/subscription/tests/test_definition.py

@@ -0,0 +1,56 @@
+from go.base.tests.utils import VumiGoDjangoTestCase
+from go.vumitools.metrics import MessagesReceivedMetric, MessagesSentMetric
+from go.apps.subscription.definition import ConversationDefinition
+from go.apps.subscription.metrics import SubscribedMetric, UnsubscribedMetric
+
+
+class TestSubscriptionConversationDefinition(VumiGoDjangoTestCase):
+    def setUp(self):
+        super(TestSubscriptionConversationDefinition, self).setUp()
+
+        self.setup_user_api()
+        self.conv = self.create_conversation(
+            conversation_type=u'subscription',
+            config={
+                'handlers': [
+                    {'campaign_name': 'campaign-1'},
+                    {'campaign_name': 'campaign-2'}]
+            })
+        self.conv_def = ConversationDefinition(self.conv)
+
+    def test_metrics_retrieval(self):
+        [m1, m2, m3, m4, m5, m6] = self.conv_def.get_metrics()
+
+        metric_name_prefix = (
+            "go.campaigns.%s.conversations.%s"
+            % (self.conv.user_account.key, self.conv.key))
+
+        self.assertEqual(
+            m1.get_full_name(),
+            '%s.messages_sent' % metric_name_prefix)
+        self.assertTrue(isinstance(m1, MessagesSentMetric))
+
+        self.assertEqual(
+            m2.get_full_name(),
+            '%s.messages_received' % metric_name_prefix)
+        self.assertTrue(isinstance(m2, MessagesReceivedMetric))
+
+        self.assertEqual(
+            m3.get_full_name(),
+            '%s.campaign-1.subscribed' % metric_name_prefix)
+        self.assertTrue(isinstance(m3, SubscribedMetric))
+
+        self.assertEqual(
+            m4.get_full_name(),
+            '%s.campaign-2.subscribed' % metric_name_prefix)
+        self.assertTrue(isinstance(m4, SubscribedMetric))
+
+        self.assertEqual(
+            m5.get_full_name(),
+            '%s.campaign-1.unsubscribed' % metric_name_prefix)
+        self.assertTrue(isinstance(m5, UnsubscribedMetric))
+
+        self.assertEqual(
+            m6.get_full_name(),
+            '%s.campaign-2.unsubscribed' % metric_name_prefix)
+        self.assertTrue(isinstance(m6, UnsubscribedMetric))

go/apps/subscription/tests/test_metrics.py

@@ -0,0 +1,54 @@
+from twisted.internet.defer import inlineCallbacks
+from go.apps.subscription.metrics import SubscriptionMetric
+from go.vumitools.tests.utils import TxMetricTestBase
+from go.vumitools.contact import ContactStore
+
+
+class ToySubscriptionMetric(SubscriptionMetric):
+    METRIC_NAME = 'toy-subscription-metric'
+    CONTACT_LOOKUP_KEY = 'toy-subscription'
+
+
+class TestSubscriptionMetric(TxMetricTestBase):
+    @inlineCallbacks
+    def setUp(self):
+        yield super(TestSubscriptionMetric, self).setUp()
+
+        self.conv = yield self.create_conversation(
+            conversation_type=u'some_conversation')
+
+        self.contact_store = ContactStore.from_user_account(self.user)
+        yield self.contact_store.contacts.enable_search()
+
+        self.contact1 = yield self.contact_store.new_contact(
+            name=u'contact-1',
+            msisdn=u'+27831234567')
+
+        self.contact2 = yield self.contact_store.new_contact(
+            name=u'contact-2',
+            msisdn=u'+27831234568')
+
+        yield self.contact1.save()
+        yield self.contact2.save()
+
+        self.metric = ToySubscriptionMetric(self.conv, 'campaign-1')
+
+    def test_name_construction(self):
+        self.assertEqual(
+            self.metric.get_full_name(),
+            "go.campaigns.test-0-user.conversations.%s."
+            "campaign-1.toy-subscription-metric" % self.conv.key)
+
+    @inlineCallbacks
+    def test_value_retrieval(self):
+        self.assertEqual(
+            (yield self.metric.get_value(self.user_api)), 0)
+
+        self.contact1.subscription['campaign-1'] = u'toy-subscription'
+        self.contact2.subscription['campaign-1'] = u'toy-subscription'
+
+        yield self.contact1.save()
+        yield self.contact2.save()
+
+        self.assertEqual(
+            (yield self.metric.get_value(self.user_api)), 2)

go/apps/subscription/tests/test_vumi_app.py

@@ -63,13 +63,6 @@ def dispatch_from(self, contact, content, **kw):
         msg = self.msg_helper.make_inbound(content, **kw)
         return self.dispatch_to_conv(msg, self.conv)
 
-    def set_subscription(self, contact, subscribed, unsubscribed):
-        for campaign_name in subscribed:
-            contact.subscription[campaign_name] = u'subscribed'
-        for campaign_name in unsubscribed:
-            contact.subscription[campaign_name] = u'unsubscribed'
-        return contact.save()
-
     @inlineCallbacks
     def test_subscribe_unsubscribe(self):
         yield self.assert_subscription(self.contact, 'foo', None)
@@ -103,28 +96,3 @@ def test_empty_message(self):
         [reply] = yield self.get_dispatched_messages()
         self.assertEqual('Unrecognised keyword.', reply['content'])
         yield self.assert_subscription(self.contact, 'foo', None)
-
-    @inlineCallbacks
-    def test_collect_metrics(self):
-        second_contact = yield self.user_api.contact_store.new_contact(
-            name=u'Second', surname=u'Contact', msisdn=u'+27831234568')
-        third_contact = yield self.user_api.contact_store.new_contact(
-            name=u'Third', surname=u'Contact', msisdn=u'+27831234569')
-        yield self.set_subscription(self.contact, [], ['bar'])
-        yield self.set_subscription(second_contact, ['foo', 'bar'], [])
-        yield self.set_subscription(third_contact, ['foo'], ['bar'])
-
-        yield self.dispatch_command(
-            'collect_metrics', conversation_key=self.conv.key,
-            user_account_key=self.user_account.key)
-
-        prefix = "campaigns.test-0-user.conversations.%s" % self.conv.key
-
-        self.assertEqual(
-            self.get_published_metrics(self.app),
-            [("%s.foo.subscribed" % prefix, 2),
-             ("%s.foo.unsubscribed" % prefix, 0),
-             ("%s.bar.subscribed" % prefix, 1),
-             ("%s.bar.unsubscribed" % prefix, 2),
-             ("%s.messages_sent" % prefix, 0),
-             ("%s.messages_received" % prefix, 0)])

go/apps/subscription/vumi_app.py

@@ -61,26 +61,3 @@ def consume_ack(self, event):
 
     def consume_delivery_report(self, event):
         return self.vumi_api.mdb.add_event(event)
-
-    @inlineCallbacks
-    def collect_metrics(self, user_api, conversation_key):
-        conv = yield user_api.get_wrapped_conversation(conversation_key)
-        contact_proxy = user_api.contact_store.contacts
-
-        campaign_names = set()
-        for handler in conv.get_config().get('handlers', []):
-            campaign_names.add(handler['campaign_name'])
-
-        for campaign_name in campaign_names:
-            self.publish_conversation_metric(
-                conv, '.'.join([campaign_name, "subscribed"]),
-                (yield contact_proxy.raw_search(
-                    "subscription-%s:subscribed" % (
-                        campaign_name,)).get_count()))
-            self.publish_conversation_metric(
-                conv, '.'.join([campaign_name, "unsubscribed"]),
-                (yield contact_proxy.raw_search(
-                    "subscription-%s:unsubscribed" % (
-                        campaign_name,)).get_count()))
-
-        yield self.collect_message_metrics(conv)

go/base/middleware.py

@@ -47,7 +47,7 @@ def process_response(self, request, response):
             response['X-Response-Time'] = response_time
 
             metric = self.metric_from_request(request)
-            metric.oneshot(response_time)
+            metric.oneshot(value=response_time)
         except AttributeError, e:
             # For cases where our request object was not processed and given a
             # `start_time` attribute

go/vumitools/app_worker.py

@@ -9,8 +9,9 @@
 from vumi.config import IConfigData, ConfigText, ConfigDict
 from vumi.connectors import IgnoreMessage
 
+from go.base import utils as base_utils
 from go.vumitools.api import VumiApiCommand, VumiApi, VumiApiEvent
-from go.vumitools.metrics import ConversationMetric, AccountMetric
+from go.vumitools.metrics import AccountMetric
 from go.vumitools.utils import MessageMetadataHelper
 
 
@@ -161,11 +162,6 @@ def process_unknown_cmd(self, method_name, *args, **kwargs):
         log.error("Unknown vumi API command: %s(%s, %s)" % (
             method_name, args, kwargs))
 
-    @inlineCallbacks
-    def collect_metrics(self, user_api, conversation_key):
-        conv = yield user_api.get_wrapped_conversation(conversation_key)
-        yield self.collect_message_metrics(conv)
-
     @inlineCallbacks
     def reconcile_cache(self, user_api, conversation_key, delta=0.01):
         """Reconcile the cached values for the conversation.
@@ -268,37 +264,24 @@ def trigger_event(self, message, event_type, content):
     def publish_app_event(self, event):
         self.app_event_publisher.publish_message(event)
 
-    def publish_conversation_metric(self, conv, name, value, agg=None):
-        # NOTE: The plan is have conversation metrics specified as part of the
-        # conversation definition, so not letting them be dynamically created
-        # is intentional. This workaround will go away very soon.
-        class DynamicConversationMetric(ConversationMetric):
-            METRIC_NAME = name
-            AGGREGATORS = [agg] if agg else ConversationMetric.AGGREGATORS
-
-        metric = DynamicConversationMetric(conv)
-        metric.oneshot(self.metrics, value)
-
     def publish_account_metric(self, acc_key, store, name, value, agg=None):
         aggs = [agg] if agg is not None else None
         metric = AccountMetric(acc_key, store, name, aggs)
         metric.oneshot(self.metrics, value)
 
     @inlineCallbacks
-    def collect_message_metrics(self, conversation):
-        """Collect message count metrics.
+    def publish_conversation_metrics(self, user_api, conversation_key):
+        conv = yield user_api.get_conversation(conversation_key)
 
-        This is a utility method for collecting common metrics. It has to be
-        called explicitly from :meth:`collect_metrics`
-        """
-        batch_id = conversation.batch.key
-        sent = yield self.vumi_api.mdb.batch_outbound_count(batch_id)
-        received = yield self.vumi_api.mdb.batch_inbound_count(batch_id)
-
-        self.publish_conversation_metric(
-            conversation, 'messages_sent', sent)
-        self.publish_conversation_metric(
-            conversation, 'messages_received', received)
+        conv_type = conv.conversation_type
+        conv_def = base_utils.get_conversation_definition(conv_type, conv)
+
+        yield gatherResults([
+            m.oneshot(self.metrics, user_api=user_api)
+            for m in conv_def.get_metrics()])
+
+    def collect_metrics(self, user_api, conversation_key):
+        return self.publish_conversation_metrics(user_api, conversation_key)
 
     def add_conv_to_msg_options(self, conv, msg_options):
         helper_metadata = msg_options.setdefault('helper_metadata', {})