Closed Bug 1580196 Opened 2 months ago Closed 2 months ago

GV Streaming Telemetry doesn't properly handle expiry

Categories

(Toolkit :: Telemetry, defect, P1)

defect
Points:
1

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox71 --- fixed

People

(Reporter: chutten, Assigned: chutten)

References

Details

Attachments

(1 file)

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)

Assignee: nobody → chutten
Status: NEW → ASSIGNED
Points: --- → 1
Priority: -- → P1
Pushed by chutten@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ca69b14d6dbf
Test that expired histograms don't stream to GV r=janerik
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
You need to log in before you can comment on or make changes to this bug.