Closed akkomar closed 5 years ago
This is caused by lack of client_id
in activation
and history_sync
pings. client_id
, together with debug header is used to identify clients and group pings. If a ping arrives without it, it is assumed to be invalid and ignored (see https://github.com/mozilla/debug-ping-view/blob/master/functions/index.js#L18-L21).
Instead of silently dropping such pings, we can:
client_id
that used the same debug tag, assume this is the submitting client and proceedUNKNOWN
placeholder and proceedThis solution has the benefit of being least surprising to users - unless "clientless" ping is submitted as the first one, they will see the ping in their view.
It's worth noting that activation
ping doesn't carry client_id
for a reason - we don't want to be able to correlate activation_id
with client_id
. By exposing these pings in DebugView we're violating this rule for selected group of internal testers/developers in a closed system. I don't think this is an issue, @Dexterp37 - what do you think?
Also, going forward we might consider using just debug id for grouping clients/pings, without the client_id
. This should depend on usage patterns.
This is caused by lack of
client_id
inactivation
andhistory_sync
pings.client_id
, together with debug header is used to identify clients and group pings. If a ping arrives without it, it is assumed to be invalid and ignored (see https://github.com/mozilla/debug-ping-view/blob/master/functions/index.js#L18-L21).
I see, thanks for nailing this down.
Also, going forward we might consider using just debug id for grouping clients/pings, without the
client_id
. This should depend on usage patterns.
I'm wondering if we can't do this right away. Do we really need the client_id
grouping? I know we settled on that with our prototype plan/design doc, but maybe it's ok to just use the debug tag for the grouping purpose.
That would allow us to just set it as UNKNOWN
or Not reported
if.. not reported.
@georgf , thoughts?
Using just the debug tag for grouping makes sense with the context here. If any overlaps occur, users could adopt and just use different debug tags.
Whether to do that right away - akomar knows best what's feasible short-term.
I'd go with the solution proposed in https://github.com/mozilla/debug-ping-view/issues/16#issuecomment-493951027 and revisit this later. I spoke to @bsurd and it looks like the current setup is convenient for QA.
I'd go with the solution proposed in #16 (comment) and revisit this later. I spoke to @bsurd and it looks like the current setup is convenient for QA.
I think we should change this now, if the fix itself is trivial, instead of having people rely on a behaviour that is going to change (on some edge cases).
I'm not sure things will change for @bsurd : they were not able to see pings with no client id, whatever behaviour we introduce now will be new to them. Is #16 (comment) really more convenient to them?
Sorry, I wasn't clear.
"Current setup being convenient for QA" was a reference to having clients grouped by debug_id and client_id on the first screen. Grouping just by the debug id seems to be less convenient for them.
By "revisiting this later" I mean when some more users have used the tool and we can ask them about their usage patterns.
Both options are pretty simple, I'm fine with changing this later according to QA's needs.
@Dexterp37, @linacambridge fix has been deployed to production instance - from now on all pings should be processed and visible.
Yep, it works perfectly now! ✨ Thanks @akkomar!
Mentioned first wrt.
history_sync
: https://mozilla.slack.com/archives/CEE12R4E8/p1558134122001500activation
is also affected, was reproduced by @Dexterp37 and @akkomar.