Make all-in-one.yaml file independant of sampling-strategies.json file #6420
Conversation
|
before you go any further please make sure you always sign commits |
| @@ -78,10 +75,6 @@ func (cfg *Config) Unmarshal(conf *confmap.Conf) error { | |||
| } | |||
|
|
|||
| func (cfg *Config) Validate() error { | |||
| if cfg.File == nil && cfg.Adaptive == nil { | |||
There was a problem hiding this comment.
allowing empty file name does not mean we allow not specifying a provider
There was a problem hiding this comment.
Okay. so what shall we take as the default strategy, (since if we want to remove dependancy on sampling-strategies.json, then there would be no default input left). I had allowed null for that reason
There was a problem hiding this comment.
Do not confuse strategy (the final output) with the provider (file or adaptive). I still want to use file provider in this case, but without a file name the provider can return a default strategy, which is probabilistic/1.0
There was a problem hiding this comment.
The reason to still use the file provider is because the user can override the file name via --set CLI flag, even if they do not provide a full config file.
There was a problem hiding this comment.
okay, got it, so I just have to add this test back again.
I had accidentally closed this pr, so creating a new one
| @@ -30,7 +30,7 @@ extensions: | |||
| remote_sampling: | |||
| # We can either use file or adaptive sampling strategy in remote_sampling | |||
| file: | |||
| path: ./cmd/jaeger/sampling-strategies.json | |||
| path: ~ | |||
There was a problem hiding this comment.
| path: ~ | |
| path: |
Which problem is this PR solving?
Description of the changes
How was this change tested?
sampling-strategies.json(the service "foo").jaeger/cmd/jaeger/internal/extension/remotesampling/extension_test.go
Line 83 in b02900c
the
sampling-strategies.jsonfile could not be removed1.) TestServerHTTP_TracesRequest,
2.) all 3 tests in cmd/query/app
which are not passing even on the main branch itself (according to me, otherwise I have made some mistake), and hence these same tests are not passing after I made my changes (my changes should not affect them anyway).
Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test