Repository: scrapinghub/spidermon (language: python)

Issue:
Built-in monitor to compare item count with previous jobs
This monitor is more useful when we are running the spider in [Scrapy Cloud](https://www.zyte.com/scrapy-cloud/). The objective is to compare the number of extracted items in current job with the number of items extracted in previous jobs.

- Get only `finished` jobs.
- Limit the number of jobs to be considered (configurable)
- if possible add more ways to decide which jobs to consider, not only the finished ones. This is useful when we have a `finished` job, but we want to exclude that execution for some reason (it failed y other reason for example)
- Configurable percentage to allow variations ("require that this job has at least 90% of items from the average 10 last jobs)


Discussion / hints:
Hi guys, please consider job comparison based on 
1. tags. It can be one or several tags
2. arguments. It can be all arguments or several arguments (if it's possible)


Fix patch:
diff --git a/docs/source/monitors.rst b/docs/source/monitors.rst
index c33564bb..f6955e95 100644
--- a/docs/source/monitors.rst
+++ b/docs/source/monitors.rst
@@ -151,10 +151,10 @@ The Basic Monitors
 Spidermon has some batteries included :)
 
 .. automodule:: spidermon.contrib.scrapy.monitors
-    :members: ItemCountMonitor, ErrorCountMonitor, WarningCountMonitor, FinishReasonMonitor,
+    :members: ItemCountMonitor, ErrorCountMonitor, WarningCountMonitor, CriticalCountMonitor, FinishReasonMonitor,
               UnwantedHTTPCodesMonitor, ItemValidationMonitor, FieldCoverageMonitor,
               RetryCountMonitor, DownloaderExceptionMonitor, SuccessfulRequestsMonitor,
-              TotalRequestsMonitor
+              TotalRequestsMonitor, JobsComparisonMonitor
 
 Is there a **Basic Scrapy Suite** ready to use?
 ------------------------------------------------
diff --git a/setup.py b/setup.py
index 87731771..178b71b9 100644
--- a/setup.py
+++ b/setup.py
@@ -1,4 +1,4 @@
-from setuptools import setup, find_packages
+from setuptools import find_packages, setup
 
 test_requirements = [
     "pytest>=2.7.0",
@@ -34,6 +34,7 @@
         "monitoring": [
             "scrapy",
             "Jinja2",
+            "scrapinghub",
             "slackclient",
             "boto",
             "boto3",
diff --git a/spidermon/contrib/scrapy/extensions.py b/spidermon/contrib/scrapy/extensions.py
index 6c1c58b3..f25be637 100644
--- a/spidermon/contrib/scrapy/extensions.py
+++ b/spidermon/contrib/scrapy/extensions.py
@@ -8,7 +8,7 @@
 from spidermon.python import factory
 from spidermon.python.monitors import ExpressionsMonitor
 from spidermon.utils.field_coverage import calculate_field_coverage
-from spidermon.utils.hubstorage import hs
+from spidermon.utils.zyte import client as hs
 
 
 class Spidermon:
diff --git a/spidermon/contrib/scrapy/monitors.py b/spidermon/contrib/scrapy/monitors.py
index c366d9d2..9b5e54ab 100644
--- a/spidermon/contrib/scrapy/monitors.py
+++ b/spidermon/contrib/scrapy/monitors.py
@@ -1,7 +1,11 @@
 import datetime
+import json
+import math
+import os
 
 from spidermon import Monitor, MonitorSuite, monitors
 from spidermon.exceptions import NotConfigured
+from spidermon.utils import zyte
 from spidermon.utils.settings import getdictorlist
 
 from ..monitors.mixins.spider import SpiderMonitorMixin, StatsMonitorMixin
@@ -15,6 +19,10 @@
 SPIDERMON_MAX_DOWNLOADER_EXCEPTIONS = "SPIDERMON_MAX_DOWNLOADER_EXCEPTIONS"
 SPIDERMON_MIN_SUCCESSFUL_REQUESTS = "SPIDERMON_MIN_SUCCESSFUL_REQUESTS"
 SPIDERMON_MAX_REQUESTS_ALLOWED = "SPIDERMON_MAX_REQUESTS_ALLOWED"
+SPIDERMON_JOBS_COMPARISON = "SPIDERMON_JOBS_COMPARISON"
+SPIDERMON_JOBS_COMPARISON_STATES = "SPIDERMON_JOBS_COMPARISON_STATES"
+SPIDERMON_JOBS_COMPARISON_TAGS = "SPIDERMON_JOBS_COMPARISON_TAGS"
+SPIDERMON_JOBS_COMPARISON_THRESHOLD = "SPIDERMON_JOBS_COMPARISON_THRESHOLD"
 
 
 class BaseScrapyMonitor(Monitor, SpiderMonitorMixin):
@@ -522,6 +530,78 @@ def test_execution_time(self):
         self.assertLess(duration.total_seconds(), max_execution_time, msg=msg)
 
 
+@monitors.name("Jobs Comparison Monitor")
+class JobsComparisonMonitor(BaseStatMonitor):
+    """
+    .. note::
+       This monitor is useful when running jobs in
+       `Zyte's Scrapy Cloud <https://www.zyte.com/scrapy-cloud/>`_.
+
+    Check for a drop in scraped item count compared to previous jobs.
+
+    You need to set the number of previous jobs to compare, using ``SPIDERMON_JOBS_COMPARISON``.
+    The default is ``0`` which disables the monitor. We use the average of the scraped items count.
+
+    You can configure which percentage of the previous item count is the minimum acceptable, by
+    using the setting ``SPIDERMON_JOBS_COMPARISON_THRESHOLD``. We expect a float number between
+    ``0.0`` and ``1.0``. The default is ``0.8``.
+
+    You can filter which jobs to compare based on their states using the
+    ``SPIDERMON_JOBS_COMPARISON_STATES`` setting. The default value is ``("finished",)``.
+
+    You can also filter which jobs to compare based on their tags using the
+    ``SPIDERMON_JOBS_COMPARISON_TAGS`` setting. Among the defined tags we consider only those
+    that are also present in the current job.
+    """
+
+    stat_name = "item_scraped_count"
+    assert_type = ">="
+
+    def _get_jobs(self, states, number_of_jobs, tags=None):
+        return zyte.client.spider.jobs.list(
+            state=states,
+            count=number_of_jobs,
+            filters=dict(has_tag=tags) if tags else None,
+        )
+
+    def _get_tags_to_filter(self):
+        """
+        Return the intersect of the desired tags to filter and
+        the ones from the current job.
+        """
+        desired_tags = self.crawler.settings.getlist(SPIDERMON_JOBS_COMPARISON_TAGS)
+        if not desired_tags:
+            return {}
+
+        current_tags = json.loads(os.environ.get("SHUB_JOB_DATA", "{}")).get("tags")
+        if not current_tags:
+            return {}
+
+        tags_to_filter = set(desired_tags) & set(current_tags)
+        return sorted(tags_to_filter)
+
+    def get_threshold(self):
+        # 1. get the number of jobs and check if monitor is enabled
+        number_of_jobs = self.crawler.settings.getint(SPIDERMON_JOBS_COMPARISON, 0)
+        if not number_of_jobs:
+            self.skipTest("Jobs comparison monitor is disabled")
+
+        # 2. get the previous jobs meeting the defined states and tags
+        states = self.crawler.settings.getlist(
+            SPIDERMON_JOBS_COMPARISON_STATES, ("finished",)
+        )
+        tags = self._get_tags_to_filter()
+        jobs = self._get_jobs(states, number_of_jobs, tags)
+
+        # 3. get the average item count and calculate the acceptable threshold
+        previous_count = sum(job.get("items", 0) for job in jobs) / len(jobs)
+        threshold = self.crawler.settings.getfloat(
+            SPIDERMON_JOBS_COMPARISON_THRESHOLD, 0.8
+        )
+        expected_item_extracted = math.ceil(previous_count * threshold)
+        return expected_item_extracted
+
+
 class SpiderCloseMonitorSuite(MonitorSuite):
     """This Monitor Suite implements the following monitors:
 
diff --git a/spidermon/utils/hubstorage.py b/spidermon/utils/hubstorage.py
deleted file mode 100644
index d14e2c08..00000000
--- a/spidermon/utils/hubstorage.py
+++ /dev/null
@@ -1,70 +0,0 @@
-"""
-Module to hold a reference to singleton Hubstorage client and Job instance
-"""
-import os
-from codecs import decode
-
-
-from scrapinghub import HubstorageClient
-
-
-class _Hubstorage:
-    def __init__(self):
-        self.available = "SHUB_JOBKEY" in os.environ
-        self._client = None
-        self._project = None
-        self._job = None
-        if self.available:
-            self.job_key = os.environ["SHUB_JOBKEY"]
-            self._project_id, self._spider_id, self._job_id = map(
-                int, self.job_key.split("/")
-            )
-        else:
-            self._project_id = None
-            self._spider_id = None
-            self._job_id = None
-
-    @property
-    def auth(self):
-        return decode(os.environ["SHUB_JOBAUTH"], "hex_codec").decode("utf-8")
-
-    @property
-    def endpoint(self):
-        return os.environ.get("SHUB_STORAGE")
-
-    @property
-    def project_id(self):
-        return self._project_id
-
-    @property
-    def spider_id(self):
-        return self._spider_id
-
-    @property
-    def job_id(self):
-        return self._job_id
-
-    @property
-    def client(self):
-        if self._client is None:
-            self._client = HubstorageClient(endpoint=self.endpoint, auth=self.auth)
-        return self._client
-
-    @property
-    def project(self):
-        if self._project is None:
-            self._project = self.client.get_project(str(self.project_id))
-        return self._project
-
-    @property
-    def job(self):
-        if self._job is None:
-            self._job = self.project.get_job((self.spider_id, self.job_id))
-        return self._job
-
-    def close(self):
-        if self._client is not None:
-            self._client.close()
-
-
-hs = _Hubstorage()
diff --git a/spidermon/utils/zyte.py b/spidermon/utils/zyte.py
new file mode 100644
index 00000000..85b25796
--- /dev/null
+++ b/spidermon/utils/zyte.py
@@ -0,0 +1,81 @@
+"""
+Module to hold a singleton to Zyte's Scrapy Cloud client.
+
+It expects that "SHUB_JOBKEY" be present as environment variable.
+Also, it will look up for Scrapy Cloud API key in `SHUB_APIKEY` Scrapy
+setting, then for the environment variables `SH_APIKEY` and `SHUB_JOBAUTH`.
+Note that "SHUB_JOBAUTH" can't access all API endpoints.
+"""
+import os
+
+try:
+    from scrapinghub import ScrapinghubClient
+
+    HAS_CLIENT = True
+except ImportError:
+    HAS_CLIENT = False
+
+
+class Client:
+    def __init__(self):
+        self.available = HAS_CLIENT and "SHUB_JOBKEY" in os.environ
+        self._client = None
+        self._project = None
+        self._spider = None
+        self._job = None
+        if self.available:
+            self.job_key = os.environ["SHUB_JOBKEY"]
+            self.project_id, self.spider_id, self.job_id = map(
+                int, self.job_key.split("/")
+            )
+        else:
+            self.project_id = None
+            self.spider_id = None
+            self.job_id = None
+
+    @property
+    def client(self):
+        if not self._client:
+            self._client = ScrapinghubClient(self._apikey())
+        return self._client
+
+    def _apikey(self):
+        from scrapy.utils.project import get_project_settings
+
+        apikey = (
+            get_project_settings().get("SHUB_APIKEY")
+            or os.environ.get("SH_APIKEY")
+            or os.environ.get("SHUB_JOBAUTH")
+        )
+        if not apikey:
+            raise RuntimeError(
+                "No Scrapy Cloud API key found. Please set `SHUB_APIKEY` in Scrapy settings,"
+                " or either `SH_APIKEY` or `SHUB_JOBAUTH` environment variables `."
+            )
+        return apikey
+
+    @property
+    def project(self):
+        if not self._project:
+            self._project = self.client.get_project(str(self.project_id))
+        return self._project
+
+    @property
+    def spider(self):
+        if not self._spider:
+            spider_name = self.job.metadata.get("spider")
+            self._spider = self.project.spiders.get(spider_name)
+        return self._spider
+
+    @property
+    def job(self):
+        if not self._job:
+            self._job = self.client.get_job(self.job_key)
+        return self._job
+
+    def close(self):
+        if self._client:
+            self._client.close()
+
+
+client = Client()


Test patch:
diff --git a/tests/contrib/scrapy/monitors/test_jobs_comparison_monitor.py b/tests/contrib/scrapy/monitors/test_jobs_comparison_monitor.py
new file mode 100644
index 00000000..95d31756
--- /dev/null
+++ b/tests/contrib/scrapy/monitors/test_jobs_comparison_monitor.py
@@ -0,0 +1,109 @@
+from unittest.mock import Mock
+
+import pytest
+from spidermon import MonitorSuite
+from spidermon.contrib.scrapy import monitors
+from spidermon.contrib.scrapy.monitors import (
+    SPIDERMON_JOBS_COMPARISON,
+    SPIDERMON_JOBS_COMPARISON_STATES,
+    SPIDERMON_JOBS_COMPARISON_TAGS,
+    SPIDERMON_JOBS_COMPARISON_THRESHOLD,
+    JobsComparisonMonitor,
+)
+
+
+@pytest.fixture
+def mock_jobs(previous_counts):
+    return Mock(return_value=[dict(items=c) for c in previous_counts])
+
+
+@pytest.fixture
+def mock_suite(mock_jobs, monkeypatch):
+    monkeypatch.setattr(JobsComparisonMonitor, "_get_jobs", mock_jobs)
+    return MonitorSuite(monitors=[JobsComparisonMonitor])
+
+
+@pytest.fixture
+def mock_suite_and_zyte_client(monkeypatch):
+    monkeypatch.setenv("SHUB_JOB_DATA", '{"tags":["tag1","tag2","tag3"]}')
+    monkeypatch.setattr(monitors, "zyte", Mock())
+    monitors.zyte.client.spider.jobs.list.return_value = []
+    return MonitorSuite(monitors=[monitors.JobsComparisonMonitor]), monitors.zyte
+
+
+@pytest.mark.parametrize(
+    ["number_of_jobs", "item_count", "previous_counts", "expected"],
+    [
+        (0, 1, [1], False),
+        (1, 0, [1], True),
+        (5, 0, [1], True),
+    ],
+)
+def test_jobs_comparison_monitor_is_enabled(
+    make_data, mock_suite, item_count, number_of_jobs, expected
+):
+    data = make_data({SPIDERMON_JOBS_COMPARISON: number_of_jobs})
+    data["stats"]["item_scraped_count"] = item_count
+    runner = data.pop("runner")
+    runner.run(mock_suite, **data)
+
+    is_enabled = runner.result.monitor_results[0].error is not None
+    assert (
+        is_enabled == expected
+        or runner.result.monitor_results[0].reason
+        == "Jobs comparison monitor is disabled"
+    )
+
+
+@pytest.mark.parametrize(
+    ["item_count", "previous_counts", "threshold", "should_raise"],
+    [
+        (90, [100], 0.9, False),
+        (90, [100, 101], 0.9, True),
+        (80, [100, 101, 99], 0.8, False),
+        (80, [100, 101, 99, 102], 0.8, True),
+    ],
+)
+def test_jobs_comparison_monitor_threshold(
+    make_data, mock_suite, item_count, threshold, should_raise
+):
+    data = make_data(
+        {SPIDERMON_JOBS_COMPARISON: 1, SPIDERMON_JOBS_COMPARISON_THRESHOLD: threshold}
+    )
+    data["stats"]["item_scraped_count"] = item_count
+    runner = data.pop("runner")
+    runner.run(mock_suite, **data)
+
+    assert should_raise == bool(runner.result.monitor_results[0].error)
+
+
+@pytest.mark.parametrize(
+    ["states", "number_of_jobs", "tags"],
+    [
+        (("finished",), 5, ("tag1", "tag2")),
+        (("foo", "bar"), 10, ("tag3",)),
+    ],
+)
+def test_arguments_passed_to_zyte_client(
+    make_data,
+    mock_suite_and_zyte_client,
+    states,
+    number_of_jobs,
+    tags,
+):
+    data = make_data(
+        {
+            SPIDERMON_JOBS_COMPARISON: number_of_jobs,
+            SPIDERMON_JOBS_COMPARISON_STATES: states,
+            SPIDERMON_JOBS_COMPARISON_TAGS: tags,
+        }
+    )
+    suite, zyte_module = mock_suite_and_zyte_client
+    runner = data.pop("runner")
+    runner.run(suite, **data)
+
+    zyte_module.client.spider.jobs.list.assert_called_with(
+        state=list(states),
+        count=number_of_jobs,
+        filters={"has_tag": list(tags)},
+    )
diff --git a/tests/utils/test_zyte.py b/tests/utils/test_zyte.py
new file mode 100644
index 00000000..49d19700
--- /dev/null
+++ b/tests/utils/test_zyte.py
@@ -0,0 +1,76 @@
+from unittest import mock
+
+import pytest
+from spidermon.utils import zyte
+
+
+@pytest.fixture
+def no_env_mock_module(monkeypatch):
+    monkeypatch.delenv("SHUB_JOBKEY", raising=False)
+    monkeypatch.setattr(zyte, "ScrapinghubClient", mock.MagicMock())
+    return zyte
+
+
+@pytest.fixture
+def mock_module(monkeypatch):
+    monkeypatch.setenv("SHUB_JOBKEY", "123/456/789")
+    monkeypatch.setenv("SH_APIKEY", "foobar")
+    monkeypatch.setattr(zyte, "ScrapinghubClient", mock.MagicMock())
+    return zyte
+
+
+def test_client_creation_no_env(no_env_mock_module):
+    client = no_env_mock_module.Client()
+    assert client.available == False
+    assert client.project_id == None
+    assert client.spider_id == None
+    assert client.job_id == None
+
+
+def test_client_creation(mock_module):
+    client = mock_module.Client()
+    assert client.available == True
+    assert client.project_id == 123
+    assert client.spider_id == 456
+    assert client.job_id == 789
+
+
+def test_client_property_project(mock_module):
+    client = mock_module.Client()
+
+    assert client._project == None
+    client.project
+    assert client._project != None
+    client._client.get_project.assert_called_with("123")
+
+
+def test_client_property_job(mock_module):
+    client = mock_module.Client()
+
+    assert client._job == None
+    client.job
+    assert client._job != None
+    client._client.get_job.assert_called_with("123/456/789")
+
+
+def test_client_property_spider(mock_module):
+    client = mock_module.Client()
+    client._job = mock.Mock()
+    client._job.metadata.get.return_value = "my_awesome_spider"
+
+    assert client._spider == None
+    client.spider
+    assert client._project != None
+    assert client._job != None
+    assert client._spider != None
+
+    client._job.metadata.get.assert_called_with("spider")
+    client._project.spiders.get.assert_called_with("my_awesome_spider")
+
+
+def test_client_close(mock_module):
+    client = mock_module.Client()
+
+    client.client
+    client.close()
+    client._client.close.assert_called()