SpanExporter interface inconsistency

[dyladan]

In the specification for SpanExporter, the Interface Definition paragraph states:

Interface Definition

The exporter must support two functions: Export and Shutdown.

However there are 3 functions listed: Export, Shutdown, and ForceFlush.

Is ForceFlush a required method on the SpanExporter interface?

Additional Context

While implementing the metrics SDK InMemoryMetricsExporter, there was some debate in the JS SIG on if we should have a separate reset method or if the forceFlush method should reset the in-memory state of the exporter. While deciding what to do, we looked at the InMemorySpanExporter for inspiration and found that it was using reset but did not even implement ForceFlush. Since ForceFlush is specified, we realized we had to implement it and an issue (open-telemetry/opentelemetry-js#3067) was created to track this. While working on the issue we became confused by the wording of the spec and are unsure if the spec is trying to require ForceFlush or specifying behavior for an optional interface. All behaviors of ForceFlush are specified as SHOULD requirements.

[yurishkuro]

@dyladan since you already did some leg work studying this portion of the spec and some implementations, do you have a concrete proposal how to address this?

[dyladan]

I see 2 options:

1. Add ForceFlush to the Interface Definition paragraph making it:

Interface Definition

The exporter must support three functions: Export, Shutdown, and ForceFlush.

2. Make ForceFlush specifically optional

Interface Definition

The exporter MUST support two functions: Export and Shutdown. The exporter MAY also support a third function: ForceFlush.

[dyladan]

If we go with 1 there may be other SDKs besides JS that need to be updated with the required function. If we go with 2 then any SDK like JS which haven't implemented it yet are not required to if they don't want to.

[jsuereth]

Looking at history of when forceflush was added, I think this may have just been an omission and your option (1) is the intended solution.

[dyladan]

Yeah I think that is most likely the original intent

[jsuereth]

What I mean was, there was a discussion in the PR about this specifically: #1452 (comment)

Bogdan suggested opening a new issue PR to resolve it.