Closed Bug 1709147 Opened 3 years ago Closed 3 years ago

Consider whether FOG's Datetime should return something better than a String in C++ and JS

Categories

(Toolkit :: Telemetry, task, P1)

task

Tracking

()

RESOLVED FIXED
90 Branch
Tracking Status
firefox90 --- fixed

People

(Reporter: chutten, Assigned: chutten)

References

(Blocks 1 open bug)

Details

(Whiteboard: [telemetry:fog:m?])

Attachments

(3 files)

The MLA languages (C++ and JS) return an ISO8601-formatted string value for testGetValue which, though precise, requires parsing to be useful to users writing instrumentation tests.

Since the APIs take better types than string (PRExplodedTime for C++, Date for JS), maybe the test API should return such a better type, too?

This bug is for considering the ramifications of this (FFI-compatible returned data type, what does a JS date look like as a jsval xpidl type, is the API support for checking string values actually better than the API support for checking these 'better' types, etc) and implementing that decision (either by documenting the rationale in The Book or by changing the behaviour (and then documenting the changed behaviour in The Book)).

Blocks: 1670259

The MLA languages (C++ and JS) return an ISO8601-formatted string value for testGetValue which, though precise, requires parsing to be useful to users writing instrumentation tests.

If the goal is to follow exactly what the other bindings do, the ISO string returned should be exactly the same that will be recorded i.e. it is truncated to the time unit. This makes it even harder for parsing (in JS at least).

We can use glean-core's test_get_value_as_string to make sure it works exactly like everywhere else. In hindsight that is what I should have done when first implementing this, oops.

However, that is for testGetValueAsString. For testGetValue we should return a date object. Not sure what that is for C++ in mozilla-central. For Javascript it is the Date object. That would match what Glean.js does.

Yeah, of the two (returning a truncated string, returning a better object) I prefer returning a nicer thing. Gaining compat with Glean.js is worth the effort on its own, and it means we can support a nice C++ API while we're at it.

Assignee: nobody → chutten
Status: NEW → ASSIGNED
Priority: P3 → P1

Depends on D115375

Pushed by chutten@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/403d0030c4e9
Move test-only pref description to proper doc section r=janerik
https://hg.mozilla.org/integration/autoland/rev/a16d49b60ee1
Break FOG's ffi mod into many mods r=janerik
https://hg.mozilla.org/integration/autoland/rev/987000aaec55
Return Date types from FOG datetime testGetValue in C++ and JS r=brizental,janerik
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch
Blocks: 1712772
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: