Clean up Span format

[aryanjassal]

Specification

As per some comments on #15, some design changes have been proposed and must be implemented.

Additional context

Example new type (instead of a single unified one):

type SpanId = IdSortableEncoded;

type SpanStart = {
  type: 'Start';
  id: SpanId;
  parentId?: SpanId;
};

type SpanStop = {
  type = 'Stop';
  id: SpanId;
  startId: SpanId;
  parentId?: SpanId;
};

Tasks

1. Change the terminology to start and stop or begin and end instead of start and end.
2. Need to incorporate the nodeId into the IdSortable
3. The parentId may or may not exist for both start and stop, and that means there's a possibility of a spans in one location and then fading out and then fading into another location

[linear]

ENG-612

[CMCDragonkai]

Node Id might already be. Fuzzy search for how IdSortable is used elsewhere.

[aryanjassal]

There is no specific type for IdSortableEncoded. This is the method signature of toMultibase in js-id. It returns a raw string.

/**
 * Encodes an multibase ID string
 */
public toMultibase(format: MultibaseFormats): string {
  return utils.toMultibase(this, format);
}

You might be remembering the Id type which is defined as follows.

type Id = IdInternal & number;

[aryanjassal]

Node Id might already be. Fuzzy search for how IdSortable is used elsewhere.

From my research, I couldn't find anywhere in Polykey where we passed the NodeId into the IdX constructor, but this signature of the constructor accepts it as a parameter. Polykey might be doing something more advanced to somehow include it in say, the vault names, but at a glance I couldn't figure it out.

class IdSortable<T extends Id = Id> implements IterableIterator<T> {
  protected nodeBits?: string;
  // ...

  public constructor({
    lastId,
    nodeId,
    timeSource = utils.timeSource,
    randomSource = utils.randomBytes,
  }: {
    lastId?: Uint8Array;
    nodeId?: Uint8Array;
    timeSource?: (lastTs?: number) => () => number;
    randomSource?: (size: number) => Uint8Array;
  } = {}) {
    // ...
  }
  
  // ...
}

[aryanjassal]

I just realised something — if nothing is consuming the events, then all the events will keep accumulating in memory in the queue, never to be consumed. Maybe I can let the user willingly discard all events if they don't intend on consuming them?

// We don't intend to consume span events
tracer.discardEvents();

It's a minor change which I can get merged alongside this issue.

[CMCDragonkai]

No if you read the observability issue, you would realise that by doing push flow subject observable, no memory will be accumulated, instead the data must be reacted to with a sink. That is better than expecting the user to garbage collect. In fact you're introducing a control panel potential footgun. You do not want to do this. Complexity needs to be reduced, not increased. Change it so that you start involving observable structure as explained with rxjs. Talk to Ed if you need to elaborate.

[CMCDragonkai]

Well do a fuzzy search over the usage of IdSortable. I'm sure it is being used somewhere perhaps in the id domain.

[aryanjassal]

No if you read the observability issue, you would realise that by doing push flow subject observable, no memory will be accumulated, instead the data must be reacted to with a sink. That is better than expecting the user to garbage collect. In fact you're introducing a control panel potential footgun. You do not want to do this. Complexity needs to be reduced, not increased. Change it so that you start involving observable structure as explained with rxjs. Talk to Ed if you need to elaborate.

I made a new issue #65 to track this, but it needs some discussion. Mainly because there would be some impact when writing events using a reactive callback. We might be able to mitigate it if we write in chunks, but then that might mean that on slower systems, backpressure might become an issue and hold back the main program from execution, which defeats the purpose of tracer being transparent.

We can shift this discussion over to #65 now.

[CMCDragonkai]

Back pressure isn't a problem because it's queued up in-memory. This makes sense since tracing is aligned with async.  Started with an infinite queue and monitor