Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create Base Stat Monitor class to help creating custom monitors #325

Merged
merged 4 commits into from
Dec 14, 2021

Conversation

rennerocha
Copy link
Collaborator

Solves #321

This PR includes a easier way to create custom monitors based on simple comparisons between a job stat and a threshold. We are talking usually about numerical values, but for EQUAL and NOTEQUAL, this can be strings as well.

A monitor that checks if a stat called test_stat is greater than or equal to a value defined in a project setting called TEST_STAT_THRESHOLDcan be defined as follows:

    class TestStatMonitor(BaseStatMonitor):
        stat_name = "test_stat"
        threshold_setting = "TEST_STAT_THRESHOLD"
        assert_type = AssertionType.GTE

The threshold value can also be defined by a method, so we are able to create more complex rules to define what the threshold should be. Some ideas that can be accomplished using that method:

  • Compare the number of items returned with the number of the latest job
  • Define the threshold as a percentage of the number of input URLs against the number of requests
  • etc
    class TestStatMonitor(BaseStatMonitor):
        stat_name = "test_stat"
        assert_type = AssertionType.GTE

        def get_threshold(self):
            # Do something to get the value
            return threshold

We still need to add the docs for this new feature and rewrite most of the built-in monitors to use this new structure.

@codecov
Copy link

codecov bot commented Nov 29, 2021

Codecov Report

Merging #325 (dac6ebb) into master (d58090d) will increase coverage by 0.37%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #325      +/-   ##
==========================================
+ Coverage   73.54%   73.92%   +0.37%     
==========================================
  Files          68       68              
  Lines        2967     2991      +24     
  Branches      335      451     +116     
==========================================
+ Hits         2182     2211      +29     
+ Misses        723      718       -5     
  Partials       62       62              
Impacted Files Coverage Δ
spidermon/contrib/scrapy/monitors.py 98.15% <100.00%> (+0.31%) ⬆️
spidermon/results/monitor.py 78.51% <100.00%> (+2.47%) ⬆️
spidermon/contrib/scrapy/runners.py 88.76% <0.00%> (+2.24%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d58090d...dac6ebb. Read the comment docs.

Copy link
Contributor

@further-reading further-reading left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall. I have a suggestion on how to make it a little clearer from a user perspective but it is still fine as is!

Copy link
Contributor

@further-reading further-reading left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

@rennerocha
Copy link
Collaborator Author

Added the missing docs. This PR is now ready for real review and when approved, to be merged :-)

@rennerocha rennerocha marked this pull request as ready for review December 3, 2021 14:01
@rennerocha rennerocha requested a review from ogiaquino December 13, 2021 19:31
@ogiaquino
Copy link
Member

LGTM

@rennerocha rennerocha merged commit b40c322 into scrapinghub:master Dec 14, 2021
@rennerocha rennerocha deleted the base-stat-monitor branch December 14, 2021 15:44
@rennerocha rennerocha added this to the 1.16.0 milestone Dec 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants