Bug 1910864/1887138 - Fix how we handle profiler urls on the perfherder backend (#8156)

* Bug 1910864 - Return None for profiler urls instead of "N/A"

Perfherder frontend has a check if the profiler urls are null, but it
doesn't check if it's "N/A" or not. That's why this was causing us to
see all the invalid links in the perfherder when user clicks on "Copy
Summary" or file a bug directly through perfherder. By changing this to
`None`, this API endpoint will return `null` to the frontend, which can
successfully understand and omit.

* Bug 1887138 - Refactor how we compute the profile urls on the backend

There were two issues with how we compute the profile urls in the
backend:

1. When we encounter an error, we were returning an error string. This
   is not good because frontend doesn't know how to handle the error
   strigs and it thinks that they are simply urls. Then it was trying to
   append the error strings on the back of profiler urls, which was
   causing invalid urls in the perf alerts.
2. We had a caching mechanism for the taskcluster metadata. But this
   caching mechanism wasn't really caching things by adding task id to
   the cache key. This was resulting a single cache for the whole "push"
   or multiple pushes. We were caching the latest metadata, and use that
   last metadata for all the alerts, which is wrong because all
   individual alerts might have differnet metadata. The good thing is,
   the taskcluseter metadata was already available to us inside the
   `alert` variable since we computed it prior this function. So we
   didn't have to do anything special, just read that from the
   dictionary.

There is one remaining piece I would like to fix still, which is how we
cache `tc_root_url`. But that's actually okay for now because a single
push should only have one `tc_root_url`. And I'm not so sure how to
pass that information to the other function where we genereate the
profile urls.

treeherder/webapp/api/performance_data.py

@@ -5,7 +5,6 @@ from urllib.parse import urlencode
 
 import django_filters
 from django.conf import settings
-from django.core.cache import cache
 from django.db import transaction
 from django.db.models import CharField, Count, Q, Subquery, Value, Case, When
 from django.db.models.functions import Concat
@@ -483,19 +482,11 @@ class PerformanceAlertSummaryViewSet(viewsets.ModelViewSet):
                     if summary["id"] == int(pk):
                         for alert in summary["alerts"]:
                             if alert["is_regression"]:
-                                taskcluster_metadata = (
-                                    cache.get("task_metadata") if cache.get("task_metadata") else {}
-                                )
                                 alert["profile_url"] = get_profile_artifact_url(
-                                    alert, taskcluster_metadata
-                                )
-                                prev_taskcluster_metadata = (
-                                    cache.get("prev_task_metadata")
-                                    if cache.get("prev_task_metadata")
-                                    else {}
+                                    alert, metadata_key="taskcluster_metadata"
                                 )
                                 alert["prev_profile_url"] = get_profile_artifact_url(
-                                    alert, prev_taskcluster_metadata
+                                    alert, metadata_key="prev_taskcluster_metadata"
                                 )
             return self.get_paginated_response(serializer.data)
 

treeherder/webapp/api/performance_serializers.py

@@ -208,7 +208,6 @@ class PerformanceAlertSerializer(serializers.ModelSerializer):
         try:
             taskcluster_metadata = get_tc_metadata(alert, alert.summary.push)
             cache.set("tc_root_url", alert.summary.repository.tc_root_url, FIVE_DAYS)
-            cache.set("task_metadata", taskcluster_metadata, FIVE_DAYS)
             return taskcluster_metadata
         except ObjectDoesNotExist:
             return {}
@@ -222,10 +221,10 @@ class PerformanceAlertSerializer(serializers.ModelSerializer):
             return {}
 
     def get_profile_url(self, alert):
-        return "N/A"
+        return None
 
     def get_prev_profile_url(self, alert):
-        return "N/A"
+        return None
 
     def get_classifier_email(self, performance_alert):
         return getattr(performance_alert.classifier, "email", None)

treeherder/webapp/api/utils.py

@@ -68,14 +68,19 @@ def get_artifact_list(root_url, task_id):
         return artifacts.get("artifacts", [])
 
 
-def get_profile_artifact_url(alert, task_metadata):
+def get_profile_artifact_url(alert, metadata_key):
     tc_root_url = cache.get("tc_root_url", "")
-    # Return a string to tell that task_id wasn't found
+    # Get the taskcluster metadata we'll use. It's determined by the caller.
+    task_metadata = alert[metadata_key]
+
+    # Return None if task_id wasn't found
     if not task_metadata.get("task_id") or not tc_root_url:
-        return "task_id not found"
+        return None
+
     # 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
@@ -86,7 +91,8 @@ def get_profile_artifact_url(alert, task_metadata):
     ]
 
     if not profile_artifact:
-        return "Artifact not available"
+        return None
+
     task_url = f"{tc_root_url}/api/queue/v1/task/{task_metadata['task_id']}"
     # There's only one profile relevant for performance per task
     artifact_url = (