Adding Watchable and Closeable to ParserProvider interface #2954
Conversation
| go func() { | ||
| err := watchable.WatchForUpdate() | ||
| switch { | ||
| case errors.Is(err, configsource.ErrSessionClosed): |
There was a problem hiding this comment.
If wanted we could add specific errors directly on parserprovider package so no import of configsource.
| "context" | ||
|
|
||
| "go.opentelemetry.io/collector/config" | ||
| ) | ||
|
|
||
| // ParserProvider is an interface that helps providing configuration's parser. | ||
| // Implementations may load the parser from a file, a database or any other source. | ||
| type ParserProvider interface { | ||
| // Get returns the config.Parser if succeed or error otherwise. | ||
| Get() (*config.Parser, error) |
There was a problem hiding this comment.
Here it will be good to pass the logger and ApplicationStartInfo.
There was a problem hiding this comment.
No need in the first version.
Codecov Report
@@ Coverage Diff @@
## main #2954 +/- ##
==========================================
- Coverage 91.68% 91.58% -0.10%
==========================================
Files 312 312
Lines 15341 15357 +16
==========================================
Hits 14065 14065
- Misses 870 883 +13
- Partials 406 409 +3
Continue to review full report at Codecov.
|
| go func() { | ||
| err := watchable.WatchForUpdate() | ||
| switch { | ||
| case errors.Is(err, configsource.ErrSessionClosed): |
| "context" | ||
|
|
||
| "go.opentelemetry.io/collector/config" | ||
| ) | ||
|
|
||
| // ParserProvider is an interface that helps providing configuration's parser. | ||
| // Implementations may load the parser from a file, a database or any other source. | ||
| type ParserProvider interface { | ||
| // Get returns the config.Parser if succeed or error otherwise. | ||
| Get() (*config.Parser, error) |
There was a problem hiding this comment.
No need in the first version.
| // If provider is watchable start a goroutine watching for updates. | ||
| if watchable, ok := app.parserProvider.(parserprovider.Watchable); ok { | ||
| go func() { | ||
| err := watchable.WatchForUpdate() |
There was a problem hiding this comment.
I guess we expected this method to block until an error occurs? If that is the case I think this should be explicit in the Watchable interface.
|
Just to get this clear. Whenever we reload the service because of a config change we will shutdown all components and launch a new instance which will then start receiving ingress, isn't? |
* install goimports w/ tools * lint fixes
Add interfaces to ParserProvider so it can handle configurations that trigger reload. Intended to be used with config sources.
Missing: a test to cover the update configuration on appplication_test.go