Closed Bug 1526343 Opened 6 years ago Closed 6 years ago

Make Glean.sendPing public

Categories

(Toolkit :: Telemetry, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED

People

(Reporter: mdroettboom, Assigned: mdroettboom)

References

Details

(Whiteboard: [telemetry:mobilesdk:m8])

Attachments

(1 file)

There's currently no way to send a custom ping, and Glean.sendPing could certainly be it. Though perhaps we want the public API to only be able to send custom pings, not built-in ones.

Priority: -- → P3
Whiteboard: [telemetry:mobilesdk:m?] → [telemetry:mobilesdk:backlog]

Recently, Glean.sendPing became Glean.sendPings (to support parallel sending), but the issue fundamentally still stands.

Whiteboard: [telemetry:mobilesdk:backlog] → [telemetry:mobilesdk:m8]
Assignee: nobody → mdroettboom

I feel like we can basically get away with making the existing sendPings public, however, I think for safety we shouldn't allow the public API to send pings that are internally managed and scheduled by glean (metrics, events, baseline). It seems like it would be too easy to screw up the carefully-orchestrated scheduling on those. How do others feel about that?

Flags: needinfo?(tlong)
Flags: needinfo?(gfritzsche)
Flags: needinfo?(alessio.placitelli)

I think that's fine, I was also a little concerned about limiting the public access to the internal pings. As long as you are limiting the public API to only sending custom pings, then I am okay with that approach.

Flags: needinfo?(tlong)

(In reply to Michael Droettboom [:mdroettboom] from comment #2)

I feel like we can basically get away with making the existing sendPings public, however, I think for safety we shouldn't allow the public API to send pings that are internally managed and scheduled by glean (metrics, events, baseline). It seems like it would be too easy to screw up the carefully-orchestrated scheduling on those. How do others feel about that?

+1 from me to just disallowing the pings owned by Glean for it.

Flags: needinfo?(gfritzsche)

(In reply to Michael Droettboom [:mdroettboom] from comment #2)

I feel like we can basically get away with making the existing sendPings public, however, I think for safety we shouldn't allow the public API to send pings that are internally managed and scheduled by glean (metrics, events, baseline). It seems like it would be too easy to screw up the carefully-orchestrated scheduling on those. How do others feel about that?

Yes. The only caveat there is that we want GleanDebugActivity to still be able to trigger pings we own (e.g. to test events or recorded metrics). One easy way to make this work would be to have 2 APIs:

  • an internal internalSendPings (or something like that), which would be unfiltered. This would be callable from the GleanDebugActivity, which has access to internals;
  • the public sendPings, which would apply ping filtering and then call internalSendPings.
Flags: needinfo?(alessio.placitelli)
Attached file GitHub Pull Request (deleted) —
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: