1943581 - Extend Glean's RLB `PingType` impl to support retrieving metadata

Status: RESOLVED FIXED

(Data Platform and Tools :: Glean: SDK, enhancement, P1)

Description

[Adam Brouwers-Harries [:aabh] [he/him] ⌚GMT]

We would like to be able to record gecko profiler markers whenever a Glean::Ping is sent by FoG. Unfortunately, at present, we do not have access to any of the Ping metadata that would be needed to record a useful marker.

We would like to record a marker during Ping::submit, here. At this point we have both a reason (an Option<&str>), and &self. The latter contains a glean::private::PingType member, which in turn contains a member of an (internal to Glean) glean_core::metrics::PingType. As this member is only accessible within the internal Glean APIs and RLB, we cannot access it, nor its details. (This is good design).

However, we still would like to get some of these details, as we need them when we record a marker! This bug proposes that the ping::private::PingType impl be extended to add a full panopoly of methods allowing access to Ping metadata. This should probably include (but may not be limited to):

// Get the name of the ping
fn get_name(&'a self) -> &'a str

// Does the ping include a client ID?
fn includes_client_id(&self) -> bool

// Should we send the ping if it's empty?
fn should_send_if_empty(&self) -> bool

// Does the ping include precise timestamps? 
fn includes_precise_timestamps(&self) -> bool

// Does the ping include info sections?
fn includes_info_sections(&self) -> bool

// Whether or not the ping is enabled
fn is_enabled(&self) -> bool

// An iterator over the pings that this ping schedules
fn get_schedules_pings(&'a self) -> Iter<'a str, T>

// An iterator over the reason codes for this ping
fn get_reason_codes(&'a self) -> Iter<'a str, T>

Comment 1

[Adam Brouwers-Harries [:aabh] [he/him] ⌚GMT]

After a bit of investigation, it looks like this API might actually be a bit difficult to expose: The glean_core::metrics::PingType structure holds an Arc to the InnerPing metadata type, which actually holds the metadata that we'd like to access. Because of that, we would (almost certainly) need to explicitly clone the metadata object whenever we want to access any of the members, as we can't take a plain reference to an object held behind an Arc.

Comment 2

[Adam Brouwers-Harries [:aabh] [he/him] ⌚GMT]

Wait, disregard: we may be able to use fn as_ref(&self) -> &T...

Comment 3

[Adam Brouwers-Harries [:aabh] [he/him] ⌚GMT]

Ah! There are already methods that do what we want, but they are internal to glean itself, i.e. not even exposed to the RLB. For example: https://searchfox.org/mozilla-central/source/third_party/rust/glean-core/src/metrics/ping.rs#138

It might be possible to make these public within glean/the RLB, and then just call them within our API.

Comment 4

[BMO Github Automation]

Attachment: [mozilla/glean] Bug 1943581 - Extend Glean's RLB impl to support retrieving metadata (#3055)

Comment 5

[Travis Long [:travis_]]

This landed and will be out with the next Glean release (probably tomorrow), so closing this as FIXED