feat: capture metrics during email validation (#16739)

Author: miketheman

bin/tests

@@ -45,7 +45,7 @@ if [ "${COVERAGE:-yes}" != "no" ]; then
   python -m coverage run -m pytest --strict-markers "${COMMAND_ARGS[@]}"
   python -m coverage combine
   python -m coverage html --show-contexts
-  python -m coverage report -m --fail-under 100
+  python -m coverage report -m --fail-under 100 --skip-covered
 else
   python -m pytest --strict-markers "${COMMAND_ARGS[@]}"
 fi;

pyproject.toml

@@ -28,9 +28,6 @@ exclude_lines = [
   "if (typing\\.)?TYPE_CHECKING:",
 ]
 
-# Don't show us anything that's already 100% covered.
-skip_covered = true
-
 [tool.curlylint]
 include = '\.(html|jinja|txt)$'
 # For jinja's i18n extension:

tests/conftest.py

@@ -212,6 +212,7 @@ def pyramid_request(pyramid_services, jinja, remote_addr, remote_addr_hashed):
     dummy_request._unauthenticated_userid = None
     dummy_request.user = None
     dummy_request.oidc_publisher = None
+    dummy_request.metrics = dummy_request.find_service(IMetricsService)
 
     dummy_request.registry.registerUtility(jinja, IJinja2Environment, name=".jinja2")
 

tests/unit/accounts/test_forms.py

@@ -414,7 +414,7 @@ def mock_validate_email(email, check_deliverability=True, *args, **kwargs):
 
 
 class TestRegistrationForm:
-    def test_validate(self):
+    def test_validate(self, metrics):
         captcha_service = pretend.stub(
             enabled=False,
             verify_response=pretend.call_recorder(lambda _: None),
@@ -432,7 +432,8 @@ def test_validate(self):
 
         form = forms.RegistrationForm(
             request=pretend.stub(
-                db=pretend.stub(query=lambda *a: pretend.stub(scalar=lambda: False))
+                db=pretend.stub(query=lambda *a: pretend.stub(scalar=lambda: False)),
+                metrics=metrics,
             ),
             formdata=MultiDict(
                 {
@@ -538,10 +539,11 @@ def test_invalid_email_error(self, pyramid_request, email):
             str(form.email.errors.pop()) == "The email address isn't valid. Try again."
         )
 
-    def test_exotic_email_success(self):
+    def test_exotic_email_success(self, metrics):
         form = forms.RegistrationForm(
             request=pretend.stub(
-                db=pretend.stub(query=lambda *a: pretend.stub(scalar=lambda: False))
+                db=pretend.stub(query=lambda *a: pretend.stub(scalar=lambda: False)),
+                metrics=metrics,
             ),
             formdata=MultiDict({"email": "[email protected]"}),
             user_service=pretend.stub(

tests/unit/manage/test_forms.py

@@ -170,12 +170,13 @@ def test_name_too_long(self, pyramid_config):
 
 
 class TestAddEmailForm:
-    def test_validate(self):
+    def test_validate(self, metrics):
         user_id = pretend.stub()
         user_service = pretend.stub(find_userid_by_email=lambda _: None)
         form = forms.AddEmailForm(
             request=pretend.stub(
-                db=pretend.stub(query=lambda *a: pretend.stub(scalar=lambda: False))
+                db=pretend.stub(query=lambda *a: pretend.stub(scalar=lambda: False)),
+                metrics=metrics,
             ),
             formdata=MultiDict({"email": "[email protected]"}),
             user_id=user_id,

tests/unit/metrics/test_init.py

@@ -19,6 +19,7 @@
     DataDogMetrics,
     IMetricsService,
     NullMetrics,
+    _metrics,
     event_handlers,
     includeme,
     views,
@@ -30,6 +31,7 @@ def test_include_defaults_to_null():
         registry=pretend.stub(settings={}),
         maybe_dotted=lambda i: i,
         register_service_factory=pretend.call_recorder(lambda factory, iface: None),
+        add_request_method=pretend.call_recorder(lambda fn, name, reify: None),
         add_subscriber=pretend.call_recorder(lambda handler, event: None),
         add_view_deriver=pretend.call_recorder(lambda deriver, under: None),
     )
@@ -49,6 +51,9 @@ def test_include_defaults_to_null():
     assert config.add_view_deriver.calls == [
         pretend.call(views.timing_view, under=viewderivers.INGRESS)
     ]
+    assert config.add_request_method.calls == [
+        pretend.call(_metrics, name="metrics", reify=True)
+    ]
 
 
 def test_include_sets_class():
@@ -60,6 +65,7 @@ def test_include_sets_class():
             pth
         ],
         register_service_factory=pretend.call_recorder(lambda factory, iface: None),
+        add_request_method=pretend.call_recorder(lambda fn, name, reify: None),
         add_subscriber=pretend.call_recorder(lambda handler, event: None),
         add_view_deriver=pretend.call_recorder(lambda deriver, under: None),
     )
@@ -79,3 +85,10 @@ def test_include_sets_class():
     assert config.add_view_deriver.calls == [
         pretend.call(views.timing_view, under=viewderivers.INGRESS)
     ]
+    assert config.add_request_method.calls == [
+        pretend.call(_metrics, name="metrics", reify=True)
+    ]
+
+
+def test_finds_service(pyramid_request):
+    assert _metrics(pyramid_request) == pyramid_request.find_service(IMetricsService)

warehouse/accounts/forms.py

@@ -282,6 +282,10 @@ def validate_email(self, field):
         try:
             resp = email_validator.validate_email(field.data, check_deliverability=True)
         except email_validator.EmailNotValidError as e:
+            self.request.metrics.increment(
+                "warehouse.accounts.forms.validate_email",
+                tags=["result:invalid", "reason:email_validator"],
+            )
             raise wtforms.validators.ValidationError(
                 self.request._("The email address isn't valid. Try again.")
             ) from e
@@ -310,6 +314,10 @@ def validate_email(self, field):
                 )
             ).scalar()
         ):
+            self.request.metrics.increment(
+                "warehouse.accounts.forms.validate_email",
+                tags=["result:invalid", "reason:prohibited_domain"],
+            )
             raise wtforms.validators.ValidationError(
                 self.request._(
                     "You can't use an email address from this domain. Use a "
@@ -321,20 +329,33 @@ def validate_email(self, field):
         userid = self.user_service.find_userid_by_email(field.data)
 
         if userid and userid == self.user_id:
+            self.request.metrics.increment(
+                "warehouse.accounts.forms.validate_email",
+                tags=["result:invalid", "reason:email_in_use_by_self"],
+            )
             raise wtforms.validators.ValidationError(
                 self.request._(
                     "This email address is already being used by this account. "
                     "Use a different email."
                 )
             )
         if userid:
+            self.request.metrics.increment(
+                "warehouse.accounts.forms.validate_email",
+                tags=["result:invalid", "reason:email_in_use_by_other"],
+            )
             raise wtforms.validators.ValidationError(
                 self.request._(
                     "This email address is already being used "
                     "by another account. Use a different email."
                 )
             )
 
+        self.request.metrics.increment(
+            "warehouse.accounts.forms.validate_email",
+            tags=["result:valid"],
+        )
+
 
 class HoneypotMixin:
     """A mixin to catch spammers. This field should always be blank"""

warehouse/locale/messages.pot

@@ -14,7 +14,7 @@ msgstr ""
 msgid "Locale updated"
 msgstr ""
 
-#: warehouse/accounts/forms.py:52 warehouse/accounts/forms.py:286
+#: warehouse/accounts/forms.py:52 warehouse/accounts/forms.py:290
 msgid "The email address isn't valid. Try again."
 msgstr ""
 
@@ -74,47 +74,47 @@ msgstr ""
 msgid "The email address is too long. Try again."
 msgstr ""
 
-#: warehouse/accounts/forms.py:314
+#: warehouse/accounts/forms.py:322
 msgid "You can't use an email address from this domain. Use a different email."
 msgstr ""
 
-#: warehouse/accounts/forms.py:325
+#: warehouse/accounts/forms.py:337
 msgid ""
 "This email address is already being used by this account. Use a different"
 " email."
 msgstr ""
 
-#: warehouse/accounts/forms.py:332
+#: warehouse/accounts/forms.py:348
 msgid ""
 "This email address is already being used by another account. Use a "
 "different email."
 msgstr ""
 
-#: warehouse/accounts/forms.py:367 warehouse/manage/forms.py:139
+#: warehouse/accounts/forms.py:388 warehouse/manage/forms.py:139
 msgid "The name is too long. Choose a name with 100 characters or less."
 msgstr ""
 
-#: warehouse/accounts/forms.py:374
+#: warehouse/accounts/forms.py:395
 msgid "URLs are not allowed in the name field."
 msgstr ""
 
-#: warehouse/accounts/forms.py:463
+#: warehouse/accounts/forms.py:484
 msgid "Invalid TOTP code."
 msgstr ""
 
-#: warehouse/accounts/forms.py:480
+#: warehouse/accounts/forms.py:501
 msgid "Invalid WebAuthn assertion: Bad payload"
 msgstr ""
 
-#: warehouse/accounts/forms.py:549
+#: warehouse/accounts/forms.py:570
 msgid "Invalid recovery code."
 msgstr ""
 
-#: warehouse/accounts/forms.py:558
+#: warehouse/accounts/forms.py:579
 msgid "Recovery code has been previously used."
 msgstr ""
 
-#: warehouse/accounts/forms.py:588
+#: warehouse/accounts/forms.py:609
 msgid "The username isn't valid. Try again."
 msgstr ""
 

warehouse/metrics/__init__.py

@@ -11,6 +11,7 @@
 # limitations under the License.
 
 from pyramid import events, viewderivers
+from pyramid.request import Request
 from pyramid_retry import IBeforeRetry
 
 from warehouse.metrics import event_handlers
@@ -21,6 +22,10 @@
 __all__ = ["IMetricsService", "NullMetrics", "DataDogMetrics", "includeme"]
 
 
+def _metrics(request: Request) -> IMetricsService:
+    return request.find_service(IMetricsService)
+
+
 def includeme(config):
     # Register our metrics service.
     metrics_class = config.maybe_dotted(
@@ -38,3 +43,6 @@ def includeme(config):
 
     # Register our view deriver that ensures we get our view timed.
     config.add_view_deriver(timing_view, under=viewderivers.INGRESS)
+
+    # Add the metrics service to the request.
+    config.add_request_method(_metrics, name="metrics", reify=True)