Allow all exporters to be batched and queued #376
Conversation
Testing Done: unit tests
| doneFns, traceExporters, _ := createExporters(opts.RawConfig, logger) | ||
|
|
||
| if spanSender == nil && len(traceExporters) == 0 { | ||
| logger.Fatal("Unrecognized sender type or no exporters configured") |
There was a problem hiding this comment.
minor nit: may be good to differentiate the two cases.
| var cfg struct { | ||
| Exporters *struct { |
There was a problem hiding this comment.
Does this radically change what the DataDog exporter will look like in the YAML file?
| @@ -33,22 +36,24 @@ func TestPrometheusExporter(t *testing.T) { | |||
| }{ | |||
| { | |||
| config: ` | |||
| exporters: | |||
There was a problem hiding this comment.
Changing this is a little worrying, does this mean that we shall no longer have sections?
|
Thank you for working on this @sjkaris! I have a bit of a concern: exporters:
<exporter_name>:
<exporter_config>to <exporter_name>:
<exporter_config>This change seems more of a Viper change than batching, but if I am not mistaken, please send that change separately as it'll make it easier to review but also to catch bugs and keep batching for exporters focused too. Thank you. |
|
This PR doesnt change the way the config is written at all. Basically, with this change, the This change is actually only for batching, the viper changes are done since without it there is no way to instantiate the exporters without a root reference. |
| zipkin: | ||
| upload_period: 1ms | ||
| endpoint: ` + cst.URL | ||
| v := viper.New() |
There was a problem hiding this comment.
This pattern is quite common throughout the code and most likely might make writing tests warrant reading through viper but also bytes.NewBuffer.
How about let's make an internal helper to do all this parsing?
func ViperFromYAML(yamlBlob []byte) (*viper.Viper, error) {
v := viper.New()
v.SetConfigType("yaml")
if err := v.ReadConfig(bytes.NewBuffer(yamlBlob)); err != nil {
return nil, err
}
return v, nil
}
cmd/ocagent/main.go
Outdated
| @@ -87,7 +89,10 @@ func runOCAgent() { | |||
| log.Fatalf("Could not instantiate logger: %v", err) | |||
| } | |||
|
|
|||
| traceExporters, metricsExporters, closeFns, err := config.ExportersFromYAMLConfig(logger, yamlBlob) | |||
| // TODO(skaris): move the rest of the configs to use viper | |||
| v := viper.New() | |||
There was a problem hiding this comment.
We forgot to set below v.SetConfigType("yaml") which directly goes to my suggestion to extract such code into a single helper.
Adds the ability for all exporters to be batched and queued (part 2 of the original batching PR #361)
Testing Done: unit tests