tracesexporter: Allow multiple datasources in configuration, and round-robin load-balance amongst them
#190
Conversation
| // Initialize implements storage.Factory | ||
| func (f *Factory) Initialize(logger *zap.Logger) error { | ||
| func (f *Factory) Initialize(logger *zap.Logger) (clickhouse.Conn, error) { |
There was a problem hiding this comment.
Is there a clear way in Go to indicate what this return-value is, semantically? (I name the return-value init_conn below, but idk if folks are going to reflexively check the source …)
I'd rather indicate in the type/signature that the user shouldn't use the returned conn for anything other than initialization …
| suffixDatasource = ".datasource" | ||
| suffixDatasources = ".datasources" |
There was a problem hiding this comment.
I'm unclear on the best path forward here, re: naming, if my changes are adopted:
- Continue to use
datasource:, even though it can now be plural; - Switch names to be more semantic, but break backwards compat;
- Match the other exporter(s) and use
DSN:or evenDSNs:?
| type stringSliceValue struct { | ||
| slice *[]string | ||
| } | ||
|
|
||
| func (ssv *stringSliceValue) String() string { | ||
| return fmt.Sprintf("%v", *ssv.slice) | ||
| } | ||
|
|
||
| func (ssv *stringSliceValue) Set(value string) error { | ||
| *ssv.slice = append(*ssv.slice, value) | ||
| return nil | ||
| } | ||
|
|
There was a problem hiding this comment.
This is mostly via Google 🙈 — I'm not really familiar with flag; is this the appropraite way to get the desired --datasource foo --datasource bar behaviour?
| // Round-robin connection-selection strategy. | ||
| func (f *Factory) selectConn() clickhouse.Conn { | ||
| if len(f.conns) > 1 { | ||
| f.conns = append(f.conns[1:], f.conns[0]) | ||
| } | ||
| return f.conns[0] | ||
| } | ||
|
|
There was a problem hiding this comment.
This is, currently, not thread-safe. I'm not sure whether parallelism is utilized in signoz-otel-collector; should I include a mutex against the list of collectors or something like that?
| // Round-robin connection-selection strategy. | ||
| func (w *SpanWriter) selectConn() clickhouse.Conn { | ||
| if len(w.conns) > 1 { | ||
| w.conns = append(w.conns[1:], w.conns[0]) | ||
| } | ||
| return w.conns[0] | ||
| } |
There was a problem hiding this comment.
(and is there a way I can centralize/dedupe Writer.selectConn and Factory.selectConn, since they're identical?)
ELLIOTTCABLE
requested review from
ankitnayan,
makeavish,
nityanandagohain,
prashant-shahi and
srikanthccv
as code owners
This is my first-ever time touching anything Go related, so please, definitely go over my patch with a fine-tooth comb … very open to any sort of criticism!
This replaces the singular
db *clickhouse.Connvalues throughout theclickhousetracesexporterwith an array ofconns. Any db usage is routed through a simple round-robin load-balancer, with the exception if the initialization, which should stay on a single initial connection from the pool.Closes #189.
I've got a few questions, will attach them as a self-review below.