Page MenuHomePhabricator

Log save_success_timing in DiscussionTools
Closed, ResolvedPublic

Description

While investigating what could be causing the Reply Tool to be taking an unexpectedly long time to save comments in T290514, @matmarex discovered that DiscussionTools is not currently recording save_success_timing data [i]. This task involves the work with implementing the instrumentation necessary to log this data.

Requirements

  • The instrumentation necessary for us to know how long after people attempt to save an edit using DiscussionTools (Reply Tool or New Discussion Tool) said edit is either saved successfully (saveReady) or saved unsuccessfully (saveFailure) [ii]

Open questions

  • Where – if at all – are we documenting information about the data that is being logged in DiscussionTools?

Done

  • Editing Engineering: instrumentation is implemented that meets the requirements above
  • @MNeisler: QA (client- and/or DB-side) verifies data is being logged in ways that fulfills the requirements
  • @MNeisler: determine what – if any – EditAttemptStep events are NOT being logged within discussiontools.
  • If necessary, documentation is updated to reflect this instrumentation change
    • Note: this will only be necessary if there are EditAttemptStep events that are NOT being logged in discussiontools. More context in T290931#7363348.

i. I assume the save_success_timing is logged in EditAttemptStep
ii. I too am assuming these are the same events from EditAttemptStep

Event Timeline

Change 720827 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/DiscussionTools@master] Record timing for 'saveSuccess' and 'saveFailure' events

https://gerrit.wikimedia.org/r/720827

ppelberg updated the task description. (Show Details)

@DLynch + @MNeisler: do you know where on-wiki – if at all – we are documenting the #discussiontools-related instrumentation [i]?


I ask the above in an attempt to know: "What documentation needs to get updated when a change like the one being made in this task gets implemented?"


i. I'm imagining something like: https://www.mediawiki.org/wiki/VisualEditor/FeatureUse_data_dictionary

@ppelberg I don't know of a specific place. Thus far everything we're logging is the same as for VisualEditor -- it's the same two editing-related schemas. (This will change as the new instrumentation is about to add a commenting-specific schema.)

Change 720827 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] Record timing for 'saveSuccess' and 'saveFailure' events

https://gerrit.wikimedia.org/r/720827

Incidentally, we should still be able to (mostly) compare this new data after this merges to historical data -- the timing added to saveSuccess/saveFailure should be about equal to the time between the saveIntent and saveSuccess/saveFailure events in a given session. (With any difference being very minor and down to transmission time, I'd think.) This would be a more complicated query for Megan to run, but...

@ppelberg I don't know of a specific place. Thus far everything we're logging is the same as for VisualEditor -- it's the same two editing-related schemas. (This will change as the new instrumentation is about to add a commenting-specific schema.)

This context is helpful; thank you, @DLynch. @MNeisler and I will discuss this open question about documentation [i] when we meet tomorrow and comment here after doing so.


i. "Where – if at all – should we be documenting the logging/instrumentation present within DiscussionTools?" <--- See T286076's ===Done section.

@ppelberg I don't know of a specific place. Thus far everything we're logging is the same as for VisualEditor -- it's the same two editing-related schemas. (This will change as the new instrumentation is about to add a commenting-specific schema.)

This context is helpful; thank you, @DLynch. @MNeisler and I will discuss this open question about documentation [i] when we meet tomorrow and comment here after doing so.

Per the conversation @MNeisler and I had on 15-September: at this time we do not expect this ticket to warrant any changes to existing documentation or the creation of new documentation.

Reason being, we expect the discussiontools interfaces to be logging all of the events included in Schema:EditAttemptStep.

In line with this expectation, as part of the QA, @MNeisler is going to determine what – if any – EditAttemptStep events are NOT being logged within discussiontools.

I've updated the task description's`=== Done` section to reflect the above.

@MNeisler: QA (client- and/or DB-side) verifies data is being logged in ways that fulfills the requirements

I confirmed that save_success_timing is currently being logged for all saveSuccess DiscussionTools events following deployment of the fix on 2021-09-17.

@MNeisler: determine what – if any – EditAttemptStep events are NOT being logged within discussiontools.

I ran a check of all other EditAttemptStep events being logged for discussiontool related events (event.integration = 'discussiontools' ) to identify if there are other fields where we are not recording data.

We are logging all events as expected with the exception of event.init_timing, which does not appear to be logging data for any event.integration = 'discussiontools' or event.integration = 'page' init events. This field currently shows NULL for all events since at least the beginning of September.

@matmarex @DLynch - Any ideas as to why we are not currently recording any data for the event.init_timing field in EditAttemptStep? Can this be fixed?

init_timing isn't particularly meaningful -- all the other timings are defined as "time since some other event", but init is the first event. We also try to log the init event basically-immediately after the click that triggers the session, so there's not much to record. (In DT it'd be bimodal -- either it'd be instant, or it'd take a few hundred milliseconds as we animate the closing of a different open reply widget.)

@DLynch - Thanks! That makes sense. I agree that the init_timing field is not that meaningful or useful so I don't have any concerns about it not being logged but it would be useful to update the schema documentation to indicate that this field is not logged (pending a potential longer-term fix/removal of this field as part of EditAttemptStep cleanup - I saw this documented as one of the issues in the long-standing T118063).

Change 725018 had a related patch set uploaded (by DLynch; author: DLynch):

[schemas/event/secondary@master] EditAttemptStep: update init_timing documentation

https://gerrit.wikimedia.org/r/725018

Change 725018 merged by jenkins-bot:

[schemas/event/secondary@master] EditAttemptStep: update init_timing documentation

https://gerrit.wikimedia.org/r/725018