feat(opentelemetry-sdk-trace-base): Add optional forceFlush property to SpanExporter interface

[JacksonWeber]

Which problem is this PR solving?

Add mandatory forceFlush property to the SpanExporter interface to bring it in line with the spec.

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes #3067

Short description of the changes

Added the forceFlush property to the SpanExporter interface and implemented its functionality in the included exporters.

Type of change

• New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Added appropriate unit tests.

Checklist:

• Followed the style guidelines of this project
• Unit tests have been added
• Documentation has been updated

[JacksonWeber]

Going to be adding some updated implementations of the forceFlush() method in the appropriate exporters to ensure they wait for in-flight requests to be completed.

[JacksonWeber]

I'm a bit confused what forceFlush() in an exporter like the InMemorySpanExporter is supposed to do when it looks like the SimpleSpanProcessor now collects any unresolvedExports and has its own forceFlush() method that will resolve the array of promises as of #3460.

[codecov]

Codecov Report

Merging #3753 (0576a9d) into main (17eca4c) will decrease coverage by 0.33%.
The diff coverage is 92.30%.

❗ Current head 0576a9d differs from pull request most recent head d22a10e. Consider uploading reports for the commit d22a10e to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3753      +/-   ##
==========================================
- Coverage   93.29%   92.96%   -0.33%     
==========================================
  Files         257      295      +38     
  Lines        7705     9038    +1333     
  Branches     1546     1842     +296     
==========================================
+ Hits         7188     8402    +1214     
- Misses        517      636     +119     

Impacted Files	Coverage Δ
...y-sdk-trace-base/src/export/ConsoleSpanExporter.ts	82.35% <50.00%> (ø)
...ackages/otlp-exporter-base/src/OTLPExporterBase.ts	95.23% <100.00%> (+0.23%)	⬆️
...ckages/opentelemetry-exporter-jaeger/src/jaeger.ts	95.06% <100.00%> (+0.12%)	⬆️
...ckages/opentelemetry-exporter-zipkin/src/zipkin.ts	100.00% <100.00%> (ø)
...-sdk-trace-base/src/export/InMemorySpanExporter.ts	100.00% <100.00%> (ø)
...y-sdk-trace-base/src/export/SimpleSpanProcessor.ts	92.50% <100.00%> (ø)

... and 41 files with indirect coverage changes

[JacksonWeber]

@pichlermarc @dyladan Should be ready for final review and merge now. Thanks for your help in this process!

[JacksonWeber]

@pichlermarc @dyladan This should be all good to merge now that conflicts/tests are resolved. Thanks!

[pichlermarc]

This looks good, thank you @JacksonWeber for getting this one to the finish line 🙂

I have a question for @dyladan @legendecas before approving though: in #3067 there was a discussion about how adding something to an interface is not considered a breaking change - is this still something that is agreed on? Seems to me like this would definitely cause some problems when released as a semver minor bump (i.e. a user's custom exporter not compiling anymore, runtime errors, type incompatibility).

[JacksonWeber]

@pichlermarc Addressed your comment so the PR should be good now, but let me know, and thank you for your review on this work!

[pichlermarc]

Looks great, thank you for addressing the comments. 🙂
(FYI I took the liberty to update the changelog entry to save ourselves once cycle 🙂 )

[JacksonWeber]

@dyladan Just needs your review and it should be ready to merge 🙂

[dyladan]

LGTM but the workflow is blocking

[dyladan]

Is this file supposed to be here?