Proposing chaining/builder pattern for setting up pipelines.
owais opened this issue · 2 comments
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
@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.