Watch config file for changes

[jpkrohling]

Viper has support for watching the configuration file for changes and it would be good to have a policy on what to do when that happens. Looking a bit around the code, I can see three main possibilities:

• Shutdown + init: this seems to be the easiest with the current codebase
• Add a OnConfigChange to all components, such as exporters and receivers, allowing them to be notified of config changes. This is the cleanest and probably best for the future, as it allows each component to hold incoming requests while in-flight requests finish.
• Nothing: just log a message stating that the config file has new content and that opentelemetry-service expects an orchestrator to kill the process to get the new config file in effect.

For the first and second options, it might also make sense to react on SIGHUP.

[ccoqueiro]

Hello folks, i have a question from customer: "We need to filter which logs we end to you guys. Right now, we use logspout API to make hot changes on container. With that we exclude logs from being sent to LogDNA. With this new LogObserver approach, we changed logspout for fluentd. Is there a way to hot-update those exclusions in fluentd? We thought about filtering in the OtelCollector GW that we’ll have for logs. If possible, we can centralize exclusions there, but we need a way to “hot-reload” its config." In this case, this options here would be useful? Thanks

[dyladan]

I would propose the following:

1. For now we implement (1) as it is the quickest win. Make the feature opt-in with a configuration like RestartOnConfigChange (or opt-out if maintainers prefer?)
2. Implement stub OnConfigChange implementation in each component here and in contrib which logs that the component does not support live reloading
3. Handle SIGHUP to call OnConfigChange
4. Create a new experimental configuration LiveReloadOnConfigChange (default false) (name up for debate) which would prevent the collector from restarting when config changes and instead call OnConfigChange handlers
5. (long term) Implement OnConfigChange in all packages
6. (probably very long term) make LiveReloadOnConfigChange default true?

If this looks agreeable I would like to be assigned and start work on this.

[Mario-Hofstaetter]

Another way of implementing this as done by prometheus is via an optional HTTP endpoint command:
https://prometheus.io/docs/prometheus/latest/configuration/configuration/

Prometheus can reload its configuration at runtime. If the new configuration is not well-formed, the changes will not be applied. A configuration reload is triggered by sending a SIGHUP to the Prometheus process or sending a HTTP POST request to the /-/reload endpoint (when the --web.enable-lifecycle flag is enabled). This will also reload any configured rule files.

[dyladan]

Another way of implementing this as done by prometheus is via an optional HTTP endpoint command: prometheus.io/docs/prometheus/latest/configuration/configuration

Prometheus can reload its configuration at runtime. If the new configuration is not well-formed, the changes will not be applied. A configuration reload is triggered by sending a SIGHUP to the Prometheus process or sending a HTTP POST request to the /-/reload endpoint (when the --web.enable-lifecycle flag is enabled). This will also reload any configured rule files.

I think manual trigger by http or signal is a reasonable behavior. In the end state I would probably expect live reloading based on file changes to be an opt-in behavior as it might cause unexpected side-effects but signal handling should be a default probably non-configurable behavior. An HTTP endpoint might make sense if there is already an HTTP API to the collector (not sure if this is the case).

[bogdandrutu]

We already have implemented the logic to do restart, but we do not have the logic to "watch" for file change. See https://github.com/open-telemetry/opentelemetry-collector/blob/main/confmap/provider/fileprovider/provider.go#L49 where we don't do anything with confmap.WatcherFunc

[swiatekm]

@bogdandrutu would you be willing to accept a PR implementing the watch in the file provider?

[bogdandrutu]

@swiatekm-sumo yes, but please check for an already existing PR and see why that did not get accepted, and try to fix that.