Adjust error instrumentation for easier aggregation

[markfields]

Work Item

Describe the outcome you expect
As it is, we have many many cases where error messages contain variable info, whether from our own system or a dependency, which dilutes error counts and can hide top errors. Assert/Error messages should be "static" for a particular code point, so that we can properly aggregate errors to recognize top errors. See #6676 design, we'll use errorCode as a distinct field from message, containing static strings only.

Approach
Query our Microsoft internal data to look at distinct values of Data_error and find patterns where a similar message has variations that preclude aggregation. Meanwhile we can also proactively update our own instrumentation to use error code (e.g #6754)

There are going to be three main buckets of reasons from my experience so far:

• Our own instrumentation including variables that should be pulled to telemetry props (hopefully Update error classes to implement IFluidErrorBase #6754 addresses these)
• Our own instrumentation copying the message from a response payload from the server, e.g. Socket IO errors. In these cases, we probably want to attempt to model the server response (e.g. JSON structure or certain expected cases based on message substrings), and produce error codes based on the model, leaving the full message in the message (Data_error) field.
• Unexpected errors arising from our own code or other client code we call that aren't instrumented with error code (e.g. dereferencing undefined or an exception arising from Scriptor or another DataStore. In these cases, error code would be added via normalizeError with errorCodeIfNone, so consider if there is a choke point where we should be normalizing.

Open questions

• How to handle error messages like this from dependencies? Related to Redact error messages from dependencies we don't control to avoid PII leaks #4908
  • message will always be uncontrolled chaos. That's why we're introducing errorCode.

Acceptance criteria
Querying production logs and grouping by errorType and errorCode should yield a relatively scoped set of buckets containing no variable data in those two fields, with not too much noise under the none buckets.

[ksbrar]

It turns out that the example errors above that caused a lot of duplication issues went away with shortcodes (#5619).

However, digging into the long tail of errors there are still cases of duplication or empty errors that can be fixed, so I will use this issue to log some of those cases.

[ksbrar]

Some errors are duplicated because they have a secondary message attached, though they boil down to the same event. Examples:

• "Error 403" vs "Error 403 (Forbidden)"
• "Error 404" vs "Error 404 (Not Found)"

In some of these cases, we have two occurrences because of the fact that we join the "performance" and "error" event tables in Kusto to track errors. These should be dealt with via better queries.

Examples to fix by hand:
*

[markfields]

I've written a tool that boils our 30k+ unique error messages down to about 100 canonical forms (e.g. prefix, or fuzzy match pattern string). The top-hitter was from us logging the message field off the socket.io response object in case of an error. I just fixed that in PR #6557

See this query

[markfields]

edit: I updated the original description to this effect too

There's a design #6676 I'm socializing around adding a new field errorCode, kept separate from the original error message. In cases where we originate an error (e.g. asserts or anywhere else we new up an error object not to wrap another), we will be sure to use error code, and likewise have a simple/static error message.

[markfields]

@vladsud once I get a bit more consensus on the direction in #6676 I'll remove the design-required label here.

[markfields]

Now that IFluidErrorBase is in, with fluidErrorCode, we can start this work in earnest. I'm about to head offline for 2 months for paternity leave, but before I go I hope to land with a concrete plan for this issue.

Will be something like opening specific issues for the couple areas that are a priority to reinstrument, and not tracking the long tail but rather depending on training the team to fix them up as needed/convenient. At that point I'll close this issue.

[markfields]

Moving this to "Next" milestone. For August, let's just focus on #6754 (Updating our own instrumentation) which will cover many of these cases. Then we can see where the noise is and what area to look at next. Also, if specific cases emerge from live site issues from the Office Fluid team then we can take those on a case-by-case basis.

[markfields]

Closing this issue, its utility has dried up now that IFluidErrorBase and fluidErrorCode are in and it's just a matter of iterating on cleaning up our errors signal release after release.