From 821d19fb7a06483f30c923b8a87c70c41d5c0e5f Mon Sep 17 00:00:00 2001
From: Adam Brouwers-Harries <abrouwersharries@mozilla.com>
Date: Wed, 20 Nov 2024 10:59:18 +0000
Subject: [PATCH] Bug 1919354 - Add profiler markers for Glean::UrlMetric
 r=florian,chutten

Differential Revision: https://phabricator.services.mozilla.com/D226182
---
 .../components/glean/api/src/private/url.rs   | 81 +++++++++++++++++--
 1 file changed, 74 insertions(+), 7 deletions(-)

diff --git a/toolkit/components/glean/api/src/private/url.rs b/toolkit/components/glean/api/src/private/url.rs
index 09f21dad5e5a..1b7f699df3fd 100644
--- a/toolkit/components/glean/api/src/private/url.rs
+++ b/toolkit/components/glean/api/src/private/url.rs
@@ -8,6 +8,51 @@ use super::{CommonMetricData, MetricId};
 
 use crate::ipc::need_ipc;
 
+#[cfg(feature = "with_gecko")]
+use super::profiler_utils::{
+    lookup_canonical_metric_name, truncate_string_for_marker, LookupError,
+};
+
+#[cfg(feature = "with_gecko")]
+use gecko_profiler::gecko_profiler_category;
+
+#[cfg(feature = "with_gecko")]
+#[derive(serde::Serialize, serde::Deserialize, Debug)]
+struct UrlMetricMarker {
+    id: MetricId,
+    val: String,
+}
+
+#[cfg(feature = "with_gecko")]
+impl gecko_profiler::ProfilerMarker for UrlMetricMarker {
+    fn marker_type_name() -> &'static str {
+        "UrlMetric"
+    }
+
+    fn marker_type_display() -> gecko_profiler::MarkerSchema {
+        use gecko_profiler::schema::*;
+        let mut schema = MarkerSchema::new(&[Location::MarkerChart, Location::MarkerTable]);
+        schema.set_tooltip_label("{marker.data.id} {marker.data.val}");
+        schema.set_table_label("{marker.name} - {marker.data.id}: {marker.data.val}");
+        schema.add_key_label_format_searchable(
+            "id",
+            "Metric",
+            Format::String,
+            Searchable::Searchable,
+        );
+        schema.add_key_label_format_searchable("val", "Value", Format::Url, Searchable::Searchable);
+        schema
+    }
+
+    fn stream_json_marker_data(&self, json_writer: &mut gecko_profiler::JSONWriter) {
+        json_writer.string_property(
+            "id",
+            lookup_canonical_metric_name(&self.id).unwrap_or_else(LookupError::as_str),
+        );
+        json_writer.string_property("val", self.val.as_str());
+    }
+}
+
 /// Developer-facing API for recording URL metrics.
 ///
 /// Instances of this class type are automatically generated by the parsers
@@ -15,7 +60,10 @@ use crate::ipc::need_ipc;
 /// registered in the metrics.yaml file.
 #[derive(Clone)]
 pub enum UrlMetric {
-    Parent(glean::private::UrlMetric),
+    Parent {
+        id: MetricId,
+        inner: glean::private::UrlMetric,
+    },
     Child(UrlMetricIpc),
 }
 #[derive(Clone, Debug)]
@@ -23,18 +71,21 @@ pub struct UrlMetricIpc;
 
 impl UrlMetric {
     /// Create a new Url metric.
-    pub fn new(_id: MetricId, meta: CommonMetricData) -> Self {
+    pub fn new(id: MetricId, meta: CommonMetricData) -> Self {
         if need_ipc() {
             UrlMetric::Child(UrlMetricIpc)
         } else {
-            UrlMetric::Parent(glean::private::UrlMetric::new(meta))
+            UrlMetric::Parent {
+                id,
+                inner: glean::private::UrlMetric::new(meta),
+            }
         }
     }
 
     #[cfg(test)]
     pub(crate) fn child_metric(&self) -> Self {
         match self {
-            UrlMetric::Parent(_) => UrlMetric::Child(UrlMetricIpc),
+            UrlMetric::Parent { .. } => UrlMetric::Child(UrlMetricIpc),
             UrlMetric::Child(_) => panic!("Can't get a child metric from a child metric"),
         }
     }
@@ -44,7 +95,23 @@ impl UrlMetric {
 impl glean::traits::Url for UrlMetric {
     pub fn set<S: Into<std::string::String>>(&self, value: S) {
         match self {
-            UrlMetric::Parent(p) => p.set(value),
+            #[allow(unused)]
+            UrlMetric::Parent { id, inner } => {
+                let value: String = value.into();
+                #[cfg(feature = "with_gecko")]
+                if gecko_profiler::can_accept_markers() {
+                    gecko_profiler::add_marker(
+                        "Url::set",
+                        gecko_profiler_category!(Telemetry),
+                        Default::default(),
+                        UrlMetricMarker {
+                            id: *id,
+                            val: truncate_string_for_marker(value.clone()),
+                        },
+                    );
+                }
+                inner.set(value)
+            }
             UrlMetric::Child(_) => {
                 log::error!(
                     "Unable to set Url metric in non-main process. This operation will be ignored."
@@ -63,7 +130,7 @@ impl glean::traits::Url for UrlMetric {
     ) -> Option<std::string::String> {
         let ping_name = ping_name.into().map(|s| s.to_string());
         match self {
-            UrlMetric::Parent(p) => p.test_get_value(ping_name),
+            UrlMetric::Parent { inner, .. } => inner.test_get_value(ping_name),
             UrlMetric::Child(_) => {
                 panic!("Cannot get test value for Url metric in non-main process!")
             }
@@ -72,7 +139,7 @@ impl glean::traits::Url for UrlMetric {
 
     pub fn test_get_num_recorded_errors(&self, error: glean::ErrorType) -> i32 {
         match self {
-            UrlMetric::Parent(p) => p.test_get_num_recorded_errors(error),
+            UrlMetric::Parent { inner, .. } => inner.test_get_num_recorded_errors(error),
             UrlMetric::Child(_) => panic!(
                 "Cannot get the number of recorded errors for Url metric in non-main process!"
             ),