diff --git a/swh/loader/core/loader.py b/swh/loader/core/loader.py --- a/swh/loader/core/loader.py +++ b/swh/loader/core/loader.py @@ -3,18 +3,17 @@ # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information -import contextlib import datetime import hashlib import logging import os import time -from typing import Any, Dict, Iterable, List, Optional +from typing import Any, ContextManager, Dict, Iterable, List, Optional, Union import sentry_sdk from swh.core.config import load_from_envvar -from swh.core.statsd import statsd +from swh.core.statsd import Statsd from swh.loader.core.metadata_fetchers import CredentialsType, get_fetchers_for_lister from swh.loader.exception import NotFound from swh.model.model import ( @@ -39,8 +38,6 @@ "max_content_size": 100 * 1024 * 1024, } -STATSD_PREFIX = "swh_loader" - class BaseLoader: """Base class for (D)VCS loaders (e.g Svn, Git, Mercurial, ...) or PackageLoader (e.g @@ -134,6 +131,10 @@ self.parent_origins = None + self.statsd = Statsd( + namespace="swh_loader", constant_tags={"visit_type": self.visit_type} + ) + @classmethod def from_config(cls, storage: Dict[str, Any], **config: Any): """Instantiate a loader from a configuration dict. @@ -494,32 +495,33 @@ return metadata - @contextlib.contextmanager - def statsd_timed(self, name, tags={}): - with statsd.timed( - f"{STATSD_PREFIX}_operation_duration_seconds", - tags={"visit_type": self.visit_type, "operation": name, **tags}, - ): - yield - - def statsd_timing(self, name, value, tags={}): - statsd.timing( - f"{STATSD_PREFIX}_operation_duration_seconds", - value, - tags={"visit_type": self.visit_type, "operation": name, **tags}, + def statsd_timed(self, name: str, tags: Dict[str, Any] = {}) -> ContextManager: + """ + Wrapper for :meth:`swh.core.statsd.Statsd.timed`, which uses the standard + metric name and tags for loaders. + """ + return self.statsd.timed( + "operation_duration_seconds", tags={"operation": name, **tags} ) - def statsd_average(self, name, value, tags={}): - statsd.increment( - f"{STATSD_PREFIX}_{name}_sum", - value, - tags={"visit_type": self.visit_type, **tags}, - ) - statsd.increment( - f"{STATSD_PREFIX}_{name}_count", - tags={"visit_type": self.visit_type, **tags}, + def statsd_timing(self, name: str, value: float, tags: Dict[str, Any] = {}) -> None: + """ + Wrapper for :meth:`swh.core.statsd.Statsd.timing`, which uses the standard + metric name and tags for loaders. + """ + self.statsd.timing( + "operation_duration_seconds", value, tags={"operation": name, **tags} ) + def statsd_average( + self, name: str, value: Union[int, float], tags: Dict[str, Any] = {} + ) -> None: + """Increments both ``{name}_sum`` (by the ``value``) and ``{name}_count`` + (by ``1``), allowing to prometheus to compute the average ``value`` over + time.""" + self.statsd.increment(f"{name}_sum", value, tags=tags) + self.statsd.increment(f"{name}_count", tags=tags) + class DVCSLoader(BaseLoader): """This base class is a pattern for dvcs loaders (e.g. git, mercurial). diff --git a/swh/loader/core/tests/test_loader.py b/swh/loader/core/tests/test_loader.py --- a/swh/loader/core/tests/test_loader.py +++ b/swh/loader/core/tests/test_loader.py @@ -152,11 +152,11 @@ mocker.patch( "swh.loader.core.metadata_fetchers._fetchers", return_value=[fetcher_cls] ) - statsd_report = mocker.patch("swh.core.statsd.statsd._report") loader = DummyBaseLoader( swh_storage, lister_name="fake-forge", lister_instance_name="" ) + statsd_report = mocker.patch.object(loader.statsd, "_report") result = loader.load() assert result == {"status": "eventful"} @@ -173,25 +173,15 @@ assert loader.parent_origins == [] assert [ - call("swh_loader_metadata_fetchers_sum", "c", 1, {"visit_type": "git"}, 1), - call("swh_loader_metadata_fetchers_count", "c", 1, {"visit_type": "git"}, 1), - call( - "swh_loader_metadata_parent_origins_sum", - "c", - 0, - {"fetcher": "fake-forge", "visit_type": "git"}, - 1, - ), - call( - "swh_loader_metadata_parent_origins_count", - "c", - 1, - {"fetcher": "fake-forge", "visit_type": "git"}, - 1, - ), - call("swh_loader_metadata_objects_sum", "c", 1, {"visit_type": "git"}, 1), - call("swh_loader_metadata_objects_count", "c", 1, {"visit_type": "git"}, 1), - ] == [c for c in statsd_report.mock_calls if "_metadata_" in c[1][0]] + call("metadata_fetchers_sum", "c", 1, {}, 1), + call("metadata_fetchers_count", "c", 1, {}, 1), + call("metadata_parent_origins_sum", "c", 0, {"fetcher": "fake-forge"}, 1), + call("metadata_parent_origins_count", "c", 1, {"fetcher": "fake-forge"}, 1), + call("metadata_objects_sum", "c", 1, {}, 1), + call("metadata_objects_count", "c", 1, {}, 1), + ] == [c for c in statsd_report.mock_calls if "metadata_" in c[1][0]] + assert loader.statsd.namespace == "swh_loader" + assert loader.statsd.constant_tags == {"visit_type": "git"} def test_base_loader_with_unknown_lister_name(swh_storage, mocker): @@ -219,11 +209,11 @@ mocker.patch( "swh.loader.core.metadata_fetchers._fetchers", return_value=[fetcher_cls] ) - statsd_report = mocker.patch("swh.core.statsd.statsd._report") loader = DummyBaseLoader( swh_storage, lister_name="fake-forge", lister_instance_name="" ) + statsd_report = mocker.patch.object(loader.statsd, "_report") result = loader.load() assert result == {"status": "eventful"} @@ -240,25 +230,15 @@ assert loader.parent_origins == [PARENT_ORIGIN] assert [ - call("swh_loader_metadata_fetchers_sum", "c", 1, {"visit_type": "git"}, 1), - call("swh_loader_metadata_fetchers_count", "c", 1, {"visit_type": "git"}, 1), - call( - "swh_loader_metadata_parent_origins_sum", - "c", - 1, - {"fetcher": "fake-forge", "visit_type": "git"}, - 1, - ), - call( - "swh_loader_metadata_parent_origins_count", - "c", - 1, - {"fetcher": "fake-forge", "visit_type": "git"}, - 1, - ), - call("swh_loader_metadata_objects_sum", "c", 1, {"visit_type": "git"}, 1), - call("swh_loader_metadata_objects_count", "c", 1, {"visit_type": "git"}, 1), - ] == [c for c in statsd_report.mock_calls if "_metadata_" in c[1][0]] + call("metadata_fetchers_sum", "c", 1, {}, 1), + call("metadata_fetchers_count", "c", 1, {}, 1), + call("metadata_parent_origins_sum", "c", 1, {"fetcher": "fake-forge"}, 1), + call("metadata_parent_origins_count", "c", 1, {"fetcher": "fake-forge"}, 1), + call("metadata_objects_sum", "c", 1, {}, 1), + call("metadata_objects_count", "c", 1, {}, 1), + ] == [c for c in statsd_report.mock_calls if "metadata_" in c[1][0]] + assert loader.statsd.namespace == "swh_loader" + assert loader.statsd.constant_tags == {"visit_type": "git"} def test_dvcs_loader(swh_storage): @@ -363,9 +343,8 @@ return meth - statsd_report = mocker.patch("swh.core.statsd.statsd._report") - loader = TimedLoader(swh_storage, origin_url="http://example.org/hello.git") + statsd_report = mocker.patch.object(loader.statsd, "_report") loader.load() if success: @@ -386,18 +365,16 @@ # even if not perfect. assert statsd_report.mock_calls == [ call( - "swh_loader_operation_duration_seconds", + "operation_duration_seconds", "ms", value * 1000, - { - "visit_type": "my-visit-type", - "operation": key, - **expected_tags.get(key, {}), - }, + {"operation": key, **expected_tags.get(key, {})}, 1, ) for (key, value) in runtimes.items() ] + assert loader.statsd.namespace == "swh_loader" + assert loader.statsd.constant_tags == {"visit_type": "my-visit-type"} class DummyDVCSLoaderExc(DummyDVCSLoader): diff --git a/swh/loader/package/tests/test_loader.py b/swh/loader/package/tests/test_loader.py --- a/swh/loader/package/tests/test_loader.py +++ b/swh/loader/package/tests/test_loader.py @@ -80,7 +80,7 @@ def test_loader_origin_visit_failure(swh_storage): """Failure to add origin or origin visit should failed immediately""" - loader = PackageLoader(swh_storage, "some-url") + loader = StubPackageLoader(swh_storage, "some-url") loader.storage = FakeStorage() actual_load_status = loader.load() @@ -111,7 +111,7 @@ ) storage.release_add([rel1, rel2]) - loader = PackageLoader(storage, "http://example.org/") # type: ignore + loader = StubPackageLoader(storage, "http://example.org/") p_info = Mock(wraps=BasePackageInfo(None, None, None)) # type: ignore @@ -160,7 +160,7 @@ synthetic=False, ) - loader = PackageLoader(storage, "http://example.org/") # type: ignore + loader = StubPackageLoader(storage, "http://example.org/") p_info = Mock(wraps=BasePackageInfo(None, None, None)) # type: ignore