IndexerCompleted events should express duration instead of startTime, endTime

[gobengo]

because JSON + ISO8601 can't express nanosecond durations https://nickb.dev/blog/iso8601-and-nanosecond-precision-across-languages/

And duration is 1) what we really care about in metrics-collector 2) what we actually have available at time of emitting IndexerCompleted 3) capable of expressing more precise measurement than previous startTime,endTime ISO8601 scheme

Plan

• Replace IndexingCompleted .indexing.{ startTime, endTime } properties with only an indexing.duration that expresses the value in nanoseconds.
  • because JSON can't express Nanoseconds or bigints, I'll serialize the nanoseconds bigint into a string + some metadata indicating that the string can be interpreted as a BigInt(string) with nanoseconds semantics

[gobengo]

• [x] start sending duration from indexer-lambda https://github.com/elastic-ipfs/indexer-lambda/pull/66
• [ ] update metrics-collector to use duration when present https://github.com/elastic-ipfs/metrics-collector/issues/34
• [ ] update metrics-collector to test accepting of indexing like { duration } as well as { startTime, endTime } https://github.com/elastic-ipfs/metrics-collector/issues/34
• [ ] indexer-lambda stop sending IndexerCompleted#indexing.endTime, only send startTime (ISO8601, mostly useful for sorting/human-reading) and duration (nanosecond units, powers prometheus metrics, etc)
• [ ] maybe metrics-collector stop accepting indexing with { endTime } (only accept { startTime, duration })

[alanshaw]

Can we just get some reasons why we need nanosecond precision?

• I don't think we'll be able to complete indexing even the smallest CAR file in less than 1ms.
• It makes calculating the duration complicated in javascript.
• It probably needs to be a string when serialized to JSON so you need to parse it on the other end.
• Anyone receiving the event and dealing with it in javascript (like, wanting to do any calculations) has a harder time than if it's milliseconds.

[gobengo]

@alanshaw we don't definitely need nanoseconds duration. happy to drop that. It still feels ideal to send an indexing duration instead of startTime/endTime pair. At the same time.... if we dont need to change it then we dont need to change it. i.e. maybe this isn't strictly needed for 'mvp'.

@alanshaw should I

• close this ticket and just not worry about changing from startTime/endTime to duration?
  • I could revert https://github.com/elastic-ipfs/indexer-lambda/pull/66
• change to duration but use milliseconds precision instead of nanoseconds
  • avoids the part that involves serializing/deserializing Nanoseconds from JSON, which I implemented here https://github.com/elastic-ipfs/indexer-lambda/pull/66/files#diff-571d39e255ae6347333c2b79a202c30a3f5a439468e5c7d7f8c1a03505a24c88R21 , but would be easy to change to serializing as milliseconds and not encoding the milliseconds Number as String (which nanoseconds/bigint requires in JSON)

I'm flexible either way.

One thing:

It makes calculating the duration complicated in javascript. ... Anyone receiving the event and dealing with it in javascript (like, wanting to do any calculations) has a harder time than if it's milliseconds.

Is it? const durationNs = BigInt(event.indexing.duration.value) vs const durationMs = Date.parse(event.indexing.endTime) - Date.parse(event.indexing.startTime))

• the latter is more characters/logic and less precision. neither is that hard to parse in JS

[alanshaw]

One thing:

It makes calculating the duration complicated in javascript. ... Anyone receiving the event and dealing with it in javascript (like, wanting to do any calculations) has a harder time than if it's milliseconds.

Is it? const durationNs = BigInt(event.indexing.duration.value) vs const durationMs = Date.parse(event.indexing.endTime) - Date.parse(event.indexing.startTime))

* the latter is more characters/logic and less precision. neither is that hard to parse in JS

Not in that case I guess. I think I was angling for duration to be in ms. If it's nanoseconds it's just a bit more difficult to do time maths with because Date is all in ms. You have to convert to bigints and remember how many zeros in a nanosecond and how to convert between.

[gobengo]

@alanshaw and I discussed and we're going to just pause this issue for now and focus on some other priorities. The IndexerCompleted events in elastic-ipfs/indexer-lambda are event.indexing.duration nanoseconds in addition to event.indexing.{startTime,endTime} ISO8601 strings. More than enough info. It's not as important that metrics-collector uses duration ns instead of endTime - startTime ms