Launchpad itself

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

  1. Git
  2. lp:~twom/launchpad
  3. stats-upload-numbers-with-your-uploads
  4. Merge into master
Proposed by Tom Wardill
Status: Superseded
Proposed branch: ~twom/launchpad:stats-upload-numbers-with-your-uploads
Merge into: launchpad:master
Diff against target: 372 lines (+100/-10)
9 files modified
lib/lp/buildmaster/model/buildfarmjobbehaviour.py (+7/-0)
lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py (+18/-1)
lib/lp/oci/model/ocirecipebuildjob.py (+4/-1)
lib/lp/oci/tests/test_ocirecipebuildjob.py (+13/-1)
lib/lp/services/statsd/__init__.py (+17/-0)
lib/lp/snappy/model/snapbuildjob.py (+4/-1)
lib/lp/snappy/tests/test_snapbuildjob.py (+13/-1)
lib/lp/soyuz/model/packagetranslationsuploadjob.py (+9/-4)
lib/lp/soyuz/tests/test_packagetranslationsuploadjob.py (+15/-1)
Reviewer Review Type Date Requested Status
Launchpad code reviewers Pending
Review via email: mp+391626@code.launchpad.net

This proposal has been superseded by a proposal from 2020-09-30.

Commit message

Add stats reporting to various UploadJobs

To post a comment you must log in.

Unmerged commits

edd5f64... by Tom Wardill

Add stats reporting to various UploadJobs

7e86ea9... by Tom Wardill

Use class_job_type in package translations upload

a0ee686... by Tom Wardill

Add counts for build dispatches

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/buildmaster/model/buildfarmjobbehaviour.py b/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
2index fa44fb9..402aff2 100644
3--- a/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
4+++ b/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
5@@ -34,6 +34,7 @@ from lp.services.config import config
6 from lp.services.helpers import filenameToContentType
7 from lp.services.librarian.interfaces import ILibraryFileAliasSet
8 from lp.services.librarian.utils import copy_and_close
9+from lp.services.statsd.interfaces.statsd_client import IStatsdClient
10 from lp.services.utils import sanitise_urls
11 from lp.services.webapp import canonical_url
12
13@@ -152,6 +153,12 @@ class BuildFarmJobBehaviourBase:
14 (status, info) = yield self._slave.build(
15 cookie, builder_type, chroot.content.sha1, filename_to_sha1, args)
16
17+ # Update stats
18+ job_type = getattr(self.build, 'job_type', None)
19+ job_type_name = job_type.name if job_type else 'UNKNOWN'
20+ getUtility(IStatsdClient).incr(
21+ 'build.count,job_type={}'.format(job_type_name))
22+
23 logger.info(
24 "Job %s (%s) started on %s: %s %s"
25 % (cookie, self.build.title, self._builder.url, status, info))
26diff --git a/lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py b/lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py
27index 8cbda44..d769732 100644
28--- a/lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py
29+++ b/lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py
30@@ -44,6 +44,7 @@ from lp.buildmaster.tests.mock_slaves import (
31 from lp.registry.interfaces.pocket import PackagePublishingPocket
32 from lp.services.config import config
33 from lp.services.log.logger import BufferLogger
34+from lp.services.statsd.tests import StatsMixin
35 from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
36 from lp.testing import (
37 TestCase,
38@@ -55,6 +56,7 @@ from lp.testing.fakemethod import FakeMethod
39 from lp.testing.layers import (
40 LaunchpadZopelessLayer,
41 ZopelessDatabaseLayer,
42+ ZopelessLayer,
43 )
44 from lp.testing.mail_helpers import pop_notifications
45
46@@ -155,8 +157,9 @@ class TestBuildFarmJobBehaviourBase(TestCaseWithFactory):
47 self.assertIs(False, behaviour.extraBuildArgs()["fast_cleanup"])
48
49
50-class TestDispatchBuildToSlave(TestCase):
51+class TestDispatchBuildToSlave(StatsMixin, TestCase):
52
53+ layer = ZopelessLayer
54 run_tests_with = AsynchronousDeferredRunTest
55
56 def makeBehaviour(self, das):
57@@ -260,6 +263,20 @@ class TestDispatchBuildToSlave(TestCase):
58 self.assertDispatched(
59 slave, logger, 'chroot-fooix-bar-y86.tar.gz', 'chroot')
60
61+ @defer.inlineCallbacks
62+ def test_dispatchBuildToSlave_stats(self):
63+ self.setUpStats()
64+ behaviour = self.makeBehaviour(FakeDistroArchSeries())
65+ builder = MockBuilder()
66+ slave = OkSlave()
67+ logger = BufferLogger()
68+ behaviour.setBuilder(builder, slave)
69+ yield behaviour.dispatchBuildToSlave(logger)
70+ self.assertEqual(1, self.stats_client.incr.call_count)
71+ self.assertEqual(
72+ self.stats_client.incr.call_args_list[0][0],
73+ ('build.count,job_type=UNKNOWN',))
74+
75
76 class TestGetUploadMethodsMixin:
77 """Tests for `IPackageBuild` that need objects from the rest of LP."""
78diff --git a/lib/lp/oci/model/ocirecipebuildjob.py b/lib/lp/oci/model/ocirecipebuildjob.py
79index 73fa4d6..18cfa2f 100644
80--- a/lib/lp/oci/model/ocirecipebuildjob.py
81+++ b/lib/lp/oci/model/ocirecipebuildjob.py
82@@ -49,6 +49,7 @@ from lp.services.job.model.job import (
83 )
84 from lp.services.job.runner import BaseRunnableJob
85 from lp.services.propertycache import get_property_cache
86+from lp.services.statsd import UploadJobStatsMixin
87 from lp.services.webapp.snapshot import notify_modified
88
89
90@@ -155,7 +156,7 @@ class OCIRecipeBuildJobDerived(
91
92 @implementer(IOCIRegistryUploadJob)
93 @provider(IOCIRegistryUploadJobSource)
94-class OCIRegistryUploadJob(OCIRecipeBuildJobDerived):
95+class OCIRegistryUploadJob(UploadJobStatsMixin, OCIRecipeBuildJobDerived):
96
97 class_job_type = OCIRecipeBuildJobType.REGISTRY_UPLOAD
98
99@@ -235,6 +236,7 @@ class OCIRegistryUploadJob(OCIRecipeBuildJobDerived):
100 try:
101 try:
102 client.upload(self.build)
103+ self.reportStats(True)
104 except OCIRegistryError as e:
105 self.error_summary = str(e)
106 self.errors = e.errors
107@@ -245,4 +247,5 @@ class OCIRegistryUploadJob(OCIRecipeBuildJobDerived):
108 raise
109 except Exception:
110 transaction.commit()
111+ self.reportStats(False)
112 raise
113diff --git a/lib/lp/oci/tests/test_ocirecipebuildjob.py b/lib/lp/oci/tests/test_ocirecipebuildjob.py
114index fd3e0ff..24aded3 100644
115--- a/lib/lp/oci/tests/test_ocirecipebuildjob.py
116+++ b/lib/lp/oci/tests/test_ocirecipebuildjob.py
117@@ -40,6 +40,7 @@ from lp.oci.model.ocirecipebuildjob import (
118 from lp.services.config import config
119 from lp.services.features.testing import FeatureFixture
120 from lp.services.job.runner import JobRunner
121+from lp.services.statsd.tests import StatsMixin
122 from lp.services.webapp import canonical_url
123 from lp.services.webhooks.testing import LogsScheduledWebhooks
124 from lp.testing import TestCaseWithFactory
125@@ -106,7 +107,7 @@ class TestOCIRecipeBuildJob(TestCaseWithFactory):
126 self.assertEqual(expected, oops)
127
128
129-class TestOCIRegistryUploadJob(TestCaseWithFactory):
130+class TestOCIRegistryUploadJob(StatsMixin, TestCaseWithFactory):
131
132 layer = LaunchpadZopelessLayer
133
134@@ -117,6 +118,7 @@ class TestOCIRegistryUploadJob(TestCaseWithFactory):
135 OCI_RECIPE_WEBHOOKS_FEATURE_FLAG: 'on',
136 OCI_RECIPE_ALLOW_CREATE: 'on'
137 }))
138+ self.setUpStats()
139
140 def makeOCIRecipeBuild(self, **kwargs):
141 ocibuild = self.factory.makeOCIRecipeBuild(
142@@ -180,6 +182,11 @@ class TestOCIRegistryUploadJob(TestCaseWithFactory):
143 self.assertIsNone(job.errors)
144 self.assertEqual([], pop_notifications())
145 self.assertWebhookDeliveries(ocibuild, ["Pending", "Uploaded"], logger)
146+ # check we got stats in statsd
147+ self.assertEqual(1, self.stats_client.incr.call_count)
148+ self.assertEqual(
149+ self.stats_client.incr.call_args_list[0][0],
150+ ('build_upload.count,job_type=REGISTRY_UPLOAD'',success=True',))
151
152 def test_run_failed_registry_error(self):
153 # A run that fails with a registry error sets the registry upload
154@@ -221,6 +228,11 @@ class TestOCIRegistryUploadJob(TestCaseWithFactory):
155 self.assertEqual([], pop_notifications())
156 self.assertWebhookDeliveries(
157 ocibuild, ["Pending", "Failed to upload"], logger)
158+ # check we got stats in statsd
159+ self.assertEqual(1, self.stats_client.incr.call_count)
160+ self.assertEqual(
161+ self.stats_client.incr.call_args_list[0][0],
162+ ('build_upload.count,job_type=REGISTRY_UPLOAD'',success=False',))
163
164 def test_run_failed_other_error(self):
165 # A run that fails for some reason other than a registry error sets
166diff --git a/lib/lp/services/statsd/__init__.py b/lib/lp/services/statsd/__init__.py
167index e69de29..31636ef 100644
168--- a/lib/lp/services/statsd/__init__.py
169+++ b/lib/lp/services/statsd/__init__.py
170@@ -0,0 +1,17 @@
171+from zope.component import getUtility
172+
173+from lp.services.statsd.interfaces.statsd_client import IStatsdClient
174+
175+
176+class UploadJobStatsMixin:
177+ """Upload job stats reporting.
178+
179+ Upload jobs have no useful common base class, so add the
180+ required methods via mixin.
181+ """
182+
183+ def reportStats(self, success):
184+ job_type = self.class_job_type.name
185+ getUtility(IStatsdClient).incr(
186+ 'build_upload.count,job_type={},success={}'.format(
187+ job_type, success))
188diff --git a/lib/lp/snappy/model/snapbuildjob.py b/lib/lp/snappy/model/snapbuildjob.py
189index 298c00f..36dd544 100644
190--- a/lib/lp/snappy/model/snapbuildjob.py
191+++ b/lib/lp/snappy/model/snapbuildjob.py
192@@ -49,6 +49,7 @@ from lp.services.job.model.job import (
193 )
194 from lp.services.job.runner import BaseRunnableJob
195 from lp.services.propertycache import get_property_cache
196+from lp.services.statsd import UploadJobStatsMixin
197 from lp.snappy.interfaces.snapbuildjob import (
198 ISnapBuildJob,
199 ISnapBuildStoreUploadStatusChangedEvent,
200@@ -179,7 +180,7 @@ class SnapBuildStoreUploadStatusChangedEvent(ObjectEvent):
201
202 @implementer(ISnapStoreUploadJob)
203 @provider(ISnapStoreUploadJobSource)
204-class SnapStoreUploadJob(SnapBuildJobDerived):
205+class SnapStoreUploadJob(SnapBuildJobDerived, UploadJobStatsMixin):
206 """A Job that uploads a snap build to the store."""
207
208 class_job_type = SnapBuildJobType.STORE_UPLOAD
209@@ -346,6 +347,7 @@ class SnapStoreUploadJob(SnapBuildJobDerived):
210 self.store_url, self.store_revision = (
211 client.checkStatus(self.status_url))
212 self.error_message = None
213+ self.reportStats(True)
214 except self.retry_error_types:
215 raise
216 except Exception as e:
217@@ -373,4 +375,5 @@ class SnapStoreUploadJob(SnapBuildJobDerived):
218 # we want to commit instead: the only database changes we make
219 # are to this job's metadata and should be preserved.
220 transaction.commit()
221+ self.reportStats(False)
222 raise
223diff --git a/lib/lp/snappy/tests/test_snapbuildjob.py b/lib/lp/snappy/tests/test_snapbuildjob.py
224index 90bb9db..ed9c096 100644
225--- a/lib/lp/snappy/tests/test_snapbuildjob.py
226+++ b/lib/lp/snappy/tests/test_snapbuildjob.py
227@@ -27,6 +27,7 @@ from lp.services.database.interfaces import IStore
228 from lp.services.features.testing import FeatureFixture
229 from lp.services.job.interfaces.job import JobStatus
230 from lp.services.job.runner import JobRunner
231+from lp.services.statsd.tests import StatsMixin
232 from lp.services.webapp.publisher import canonical_url
233 from lp.services.webhooks.testing import LogsScheduledWebhooks
234 from lp.snappy.interfaces.snap import SNAP_TESTING_FLAGS
235@@ -95,7 +96,7 @@ class TestSnapBuildJob(TestCaseWithFactory):
236 ISnapBuildJob)
237
238
239-class TestSnapStoreUploadJob(TestCaseWithFactory):
240+class TestSnapStoreUploadJob(StatsMixin, TestCaseWithFactory):
241
242 layer = LaunchpadZopelessLayer
243
244@@ -104,6 +105,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
245 self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS))
246 self.status_url = "http://sca.example/dev/api/snaps/1/builds/1/status"
247 self.store_url = "http://sca.example/dev/click-apps/1/rev/1/"
248+ self.setUpStats()
249
250 def test_provides_interface(self):
251 # `SnapStoreUploadJob` objects provide `ISnapStoreUploadJob`.
252@@ -183,6 +185,11 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
253 self.assertEqual([], pop_notifications())
254 self.assertWebhookDeliveries(
255 snapbuild, ["Pending", "Uploaded"], logger)
256+ # check we got stats in statsd
257+ self.assertEqual(1, self.stats_client.incr.call_count)
258+ self.assertEqual(
259+ self.stats_client.incr.call_args_list[0][0],
260+ ('build_upload.count,job_type=STORE_UPLOAD'',success=True',))
261
262 def test_run_failed(self):
263 # A failed run sets the store upload status to FAILED.
264@@ -204,6 +211,11 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
265 self.assertEqual([], pop_notifications())
266 self.assertWebhookDeliveries(
267 snapbuild, ["Pending", "Failed to upload"], logger)
268+ # check we got stats in statsd
269+ self.assertEqual(1, self.stats_client.incr.call_count)
270+ self.assertEqual(
271+ self.stats_client.incr.call_args_list[0][0],
272+ ('build_upload.count,job_type=STORE_UPLOAD'',success=False',))
273
274 def test_run_unauthorized_notifies(self):
275 # A run that gets 401 from the store sends mail.
276diff --git a/lib/lp/soyuz/model/packagetranslationsuploadjob.py b/lib/lp/soyuz/model/packagetranslationsuploadjob.py
277index 0af6707..0574fd1 100644
278--- a/lib/lp/soyuz/model/packagetranslationsuploadjob.py
279+++ b/lib/lp/soyuz/model/packagetranslationsuploadjob.py
280@@ -33,6 +33,7 @@ from lp.services.job.runner import BaseRunnableJob
281 from lp.services.librarian.interfaces import ILibraryFileAliasSet
282 from lp.services.librarian.utils import filechunks
283 from lp.services.mail.sendmail import format_address_for_person
284+from lp.services.statsd import UploadJobStatsMixin
285 from lp.soyuz.interfaces.packagetranslationsuploadjob import (
286 IPackageTranslationsUploadJob,
287 IPackageTranslationsUploadJobSource,
288@@ -82,7 +83,7 @@ class PackageTranslationsUploadJobDerived(
289 config = config.IPackageTranslationsUploadJobSource
290
291 def __init__(self, job):
292- assert job.base_job_type == JobType.UPLOAD_PACKAGE_TRANSLATIONS
293+ assert job.base_job_type == self.class_job_type
294 self.job = job
295 self.context = self
296
297@@ -97,7 +98,7 @@ class PackageTranslationsUploadJobDerived(
298 def create(cls, distroseries, libraryfilealias, sourcepackagename,
299 requester):
300 job = Job(
301- base_job_type=JobType.UPLOAD_PACKAGE_TRANSLATIONS,
302+ base_job_type=cls.class_job_type,
303 requester=requester,
304 base_json_data=json.dumps(
305 {'distroseries': distroseries.id,
306@@ -112,7 +113,7 @@ class PackageTranslationsUploadJobDerived(
307 def iterReady(cls):
308 jobs = IStore(Job).find(
309 Job, Job.id.is_in(Job.ready_jobs),
310- Job.base_job_type == JobType.UPLOAD_PACKAGE_TRANSLATIONS)
311+ Job.base_job_type == cls.class_job_type)
312 return (cls(job) for job in jobs)
313
314 def getOperationDescription(self):
315@@ -151,7 +152,10 @@ class PackageTranslationsUploadJobDerived(
316
317 @implementer(IPackageTranslationsUploadJob)
318 @provider(IPackageTranslationsUploadJobSource)
319-class PackageTranslationsUploadJob(PackageTranslationsUploadJobDerived):
320+class PackageTranslationsUploadJob(
321+ PackageTranslationsUploadJobDerived, UploadJobStatsMixin):
322+
323+ class_job_type = JobType.UPLOAD_PACKAGE_TRANSLATIONS
324
325 def attachTranslationFiles(self, by_maintainer):
326 distroseries = self.distroseries
327@@ -181,3 +185,4 @@ class PackageTranslationsUploadJob(PackageTranslationsUploadJobDerived):
328
329 def run(self):
330 self.attachTranslationFiles(True)
331+ self.reportStats(True)
332diff --git a/lib/lp/soyuz/tests/test_packagetranslationsuploadjob.py b/lib/lp/soyuz/tests/test_packagetranslationsuploadjob.py
333index cef16f6..2ff0f90 100644
334--- a/lib/lp/soyuz/tests/test_packagetranslationsuploadjob.py
335+++ b/lib/lp/soyuz/tests/test_packagetranslationsuploadjob.py
336@@ -14,6 +14,7 @@ from lp.services.features.testing import FeatureFixture
337 from lp.services.job.interfaces.job import JobStatus
338 from lp.services.job.tests import block_on_job
339 from lp.services.mail.sendmail import format_address_for_person
340+from lp.services.statsd.tests import StatsMixin
341 from lp.services.tarfile_helpers import LaunchpadWriteTarFile
342 from lp.soyuz.interfaces.packagetranslationsuploadjob import (
343 IPackageTranslationsUploadJob,
344@@ -77,7 +78,7 @@ class LocalTestHelper(TestCaseWithFactory):
345 return self.factory.makeLibraryFileAlias(content=tarfile_content)
346
347
348-class TestPackageTranslationsUploadJob(LocalTestHelper):
349+class TestPackageTranslationsUploadJob(LocalTestHelper, StatsMixin):
350
351 layer = LaunchpadZopelessLayer
352
353@@ -147,6 +148,19 @@ class TestPackageTranslationsUploadJob(LocalTestHelper):
354 # Check if the file in tar_content is queued:
355 self.assertTrue("po/foobar.pot", entries_in_queue[0].path)
356
357+ def test_stats(self):
358+ self.setUpStats()
359+ _, _, job = self.makeJob()
360+ method = FakeMethod()
361+ removeSecurityProxy(job).attachTranslationFiles = method
362+ transaction.commit()
363+ _, job.run()
364+ self.assertEqual(1, self.stats_client.incr.call_count)
365+ self.assertEqual(
366+ self.stats_client.incr.call_args_list[0][0],
367+ ('build_upload.count,job_type=UPLOAD_PACKAGE_TRANSLATIONS'
368+ ',success=True',))
369+
370
371 class TestViaCelery(LocalTestHelper):
372 """PackageTranslationsUploadJob runs under Celery."""

Subscribers

People subscribed via source and target branches

to status/vote changes: