Skip to content

Flush FedifySpanExporter exports before the invocation ends #1382

Open

Description

@dahlia

Motivation

FedifySpanExporter.export() starts an asynchronous KvStore.set() for each record and returns. forceFlush() and shutdown() are currently no-ops, so nothing waits for those writes.

On Cloudflare Workers, SimpleSpanProcessor calls export() synchronously from span.end() inside Federation.fetch(). The response can be returned before the KV writes finish, and the isolate can terminate them. BatchSpanProcessor is worse because its timer does not survive the response.

The result is missing trace records in the debugger and in getRecentTraces(). ActivityPub delivery is not affected.

Proposed change

Track in-flight exports and make the exporter's lifecycle methods wait for them:

readonly #inFlight = new Set<Promise<void>>();

export(spans, resultCallback): void {
  const promise = this.#exportAsync(spans)
    .then(() => resultCallback({ code: ExportResultCode.SUCCESS }))
    .catch((error) => {
      logger.error("Failed to export spans to KvStore: {error}", { error });
      resultCallback({ code: ExportResultCode.FAILED });
    });
  this.#inFlight.add(promise);
  void promise.finally(() => this.#inFlight.delete(promise));
}

async forceFlush(): Promise<void> {
  while (this.#inFlight.size > 0) {
    await Promise.allSettled([...this.#inFlight]);
  }
}

async shutdown(): Promise<void> {
  await this.forceFlush();
}

Add waitUntil?: WaitUntilCallback to FedifySpanExporterOptions and pass a never-rejecting wrapper for the export promise to it. That is the direct Workers fix. The callback itself is introduced in #1383. forceFlush() remains useful for an explicit ctx.waitUntil(exporter.forceFlush()) call and for Node.js shutdown.

Use SimpleSpanProcessor on Workers and document that BatchSpanProcessor timers do not run after the response. Fedify's OpenTelemetry manual page, docs/manual/opentelemetry.md, currently shows SimpleSpanProcessor but does not mention the Workers lifetime.

Scope

  • packages/fedify/src/otel/exporter.ts.
  • docs/manual/opentelemetry.md.
  • Tests with a KvStore whose set() resolves after a delay: forceFlush() must wait, export() must still call resultCallback once, and a rejection must report FAILED without becoming unhandled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    Fields

    Priority

    None yet

    Effort

    None yet

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions