Page MenuHomeSoftware Heritage
Differential D5463

Add metric to monitor "save code now" efficiency
ClosedPublic
Actions

Authored by ardumont on Apr 8 2021, 4:33 PM.

Details

Reviewers
anlambert
Group Reviewers
Reviewers
Maniphest Tasks
T1481: add metric to monitor "save code now" efficiency
Commits
rDWAPPS7131f6cd4993: Add metric to monitor "save code now" efficiency
Summary

This only deals with the finalized save code now statuses (succeeded, failed) as it's
the only information referenced in the model.

Related to T1481

direct metrics:

which allows to have a rate tendency graph:

avg over time:

Test Plan

docker, save code now and checking metrics out of the endpoint and prometheus graph as in the screenshots ^

Diff Detail

Repository
rDWAPPS Web applications
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 20549
Build 31896: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 31895: arc lint + arc unit

Unit TestsFailed
View All

TimeTest
29 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.misc.test_metrics::Tests / Python tests / test_origin_save_metrics
client = <django.test.client.Client object at 0x7eff6cb00470> @pytest.mark.django_db
216 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.admin.test_origin_save::Tests / Python tests / test_accept_pending_save_request
1,029 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.admin.test_origin_save::Tests / Python tests / test_add_authorized_origin_url
123 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.admin.test_origin_save::Tests / Python tests / test_add_unauthorized_origin_url
126 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.admin.test_origin_save::Tests / Python tests / test_reject_pending_save_request
View Full Test Results (1 Failed · 623 Passed · 5 Skipped)

Event Timeline

ardumont created this revision.Apr 8 2021, 4:33 PM
ardumont edited the summary of this revision. (Show Details)Apr 8 2021, 4:34 PM
ardumont edited the test plan for this revision. (Show Details)
swh-public-ci added a comment.Apr 8 2021, 4:40 PM

Build has FAILED

Patch application report for D5463 (id=19512)

Rebasing onto cf69ace028...

Current branch diff-target is up to date.
Changes applied before test
commit 2436835d247243de4d482485e9ff0f85c01eeca7
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 8 16:23:10 2021 +0200

    Add metric to monitor "save code now" efficiency
    
    This only deals with the finalized save code now statuses (succeeded, failed) as it's
    the only information referenced in the model.
    
    Related to T1481

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/663/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/663/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 8 2021, 4:40 PM
Harbormaster failed remote builds in B20549: Diff 19512!
anlambert added a comment.Apr 8 2021, 4:48 PM

I do not recall exactly what is is possible to do with Prometheus queries but one interesting metric to compute would be the average save code now request delay.

For the tests, I think you can inspire from https://forge.softwareheritage.org/source/swh-web/browse/master/swh/web/tests/misc/test_metrics.py

swh/web/common/origin_save.py
626

Looking at the value computed below, this does not correspond to a save request duration but rather to the delay between the request submission and the swh visit so ACCEPTED_SAVE_REQUESTS_DELAY_METRIC feels like a better name.

628

you need to change that identifier as it is the same as the gauge above.

ardumont added a comment.Apr 8 2021, 5:52 PM

Thanks for the suggestions, will adapt.

swh/web/common/origin_save.py
626

indeed.

628

ugh right, how did i miss that?!

692

Do you think it's sensible enough? Or should i just add that check within the if conditional just above?

ardumont added inline comments.Apr 8 2021, 6:00 PM
swh/web/common/origin_save.py
628

ah i know, i renamed it from the original name with counter to gauge.
And i missed the clash.

anlambert added inline comments.Apr 8 2021, 6:01 PM
swh/web/common/origin_save.py
692

Oh right, you should not assert on visit_date as it can be None before a visit is found.

ardumont updated this revision to Diff 19533.Apr 8 2021, 6:41 PM

Fix first remarks

swh-public-ci added a comment.Apr 8 2021, 6:48 PM

Build is green

Patch application report for D5463 (id=19533)

Rebasing onto 6e8bffe487...

First, rewinding head to replay your work on top of it...
Applying: Add metric to monitor "save code now" efficiency
Changes applied before test
commit f5208cd70c40985778868d4c466c39aebff0b1eb
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 8 16:23:10 2021 +0200

    Add metric to monitor "save code now" efficiency
    
    This only deals with the finalized save code now statuses (succeeded, failed) as it's
    the only information referenced in the model.
    
    Related to T1481

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/664/ for more details.

Harbormaster completed remote builds in B20570: Diff 19533.Apr 8 2021, 6:48 PM
ardumont requested review of this revision.Apr 8 2021, 6:48 PM
ardumont planned changes to this revision.Apr 8 2021, 6:56 PM
ardumont updated this revision to Diff 19538.Apr 8 2021, 7:21 PM

Fix gauge

ardumont planned changes to this revision.Apr 8 2021, 7:25 PM
swh-public-ci added a comment.Apr 8 2021, 7:28 PM

Build is green

Patch application report for D5463 (id=19538)

Rebasing onto 6e8bffe487...

First, rewinding head to replay your work on top of it...
Applying: Add metric to monitor "save code now" efficiency
Changes applied before test
commit 20b25b2ca74b27110705c087962736ba88cfbfa8
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 8 16:23:10 2021 +0200

    Add metric to monitor "save code now" efficiency
    
    This only deals with the finalized save code now statuses (succeeded, failed) as it's
    the only information referenced in the model.
    
    Related to T1481

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/665/ for more details.

Harbormaster completed remote builds in B20574: Diff 19538.Apr 8 2021, 7:28 PM
ardumont updated this revision to Diff 19546.Apr 9 2021, 10:51 AM

Rename metric to use the delay term

ardumont planned changes to this revision.Apr 9 2021, 10:51 AM
swh-public-ci added a comment.Apr 9 2021, 10:58 AM

Build is green

Patch application report for D5463 (id=19546)

Rebasing onto 6e8bffe487...

Current branch diff-target is up to date.
Changes applied before test
commit cf2d177fe47c5c798aa30f76b248831d59f80ec7
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 8 16:23:10 2021 +0200

    Add metric to monitor "save code now" efficiency
    
    This only deals with the finalized save code now statuses (succeeded, failed) as it's
    the only information referenced in the model.
    
    Related to T1481

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/666/ for more details.

Harbormaster completed remote builds in B20584: Diff 19546.Apr 9 2021, 10:58 AM
ardumont updated this revision to Diff 19552.Apr 9 2021, 11:22 AM

Add test on new metric

ardumont updated this revision to Diff 19553.Apr 9 2021, 11:24 AM

Remove unintended changes

swh-public-ci added a comment.Apr 9 2021, 11:29 AM

Build is green

Patch application report for D5463 (id=19552)

Rebasing onto 6e8bffe487...

Current branch diff-target is up to date.
Changes applied before test
commit 5d88f646a3bff0321a82e4de18e0d59a18c9564c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 8 16:23:10 2021 +0200

    Add metric to monitor "save code now" efficiency
    
    This only deals with the finalized save code now statuses (succeeded, failed) as it's
    the only information referenced in the model.
    
    Related to T1481

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/667/ for more details.

Harbormaster completed remote builds in B20589: Diff 19552.Apr 9 2021, 11:29 AM
swh-public-ci added a comment.Apr 9 2021, 11:36 AM

Build is green

Patch application report for D5463 (id=19553)

Rebasing onto 6e8bffe487...

Current branch diff-target is up to date.
Changes applied before test
commit 83c18187b72036a36a010ee487977cafde22539c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 8 16:23:10 2021 +0200

    Add metric to monitor "save code now" efficiency
    
    This only deals with the finalized save code now statuses (succeeded, failed) as it's
    the only information referenced in the model.
    
    Related to T1481

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/668/ for more details.

Harbormaster completed remote builds in B20590: Diff 19553.Apr 9 2021, 11:36 AM
ardumont added inline comments.Apr 9 2021, 11:43 AM
swh/web/tests/misc/test_metrics.py
114 ↗(On Diff #19553)

right, i'll need to amend data to actually pass through here then...

anlambert added inline comments.Apr 9 2021, 11:50 AM
swh/web/common/origin_save.py
641

actual time of visit by Software Heritage

visits are created at the beginning of the loading so final status should not be mentioned here.

694–697

I am wondering if computing the average delay time would not be a better choice here.

If I understand correctly, you send to prometheus the cumulative delay times for all requests with a swh visit.
Sending an average delay value will enable to see on a graph the evolution of requests processing delay over
time which I think is what we want to monitor here from my point of view.

Any thoughts on this ?

ardumont added inline comments.Apr 9 2021, 11:56 AM
swh/web/common/origin_save.py
694–697

Yes, but i'll let prometheus/grafana do the computations, the rate (as screenshot above) and the avg for example.

anlambert added inline comments.Apr 9 2021, 11:58 AM
swh/web/common/origin_save.py
694–697

Ack, I guess you can get the number of requests by querying the other metrics (number of requests per visit type and statuses), right ?

ardumont edited the summary of this revision. (Show Details)Apr 9 2021, 12:27 PM
ardumont added inline comments.Apr 9 2021, 12:32 PM
swh/web/common/origin_save.py
694–697

I don't think we need the number of requests, we want a tendency which we can have using
the aggregation over time functions [1].

I've updated the diff description with screenshots to demo it as well.

But yeah, i think we can compose other metrics if need be.

[1] https://prometheus.io/docs/prometheus/latest/querying/functions/#aggregation_over_timedescription

ardumont updated this revision to Diff 19564.Apr 9 2021, 1:01 PM

Adapt data so the metric actually display non null values.

swh-public-ci added a comment.Apr 9 2021, 1:09 PM

Build is green

Patch application report for D5463 (id=19564)

Rebasing onto 6e8bffe487...

Current branch diff-target is up to date.
Changes applied before test
commit d6237c835fce4832065f5cb9e7cf87f38e51e442
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 8 16:23:10 2021 +0200

    Add metric to monitor "save code now" efficiency
    
    This only deals with the finalized save code now statuses (succeeded, failed) as it's
    the only information referenced in the model.
    
    Related to T1481

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/669/ for more details.

Harbormaster completed remote builds in B20597: Diff 19564.Apr 9 2021, 1:09 PM
anlambert accepted this revision.Apr 9 2021, 1:36 PM
anlambert added inline comments.
swh/web/common/origin_save.py
641

or simply "actual time of ingestion"

694–697

I don't think we need the number of requests, we want a tendency which we can have using
the aggregation over time functions [1].

If we want an accurate average delay time, yes we do.

But this can be computed from Prometheus so current metric looks good to me.

This revision is now accepted and ready to land.Apr 9 2021, 1:36 PM
ardumont updated this revision to Diff 19568.Apr 9 2021, 2:02 PM
  • Rebase
  • adapt comment according to suggestion (i had forgotten that one)
swh-public-ci added a comment.Apr 9 2021, 2:09 PM

Build is green

Patch application report for D5463 (id=19568)

Rebasing onto 0eb0c33a65...

Current branch diff-target is up to date.
Changes applied before test
commit 7131f6cd4993cea8802770c369b04a3f008fc779
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 8 16:23:10 2021 +0200

    Add metric to monitor "save code now" efficiency
    
    This only deals with the finalized save code now statuses (succeeded, failed) as it's
    the only information referenced in the model.
    
    Related to T1481

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/671/ for more details.

Harbormaster completed remote builds in B20601: Diff 19568.Apr 9 2021, 2:09 PM
Closed by commit rDWAPPS7131f6cd4993: Add metric to monitor "save code now" efficiency (authored by ardumont). · Explain WhyApr 9 2021, 2:18 PM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDWAPPS7131f6cd4993: Add metric to monitor "save code now" efficiency.

Revision Contents
Changeset List

PathSize
swh/
web/
common/
36 lines

Diff 19512

View Options

swh/web/common/origin_save.py

Loading...
About · Developer information · Legal