signalfx/splunk-otel-js

Proposing chaining/builder pattern for setting up pipelines.

owais opened this issue · 2 comments

owais commented

Today the API for our configuration code looks something like the following:

startTracing({
  endpoint: 'http://localhost:14268/api/traces',
  serviceName: 'my-node-service',
  spanProcessorFactory: processorFactory,
});

This design was mostly inherited from SignalFx libaries. While it works just fine, I think we can probably do better. I propose to use the chaining/builder pattern. For example, the above call could look like:

pipeline = newTracingPipeline()
     .withServiceName('my-node-service')
     .withEndpoint('http://localhost:14268/api/traces',)
     .withSpanProcessorFactory(processorFactory)
     .start()

// optionally could expose a .stop() method that flushes in-memory data
pipeline.stop()

Some advantages to this:

  • Can have different builder methods for tracing, metrics and logging easily while following consistent approach.
  • Can allow building multiple pipelines instead of a single, global one. This can be useful to configure multiple providers/processors/exporter for different parts of a monolith application for example.
  • Easier to maintain internals of constructor as validation/parsing logic for each config option would naturally be self-contained and testable.
  • Different methods can still be composed to provide a single super builder which initializes all signals
owais commented

@jtmalinowski @jtmal-signalfx thoughts on this? I still don't know which account you prefer :)

One drawback is that we loose the ability to warn about missing configuration options at compile time, we can only do it when .start() is run. I think chaining got a bit less popular when TS started getting popular for this reason. But, that's not a blocker for me.

Another way to build multi-pipeline setup could:

const baseConfig = { /* settings */ };
const pipeline1 = startTracing({
  ...baseConfig,
  override1: value1,
});
const pipeline1 = startTracing({
  ...baseConfig,
  override1: value2,
});

And yet another:

const basePipeline = new Pipeline({ /* settings */ });

const pipeline1 = basePipeline.cloneWith({ /* all properties are optional */ });
pipeline1.start();

If you have a strong preference, let me know. I suspect that, long-term, stronger TS integration will result in better customer experience and less time spent on support. Regardless of whichever option we implement, de-singletonizing, and detaching start from constructor is great.