Bug 1843109 - Add tests in alerts API for profiler links

tests/conftest.py

@@ -887,6 +887,41 @@ def _fetch_intermittent_bugs(additional_params, limit, duplicate_chain_length):
     )
 
 
+class MockResponse:
+    def __init__(self):
+        self.status_code = 200
+        self.content = {
+            "artifacts": [
+                {
+                    "storageType": "fake",
+                    "name": "fake/path.json",
+                    "expires": "2999-12-31T23:59:59.999Z",
+                    "contentType": "application/octet-stream",
+                }
+            ]
+        }
+
+
+@pytest.fixture
+def mock_get_artifact_list(monkeypatch):
+    import treeherder.webapp.api.utils
+
+    def _mock_get(url, params=None):
+        return MockResponse()
+
+    monkeypatch.setattr(treeherder.webapp.api.utils, 'fetch_json', _mock_get)
+
+
+@pytest.fixture
+def mock_cache(monkeypatch):
+    import django.core.cache.cache
+
+    def mockreturn_cache(*args, **kwargs):
+        return {"task_id": "some_id", "retry_id": 0}
+
+    monkeypatch.setattr(django.core.cache.cache, 'get', mockreturn_cache)
+
+
 @pytest.fixture
 def text_log_error_lines(test_job, failure_lines):
     lines = [

tests/webapp/api/test_performance_alerts_api.py

@@ -37,6 +37,8 @@ def test_alerts_get(
         'series_signature',
         'taskcluster_metadata',
         'prev_taskcluster_metadata',
+        'profile_url',
+        'prev_profile_url',
         'summary_id',
         'status',
         't_value',
@@ -60,8 +62,12 @@ def test_alerts_put(
     client,
     push_stored,
     test_repository,
+    test_perf_datum,
+    test_perf_datum_2,
     test_perf_alert,
     test_perf_alert_summary_2,
+    test_taskcluster_metadata,
+    test_taskcluster_metadata_2,
     test_user,
     test_sheriff,
 ):
@@ -110,8 +116,12 @@ def test_reassign_different_repository(
     push_stored,
     test_repository,
     test_repository_2,
+    test_perf_datum,
+    test_perf_datum_2,
     test_perf_alert,
     test_perf_alert_summary_2,
+    test_taskcluster_metadata,
+    test_taskcluster_metadata_2,
     test_sheriff,
 ):
     # verify that we can't reassign to another performance alert summary
@@ -535,7 +545,15 @@ def test_timestamps_on_manual_created_alert_via_their_endpoints(
     assert manual_alert.created <= manual_alert.first_triaged
 
 
-def test_alert_timestamps_via_endpoint(authorized_sheriff_client, test_sheriff, test_perf_alert):
+def test_alert_timestamps_via_endpoint(
+    authorized_sheriff_client,
+    test_sheriff,
+    test_perf_datum,
+    test_perf_datum_2,
+    test_perf_alert,
+    test_taskcluster_metadata,
+    test_taskcluster_metadata_2,
+):
     # updating autogenerated alert:
     # created doesn't change, last_updated & first_triaged update
     old_created = test_perf_alert.created
@@ -572,10 +590,14 @@ def test_alert_timestamps_via_endpoint(authorized_sheriff_client, test_sheriff,
 def test_related_alerts_timestamps_via_endpoint(
     authorized_sheriff_client,
     test_sheriff,
+    test_perf_datum,
+    test_perf_datum_2,
     test_perf_alert,
     relation,
     test_perf_alert_summary,
     test_perf_alert_summary_2,
+    test_taskcluster_metadata,
+    test_taskcluster_metadata_2,
 ):
     # downstream/reassign use case
     assert test_perf_alert.first_triaged is None

tests/webapp/api/test_performance_alertsummary_api.py

@@ -99,6 +99,8 @@ def test_alert_summaries_get(
         'series_signature',
         'taskcluster_metadata',
         'prev_taskcluster_metadata',
+        'profile_url',
+        'prev_profile_url',
         'is_regression',
         'starred',
         'manually_created',
@@ -174,6 +176,8 @@ def test_alert_summaries_get_onhold(
         'series_signature',
         'taskcluster_metadata',
         'prev_taskcluster_metadata',
+        'profile_url',
+        'prev_profile_url',
         'is_regression',
         'starred',
         'manually_created',

treeherder/webapp/api/performance_serializers.py

@@ -1,7 +1,7 @@
 import decimal
 
 from django.contrib.auth.models import User
-from django.core.cache import cache, caches
+from django.core.cache import cache
 from django.core.exceptions import ObjectDoesNotExist
 from django.db import transaction
 from rest_framework import exceptions, serializers
@@ -18,7 +18,7 @@
     PerformanceSignature,
     PerformanceTag,
 )
-from treeherder.webapp.api.utils import to_timestamp, get_profile_artifact_url
+from treeherder.webapp.api.utils import to_timestamp, get_profile_artifact_url, FIVE_DAYS
 
 
 def get_tc_metadata(alert, push):
@@ -27,12 +27,14 @@ def get_tc_metadata(alert, push):
         repository=alert.series_signature.repository,
         push=push,
     ).first()
-    metadata = TaskclusterMetadata.objects.get(job=datum.job)
-    task_metadata = {
-        'task_id': metadata.task_id,
-        'retry_id': metadata.retry_id,
-    }
-    return task_metadata
+    if datum:
+        metadata = TaskclusterMetadata.objects.get(job=datum.job)
+        return {
+            'task_id': metadata.task_id,
+            'retry_id': metadata.retry_id,
+        }
+    else:
+        return {}
 
 
 class OptionalBooleanField(serializers.BooleanField):
@@ -204,29 +206,37 @@ def update(self, instance, validated_data):
 
     def get_taskcluster_metadata(self, alert):
         try:
-            task_metadata = get_tc_metadata(alert, alert.summary.push)
-            cache.set("task_metadata", task_metadata)
-            return task_metadata
+            taskcluster_metadata = get_tc_metadata(alert, alert.summary.push)
+            cache.set("task_metadata", taskcluster_metadata, FIVE_DAYS)
+            return taskcluster_metadata
         except ObjectDoesNotExist:
             return {}
 
     def get_prev_taskcluster_metadata(self, alert):
         try:
-            task_metadata = get_tc_metadata(alert, alert.summary.prev_push)
-            cache.set("prev_task_metadata", task_metadata)
-            return task_metadata
+            taskcluster_metadata = get_tc_metadata(alert, alert.summary.prev_push)
+            cache.set("prev_task_metadata", taskcluster_metadata, FIVE_DAYS)
+            return taskcluster_metadata
         except ObjectDoesNotExist:
             return {}
 
     def get_profile_url(self, alert):
-        task_metadata = cache.get("task_metadata")
-        url = get_profile_artifact_url(alert, task_metadata)
+        if alert.is_regression:
+            taskcluster_metadata = cache.get("task_metadata") if cache.get("task_metadata") else {}
+            url = get_profile_artifact_url(alert, taskcluster_metadata)
+        else:
+            url = "N/A"
 
         return url
 
     def get_prev_profile_url(self, alert):
-        prev_task_metadata = cache.get("prev_task_metadata")
-        url = get_profile_artifact_url(alert, prev_task_metadata)
+        if alert.is_regression:
+            prev_taskcluster_metadata = (
+                cache.get("prev_task_metadata") if cache.get("prev_task_metadata") else {}
+            )
+            url = get_profile_artifact_url(alert, prev_taskcluster_metadata)
+        else:
+            url = "N/A"
 
         return url
 

treeherder/webapp/api/utils.py

@@ -1,13 +1,13 @@
-import requests
-import json
-import taskcluster_urls as liburls
-
 import time
 from datetime import datetime, timedelta
 
 import django_filters
+import taskcluster_urls
+from django.core.cache import cache
 from django.db.models import Aggregate, CharField
 
+from treeherder.utils.http import fetch_json
+
 # queries are faster when filtering a range by id rather than name
 # trunk: mozilla-central, autoland
 # firefox-releases: mozilla-beta, mozilla-release
@@ -18,6 +18,8 @@
     'comm-releases': [38, 135],
 }
 
+FIVE_DAYS = 432000
+
 
 class GroupConcat(Aggregate):
     function = 'GROUP_CONCAT'
@@ -55,19 +57,39 @@ def get_end_of_day(date):
     return date + timedelta(days=1, microseconds=-1)
 
 
+def get_artifact_list(root_url, task_id):
+    artifacts_url = taskcluster_urls.api(root_url, 'queue', 'v1', f"task/{task_id}/artifacts")
+    artifacts = {"artifacts": []}
+    try:
+        artifacts = fetch_json(artifacts_url)
+    except Exception as e:
+        print(e)
+    finally:
+        return artifacts.get("artifacts", [])
+
+
 def get_profile_artifact_url(alert, task_metadata):
     tc_root_url = alert.summary.repository.tc_root_url
-    index_url = liburls.api(
-        tc_root_url,
-        'queue',
-        'v1',
-        f"task/{task_metadata['task_id']}/runs/{task_metadata['retry_id']}/artifacts"
-    )
-    response = requests.get(index_url)
-    artifacts = json.loads(response.content)
+    # Return a string to tell that task_id wasn't found
+    if not task_metadata.get('task_id'):
+        return "task_id not found"
+    # If the url was already cached, don't calculate again, just return it
+    if cache.get(task_metadata.get('task_id')):
+        return cache.get(task_metadata.get('task_id'))
+    artifacts_json = get_artifact_list(tc_root_url, task_metadata.get('task_id'))
     profile_artifact = [
-        artifact for artifact in artifacts["artifacts"]
-        if artifact["name"].startswith("public/test_info/profile_")
-           and artifact["name"].endswith(".zip")
+        artifact
+        for artifact in artifacts_json
+        if artifacts_json
+        and artifact.get("name", "").startswith("public/test_info/profile_")
+        and artifact.get("name", "").endswith(".zip")
     ]
-    return f"{index_url}/{profile_artifact[0]['name']}"
+    if not profile_artifact:
+        return "Artifact not found"
+    task_url = f"{tc_root_url}/api/queue/v1/task/{task_metadata['task_id']}"
+    artifact_url = (
+        f"{task_url}/runs/{str(task_metadata['retry_id'])}/artifacts/{profile_artifact[0]['name']}"
+    )
+    cache.set(task_metadata.get('task_id'), artifact_url + " from cache", FIVE_DAYS)
+
+    return artifact_url

ui/perfherder/alerts/AlertTableRow.jsx

@@ -403,6 +403,10 @@ export default class AlertTableRow extends React.Component {
       !alert.series_signature.tags.includes('interactive') &&
       !browsertimeBenchmarksTests.includes(alert.series_signature.suite) &&
       sxsTriggered;
+    const showProfilerLinks =
+      alert.profile_url.endsWith('.zip') &&
+      alert.prev_profile_url.profile_url.endsWith('.zip');
+    const showTools = showSideBySideLink || showProfilerLinks;
 
     const backfillStatusInfo = this.getBackfillStatusInfo(alert);
     let sherlockTooltip = backfillStatusInfo && backfillStatusInfo.message;
@@ -506,9 +510,10 @@ export default class AlertTableRow extends React.Component {
             />
           </div>
         </td>
-        {alertSummary.framework === browsertimeId && (
-          <td className="table-width-md">
-            {showSideBySideLink ? (
+        <td className="table-width-md">
+          {showTools &&
+            alertSummary.framework === browsertimeId &&
+            showSideBySideLink && (
               <span className="text-darker-info">
                 <a
                   href={this.buildSideBySideLink()}
@@ -524,13 +529,38 @@ export default class AlertTableRow extends React.Component {
                   />
                 </a>
               </span>
-            ) : (
-              <Badge className="mb-1" color="light">
-                None
-              </Badge>
             )}
-          </td>
-        )}
+          {showTools && showProfilerLinks && (
+            <span className="text-darker-info">
+              `P(`
+              <a
+                href={alert.prev_profile_url}
+                target="_blank"
+                rel="noopener noreferrer"
+                className="text-dark button btn border p-0 border-0 bg-transparent"
+                aria-label="side-by-side"
+              >
+                before
+              </a>
+              `/`
+              <a
+                href={alert.profile_url}
+                target="_blank"
+                rel="noopener noreferrer"
+                className="text-dark button btn border p-0 border-0 bg-transparent"
+                aria-label="side-by-side"
+              >
+                after
+              </a>
+              `)`
+            </span>
+          )}
+          {!showTools && (
+            <Badge className="mb-1" color="light">
+              None
+            </Badge>
+          )}
+        </td>
         <td className="table-width-lg">
           <div className="information-container">
             <div className="option">