1601553 - Consider providing public interfaces for private glean metrics types

Status: RESOLVED WORKSFORME

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

Description

[Ryan Kelly [:rfkelly]]

As Glean consumers, the app-services team have sometimes found it useful to write code that's generic over a set of concrete MetricsType instances. One simple example is here where we want to wrap some re-useable code around a pair of counters, and use the same code for several different pairs of counters. Another example is in the support-sync-telemetry package where we have almost identical processHistoryPing and processBookmarksPing methods, which could profitably share much of their implementation.

The glean MetricsType classes are "hidden" in the mozilla.components.service.glean.private namespace, which IIUC is to discourage consumers from trying to create their own instances that are not declared in the metrics.yaml file. For the generic code linked above, I ended up importing them from the private namespace so that I could use them in type declarations, and adding a comment next the import saying that we promise not to create our own instances. This works, but feels kind of iffy, and made :Dexter worried that we might be doing the wrong thing when he first looked at the code :-)

Would you consider exposing a set of public interfaces that consuming code can safely import but not instantiate, one for each of the private metrics types? This would let us write generic code like the above without having to import anything from glean's private namespace.

Comment 1

[Alessio Placitelli [:Dexter]]

(In reply to Ryan Kelly [:rfkelly] from comment #0)

Would you consider exposing a set of public interfaces that consuming code can safely import but not instantiate, one for each of the private metrics types? This would let us write generic code like the above without having to import anything from glean's private namespace.

I think this is a good idea and sounds like a less scary alternative to using stuff in a private package :)
Since you already have working code, I'm moving this to our 'backlog', as we have a few higher priority items to take care of.

Thank you for reporting!

Comment 2

[Michael Droettboom [:mdroettboom]]

Bug 1647730 is related to this.

We are looking into how we can prioritize this work now. At least part of this is a requirement for Nimbus so it has a proper Rust API to call. For a-s which will remain on the other side of an FFI, it will be more complicated, but I think that's a stepping stone.

Comment 3

[Alessio Placitelli [:Dexter]]

We now have this in Rust! They are public traits and they are currently being used in FOG.