Closed Bug 1730112 Opened 4 years ago Closed 3 years ago

FOG does not record error on malformed UUIDs

Categories

(Data Platform and Tools :: Glean: SDK, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Dexter, Unassigned)

References

Details

This is what powers FOG's UuidMetric type JS API. It uses string and this means that it could be ok, for the API, to accept an empty string or a non standard UUID.
In case the UUID would fail to parse (in FOG) no error would be recorded (because the failure is on parsing on the FOG side).

I think in the UUID case, FOG should be using set_from_str, which is currently not exposed by glean-core.
Because in that case, if the UUID is bogus, it would print and record an error .

I'll slyly move this to the Glean SDK where the first part of the work will need to be done (exposing an API on the RLB).

set_from_str was previously an internal-only API, so some thought should be given to what we're doing by adding it to the RLB "just" so FOG can have consistent error handling.

See also bug 1691073 where we're trying to figure out a "better way" for error handling. Might be related, might not.

Component: Telemetry → Glean: SDK
Product: Toolkit → Data Platform and Tools
See Also: → 1691073
Assignee: nobody → jrediger
Priority: -- → P2

Exposing the set_from_str is reasonably easy.
A more general way for error reporting might be better solved by UniFFI (because we can avoid all the boilerplate necessary and let UniFFI handle that for us, mostly).

So this comes down to: do we invest the bit of time to expose set_from_str now to record errors for UUID from Desktop
or do we wait for the better solution, which won't be here before Q4 for sure?

:chutten, do you anticipate UUID users that could run into errors anytime soon?

Flags: needinfo?(chutten)
See Also: → 1704504

No doubt someone'll make a point of misusing it, but UUIDs are infrequently used at all, so misuse should be infrequent.

We could, in FOG, "just" instrument the failure-to-parse on the "metrics" ping (best we can do until bug 1691073) in the interim so we have something to check when things start smelling "off". But I'd really like bug 1691073 to provide the language-specific SDKs a better reporting mechanism first. I'd rather do this once in bug 1704504 rather than piecemeal like this.

Flags: needinfo?(chutten)
Assignee: jrediger → nobody
Priority: P2 → P3

The new UniFFI-based API takes a string, and thus parsing happens in Rust. It records an error when that fails.
There's no urgent need to backport that to current Glean.

Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.