Closed Bug 1675286 Opened 4 months ago Closed 3 months ago

Enable events for the C++ and JS API of FOG

Categories

(Toolkit :: Telemetry, task, P1)

task

Tracking

()

RESOLVED FIXED
85 Branch
Tracking Status
firefox85 --- fixed

People

(Reporter: janerik, Assigned: janerik)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [telemetry:fog:m6])

Attachments

(2 files, 1 obsolete file)

Event metrics are generic over their extra keys.
That's a bit of a challenge for both the ID->object map in Rust and the way the wrapper has to work for C++ and JS.

This bug is about investigating the proper way to just that.

In theory we want the Rust API for events to be as safe and simple to use as possible.
That means we want it to be strictly typed over the extra keys a certain event can take,
disallowing to mix foreign extra keys into an event and statically preventing that.

The metric types as implemented in Rust for FOG are serving two purposes:

  1. A nice Rust API for Rust consumers.
  2. The API that is wrapped in C++/JavaScript bindings.

The nice event metric API with static checks requires a generic type:
EventMetric<K>, where K holds the allowed extra keys, generated by glean_parser at build time.

Now in FOG each metric is identified by its metric id, by which it can be looked up in a map.
That map can hold only a single type of a metric.
If we have n different event metrics with n different sets of extra keys we can't store them in a single map.
We also can't have a map per event (that would easily become a lot of maps and also we don't have the generic type on the C++ side)

We can resolve this by weakening the API a bit:

We make an EventMetric only generic over whether it has no extra keys or some extra keys.
This allows us to separate these events in 2 categories, requiring only 2 maps.

This further allows us to implement functionatility for either group specifically.

Events with no extra keys don't take anything on record().
Nothing needs to be serialized and it requires no runtime checks that extra values are allowed. There are none.

Events with some extra keys require a bit more work.

  • For Rust consumers we can introduce a runtime type check to ensure the right extra keys are passed.
  • For C++/JavaScript consumers we only get the string->string mapping of extra keys/values.
    With the generated code we guide people to do the right thing.
    For misuse that works around generated code we still have some checks
    (by only mapping valid extra keys to their serialized variant).

What does this mean for the eventual RLB types?

They will need to use the same strategy, otherwise we can't wrap them in FOG.
That's a severe limitation of the API for Rust consumers outside of Firefox,
but I don't see any other way to make it work right now.

Note: a first implementation of the C++ side of things will follow in a
separate commit.

Attachment #9187665 - Attachment is obsolete: true

Events are special in that they are generic over the type of extra keys
they can handle.
This makes them unsuitable to store in simple hashmaps, as (nearly) every event metric is its own type.

To still keep the nice and statically-checked Rust API, but also have
C++ and JS APIs we defer it all to a big ol' lookup table using match
at record() time.

Note:
While passing extras works it's not yet perfect.
The C++ API is hard to use.
The JavaScript API works, but is not documented.
Additionally testGetValue doesn't work yet for events in either language.

Blocks: 1678331
Blocks: 1678567
Duplicate of this bug: 1673644
Depends on: 1680230
Blocks: 1677962
Pushed by jrediger@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/45c8c0f1d7bf
Implement C++ and JS APIs for events. r=emilio,chutten
https://hg.mozilla.org/integration/autoland/rev/6c6a59d7c472
Use the RLB event implementation. r=chutten
Pushed by jrediger@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1b95ece88bff
Implement C++ and JS APIs for events. r=emilio,chutten
https://hg.mozilla.org/integration/autoland/rev/9c54c0487e03
Use the RLB event implementation. r=chutten
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 85 Branch
You need to log in before you can comment on or make changes to this bug.