Merge ~twom/launchpad:stats-upload-numbers-with-your-uploads into launchpad:master

[Tom Wardill]

Add stats reporting to various UploadJobs

[Colin Watson (cjwatson)]

review: Needs Information

2862122... by Tom Wardill on 2020-10-29

Move stats generation to the base job

33ed7fd... by Tom Wardill on 2020-10-30

Add comments

[Colin Watson (cjwatson)]

review: Needs Information

e4fba93... by Tom Wardill on 2020-11-03

Move stats counting to BaseRunnableJob

f0e82b1... by Tom Wardill on 2020-11-03

Tidy imports

[Colin Watson (cjwatson)]

review: Approve

diff --git a/lib/lp/oci/tests/test_ocirecipebuildjob.py b/lib/lp/oci/tests/test_ocirecipebuildjob.py
index 598940f..59554e6 100644
--- a/lib/lp/oci/tests/test_ocirecipebuildjob.py
+++ b/lib/lp/oci/tests/test_ocirecipebuildjob.py
@@ -57,6 +57,7 @@ from lp.services.job.tests import (
     block_on_job,
     pop_remote_notifications,
     )
+from lp.services.statsd.tests import StatsMixin
 from lp.services.webapp import canonical_url
 from lp.services.webhooks.testing import LogsScheduledWebhooks
 from lp.testing import (
@@ -201,7 +202,8 @@ class MultiArchRecipeMixin:
         return build_request
 
 
-class TestOCIRegistryUploadJob(TestCaseWithFactory, MultiArchRecipeMixin):
+class TestOCIRegistryUploadJob(TestCaseWithFactory, MultiArchRecipeMixin,
+                               StatsMixin):
 
     layer = LaunchpadZopelessLayer
 
@@ -212,6 +214,7 @@ class TestOCIRegistryUploadJob(TestCaseWithFactory, MultiArchRecipeMixin):
             OCI_RECIPE_WEBHOOKS_FEATURE_FLAG: 'on',
             OCI_RECIPE_ALLOW_CREATE: 'on'
             }))
+        self.setUpStats()
 
     def makeOCIRecipeBuild(self, **kwargs):
         ocibuild = self.factory.makeOCIRecipeBuild(
@@ -291,6 +294,14 @@ class TestOCIRegistryUploadJob(TestCaseWithFactory, MultiArchRecipeMixin):
         self.assertIsNone(job.errors)
         self.assertEqual([], pop_notifications())
         self.assertWebhookDeliveries(ocibuild, ["Pending", "Uploaded"], logger)
+        self.assertEqual(4, self.stats_client.incr.call_count)
+        calls = [x[0][0] for x in self.stats_client.incr.call_args_list]
+        self.assertThat(calls, MatchesListwise([
+            Equals('job.start_count,type=OCIRecipeRequestBuildsJob,env=test'),
+            Equals(
+                'job.complete_count,type=OCIRecipeRequestBuildsJob,env=test'),
+            Equals('job.start_count,type=OCIRegistryUploadJob,env=test'),
+            Equals('job.complete_count,type=OCIRegistryUploadJob,env=test')]))
 
     def test_run_multiple_architectures(self):
         build_request = self.makeBuildRequest()
@@ -323,6 +334,15 @@ class TestOCIRegistryUploadJob(TestCaseWithFactory, MultiArchRecipeMixin):
             [((build_request, set(builds[:1])), {}),
              ((build_request, set(builds)), {})],
             client.uploadManifestList.calls)
+        calls = [x[0][0] for x in self.stats_client.incr.call_args_list]
+        self.assertThat(calls, MatchesListwise([
+            Equals('job.start_count,type=OCIRecipeRequestBuildsJob,env=test'),
+            Equals(
+                'job.complete_count,type=OCIRecipeRequestBuildsJob,env=test'),
+            Equals('job.start_count,type=OCIRegistryUploadJob,env=test'),
+            Equals('job.complete_count,type=OCIRegistryUploadJob,env=test'),
+            Equals('job.start_count,type=OCIRegistryUploadJob,env=test'),
+            Equals('job.complete_count,type=OCIRegistryUploadJob,env=test')]))
 
     def test_failing_upload_does_not_retries_automatically(self):
         build_request = self.makeBuildRequest(include_i386=False)
diff --git a/lib/lp/services/job/runner.py b/lib/lp/services/job/runner.py
index 9ac7863..b0b65a2 100644
--- a/lib/lp/services/job/runner.py
+++ b/lib/lp/services/job/runner.py
@@ -61,6 +61,7 @@ from twisted.python import (
     failure,
     log,
     )
+from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
 from lp.services import scripts
@@ -78,6 +79,7 @@ from lp.services.mail.sendmail import (
     MailController,
     set_immediate_mail_delivery,
     )
+from lp.services.statsd.interfaces.statsd_client import IStatsdClient
 from lp.services.timeout import (
     get_default_timeout_function,
     set_default_timeout_function,
@@ -320,6 +322,30 @@ class BaseRunnableJob(BaseRunnableJobSource):
             manage_transaction, abort_transaction,
             add_commit_hook=self.celeryRunOnCommit)
 
+    def start(self, manage_transaction=False):
+        """See `IJob`."""
+        self.job.start(manage_transaction=manage_transaction)
+        statsd = getUtility(IStatsdClient)
+        statsd.incr('job.start_count,type={},env={}'.format(
+            self.__class__.__name__,
+            statsd.lp_environment))
+
+    def complete(self, manage_transaction=False):
+        """See `IJob`."""
+        self.job.complete(manage_transaction=manage_transaction)
+        statsd = getUtility(IStatsdClient)
+        statsd.incr('job.complete_count,type={},env={}'.format(
+            self.__class__.__name__,
+            statsd.lp_environment))
+
+    def fail(self, manage_transaction=False):
+        """See `IJob`."""
+        self.job.fail(manage_transaction=manage_transaction)
+        statsd = getUtility(IStatsdClient)
+        statsd.incr('job.fail_count,type={},env={}'.format(
+            self.__class__.__name__,
+            statsd.lp_environment))
+
 
 class BaseJobRunner(LazrJobRunner):
     """Runner of Jobs."""
diff --git a/lib/lp/services/job/tests/test_runner.py b/lib/lp/services/job/tests/test_runner.py
index 9827f1c..ec8dd11 100644
--- a/lib/lp/services/job/tests/test_runner.py
+++ b/lib/lp/services/job/tests/test_runner.py
@@ -45,6 +45,7 @@ from lp.services.job.runner import (
     )
 from lp.services.log.logger import BufferLogger
 from lp.services.scripts.logger import OopsHandler
+from lp.services.statsd.tests import StatsMixin
 from lp.services.timeline.requesttimeline import get_request_timeline
 from lp.services.timeout import (
     get_default_timeout_function,
@@ -158,11 +159,15 @@ class RaisingRetryJob(NullJob):
         raise RetryError()
 
 
-class TestJobRunner(TestCaseWithFactory):
+class TestJobRunner(StatsMixin, TestCaseWithFactory):
     """Ensure JobRunner behaves as expected."""
 
     layer = LaunchpadZopelessLayer
 
+    def setUp(self):
+        super(TestJobRunner, self).setUp()
+        self.setUpStats()
+
     def makeTwoJobs(self):
         """Test fixture.  Create two jobs."""
         return NullJob("job 1"), NullJob("job 2")
@@ -174,6 +179,12 @@ class TestJobRunner(TestCaseWithFactory):
         runner.runJob(job_1, None)
         self.assertEqual(JobStatus.COMPLETED, job_1.job.status)
         self.assertEqual([job_1], runner.completed_jobs)
+        self.assertEqual(
+            self.stats_client.incr.call_args_list[0][0],
+            ('job.start_count,type=NullJob,env=test',))
+        self.assertEqual(
+            self.stats_client.incr.call_args_list[1][0],
+            ('job.complete_count,type=NullJob,env=test',))
 
     def test_runAll(self):
         """Ensure runAll works in the normal case."""
@@ -219,6 +230,12 @@ class TestJobRunner(TestCaseWithFactory):
         oops = self.oopses[-1]
         self.assertIn('Fake exception.  Foobar, I say!', oops['tb_text'])
         self.assertEqual(["{'foo': 'bar'}"], oops['req_vars'].values())
+        self.assertEqual(
+            self.stats_client.incr.call_args_list[0][0],
+            ('job.start_count,type=NullJob,env=test',))
+        self.assertEqual(
+            self.stats_client.incr.call_args_list[1][0],
+            ('job.fail_count,type=NullJob,env=test',))
 
     def test_oops_messages_used_when_handling(self):
         """Oops messages should appear even when exceptions are handled."""