1655932 - Validate that discarded_exceeding_pings_size is working as expected

Status: NEW

(Data Platform and Tools :: Glean: SDK, task, P4)

Description

[Beatriz Rizental [:brizental]]

This new metric was just added by Bug 1597761, we should verify that it is recording as expected in a couple of weeks.

Comment 1

[Beatriz Rizental [:brizental]]

Currently I don't think there is a good way to reliably validate that this is working as expected.

I wrote a query [1] that checks the size and number of pings vs. how many pings were deleted due to size overflow, but that query is broken in two ways:

1. It is looking at error tables, because I do not have permissions to look at the ping tables;
2. It assumes pings with more than 1MB are sent, when in fact they are rejected by the pipeline with a 413 code;

Number 2 makes me question the usefulness of the whole "discard pings that are over 1MB" feature, since we kind of already had that since the beggining. A 413, as all 4xx status codes, are considered unrecoverable and will make Glean delete the ping anyways. Having that built in will only save us the trip of going through the whole upload cycle with a ping that will ultimately be discarded.

Aaanyways, I am untaking until Bug 1662470 is resolved. When it is I can tweak [1] to use the content-length of sent pings. I can then add the resulting query to our monitoring dashboard, and we can continually monitor discarded pings vs. size / number of pings. Ideally we should never see any discarded pings (and for the time being we haven't seen any, thus the difficulty in validating this), if we monitor this periodically we can then see when we do get them and take action if needed. If we never / rarely see any discarded pings we can revisit the usefulness of this feature, if the size limit should be tweaked, or even if the metric is broken, etc.

[:chutten] Do you possibly have any clever ways I could possibly validate this?

[1]: Analyze number of discarded pings due to size overflow

Comment 2

[Alessio Placitelli [:Dexter]]

(In reply to Beatriz Rizental from comment #1)

2. It assumes pings with more than 1MB are sent, when in fact they are rejected by the pipeline with a 413 code;

This is an interesting implementation detail of the pipeline of which I wasn't aware of.

Number 2 makes me question the usefulness of the whole "discard pings that are over 1MB" feature, since we kind of already had that since the beggining.

I disagree. If we know pings will get discarded, we might as well not send them at all in order to save user's bandwidth, especially on mobile.

Comment 3

[Chris H-C :chutten]

(In reply to Alessio Placitelli [:Dexter] - Back 1st of September from comment #2)

(In reply to Beatriz Rizental from comment #1)

2. It assumes pings with more than 1MB are sent, when in fact they are rejected by the pipeline with a 413 code;

This is an interesting implementation detail of the pipeline of which I wasn't aware of.

TIL as well, but it's my fault for not reading the spec.

(In reply to Beatriz Rizental from comment #1)

[:chutten] Do you possibly have any clever ways I could possibly validate this?

Hm. I'd ask someone on pipeline what, if any, reporting is done on 413 responses.... annnnnd I see :benwu's already provided something over on that other bug. Looks like we can get counts per submission date and project, but not by application id or path or User Agent or anything. If we were hitting the limit in the past, your fix would result in a sudden decrease in the counts (might be hidden in the noise), but I doubt any of our apps were hitting the limit anyway...

It might be that the best way forward is to make a Glean Health Ping that is small and internal and sends only Glean-internal health metrics from the core and language binding. Then we could add counts of "Ping too large" to this ping which will never be too large and reliably presume the absence of the error in the ping as absence of the error. (the user-facing Glean Error Stream.

But as the system sits today, I don't think there's anything we can do to verify this.

Comment 4

[Beatriz Rizental [:brizental]]

I am untaking this as it seems to be not possible to validate it at the moment.

I opened Bug 1667815 to consider creating a health ping as suggested by [:chutten] here and I also believe we could get some insight on this after Bug 1662470 is resolved.