1580196 - GV Streaming Telemetry doesn't properly handle expiry

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P1)

Description

[Chris H-C :chutten]

Due to Histogram's interesting calling conventions in JS, we always have a backing Histogram instance for any given Accumulate() call. If the histogram is expired, that instance is gExpiredHistogram (or gExpiredKeyedHistogram, depending if you need a KeyedHistogram).

This means internal_HistogramAdd is called whether the histogram is expired or not. If it's expired, the samples get dumped on gExpiredHistogram and go nowhere.

Unless we're in GV Streaming mode. In that case, just before we dump the sample on gExpiredHistogram we redirect it to GeckoViewStreamingTelemetry.

Instead we should ensure that the histogram we intend to stream isn't expired.

This might just be a check for if (histogram == gExpiredHistogram), but we need to double-check that expired keyed histograms also send gExpiredHistogram to internal_HistogramAdd. (For now, since GV Streaming doesn't support keyed metrics, we could ensure that the histogram isn't keyed and isn't gExpiredHistogram and that'd cover it)

Comment 1

[Chris H-C :chutten]

Attachment: Bug 1580196 - Test that expired histograms don't stream to GV r?janerik

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ca69b14d6dbf
Test that expired histograms don't stream to GV r=janerik

Comment 3

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/ca69b14d6dbf