refactor(sdk-logs, sdk-trace)!: refactor BatchSpanProcessor and BatchLogRecordProcessor constructor signature

[trentm]

Note: The BatchSpanProcessor from sdk-trace-base is not being changed.
A breaking change on sdk-trace is allowed because sdk-trace hasn't been published yet.

This changes BatchSpanProcessor and BatchLogRecordProcessor to take a single options argument, and changes the name of BufferConfig type to BatchLogRecordProcessorOptions.

Before (effectively):

// in sdk-logs
class BatchLogRecordProcessor implements LogRecordProcessor {
  constructor(exporter: LogRecordExporter, config: BufferConfig) { ... }
}

// in sdk-trace
class BatchSpanProcessor implements SpanProcessor {
  constructor(exporter: SpanExporter, config: BufferConfig) { ... }
}

// usage example
const processor = new BatchLogRecordProcessor(exporter, { maxQueueSize: 1000 });
const processor = new BatchSpanProcessor(exporter, { maxQueueSize: 1000 });

After (effectively):

class BatchLogRecordProcessor implements LogRecordProcessor {
  constructor(options: BatchLogRecordProcessorOptions) { ... }
}

class BatchSpanProcessor implements SpanProcessor {
  constructor(options: BatchSpanProcessorOptions) { ... }
}

// usage example
const processor = new BatchLogRecordProcessor({ exporter, maxQueueSize: 1000 });
const processor = new BatchSpanProcessor({ exporter, maxQueueSize: 1000 });

A note on two BatchSpanProcessor classes

With this change the two BatchSpanProcessor classes (one in sdk-trace, one in sdk-trace-base) have slightly different call signatures. That will potentially be confusing to users for a while.

The plan (as discussed in the description of #6775) is to:

• migrate usage of sdk-trace-base, sdk-trace-node, and sdk-trace-web over to sdk-trace (in this repo, the contrib repo, and opentelemetry.io docs),
• then deprecate the sdk-trace-* packages in favour of sdk-trace. I'll add a @deprecated to BatchSpanProcessor in sdk-trace-base at that point.

Motivation

The change away from interface BufferConfig was motivated by confusion on this PR: #6504 (comment)
That PR needed to add a meterProvider option to a BatchSpanProcessor -- on which self-observability SDK metrics can be added. Adding meterProvider to a type called BufferConfig feels wrong, even though it belongs on the type that holds all the options for a BatchSpanProcessor.

Two arguments or one to the constructor?

The interface change could also have been the following. I.e. just change the name of BufferConfig.

class BatchSpanProcessor implements SpanProcessor {
  constructor(exporter: SpanExporter, options?: BatchSpanProcessorOptions) { ... }
}

This would have been a smaller change. Also, user code would have have to change how arguments were passed.
Thoughts? Preferences?

There aren't many examples of SDK classes that take required arguments and optional arguments to have a clear patterns on which constructor style should be preferred for consistency.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.95%. Comparing base (e8b0b76) to head (31b2203).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6817   +/-   ##
=======================================
  Coverage   94.95%   94.95%           
=======================================
  Files         382      383    +1     
  Lines       12912    12968   +56     
  Branches     2945     2961   +16     
=======================================
+ Hits        12260    12314   +54     
- Misses        652      654    +2     

Files with missing lines	Coverage Δ
...ental/packages/opentelemetry-sdk-node/src/utils.ts	97.01% <100.00%> (ø)
...sdk-logs/src/export/BatchLogRecordProcessorBase.ts	91.21% <100.00%> (ø)
...platform/browser/export/BatchLogRecordProcessor.ts	100.00% <100.00%> (ø)
...rc/platform/node/export/BatchLogRecordProcessor.ts	66.66% <100.00%> (ø)
...etry-sdk-trace-base/src/BatchSpanProcessor-shim.ts	100.00% <100.00%> (ø)
...ges/sdk-trace/src/export/BatchSpanProcessorBase.ts	95.68% <100.00%> (ø)
.../src/platform/browser/export/BatchSpanProcessor.ts	100.00% <100.00%> (ø)
...ace/src/platform/node/export/BatchSpanProcessor.ts	100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[overbalance]

I like single argument objects 👍 Also, experimental/packages/exporter-logs-otlp-grpc/README.md has a few examples that need the new signature as well

[JacksonWeber]

I like single argument objects 👍 Also, experimental/packages/exporter-logs-otlp-grpc/README.md has a few examples that need the new signature as well

exporter-logs-otlp-http has some examples that need updating as well 😄

[pichlermarc]

The interface change could also have been the following. I.e. just change the name of BufferConfig.

class BatchSpanProcessor implements SpanProcessor {
  constructor(exporter: SpanExporter, options?: BatchSpanProcessorOptions) { ... }
}

This would have been a smaller change. Also, user code would have have to change how arguments were passed. Thoughts? Preferences?

There aren't many examples of SDK classes that take required arguments and optional arguments to have a clear patterns on which constructor style should be preferred for consistency.

IMO your proposal is the way to go. I've had interactions with users that were confused why the PeriodicExportingMetricReader and the BatchSpanProcesssor have different ways of setting the exporter. Aligning them like this is a welcome change.

---

Change looks good % the things Jared and Jackson have already mentioned :)

[trentm]

[Jared]

Also, experimental/packages/exporter-logs-otlp-grpc/README.md

[Jackson]

exporter-logs-otlp-http has some examples that need updating as well

Thanks. commit 31b2203