agent icon indicating copy to clipboard operation
agent copied to clipboard

Flow: consider removing singleton component concept

Open rfratto opened this issue 2 years ago • 2 comments

The initial implementation of the Flow controller allows components to register themselves as "singletons," meaning only one of that component may be instantiated:

https://github.com/grafana/agent/blob/c07d3875cb402719e6003fefe694f26606ebb5d3/component/registry.go#L68-L89

This was introduced because some integrations only support existing once per process:

  • agent
  • cAdvisor
  • eBPF
  • eventhandler
  • node_exporter
  • process_exporter
  • snmp_exporter
  • statsd_exporter
  • windows_exporter

I think the singleton component will cause long-term problems, especially around if we ever wanted to support modules, which seems inevitable: two modules that both launch node_exporter with different settings will conflict, and one will be rejected based on load order.

I think we should slowly invest effort in:

  1. Determining which of these integrations breaks if more than one exists per process (I know node_exporter and cAdvisor do due to using global flags to function)
  2. Ensuring that the Flow components of these integrations do not function as singletons
  3. Removing the singleton flag from component registration

This is probably a blocker for Flow leaving experimental. Alternative suggestions are welcome for how we can potentially avoid conflict for modules (which although unplanned at the moment, feel inevitable for Flow).

Integrations which need updates to support multiple instances:

  • [ ] cAdvisor
  • [ ] node_exporter

List of integrations which could support multiple instances today:

  • agent

List of integrations which need investigation:

  • eBPF
  • eventhandler
  • process_exporter
  • snmp_exporter
  • statsd_exporter
  • windows_exporter

rfratto avatar Jul 07 '22 00:07 rfratto

cc @rfratto @captncraig while looking at #2095, I realised that when we moved from HCL to using River throughout the controller in #1954, we lost the handling around Singleton field.

Right now, it's not being utilised anywhere and for example, you can have two instances of prometheus.integrations.node_exporter at the same time. Does this make it any more pressing to figure out a solution?

tpaschalis avatar Sep 15 '22 13:09 tpaschalis

Right now, it's not being utilised anywhere and for example, you can have two instances of prometheus.integrations.node_exporter at the same time. Does this make it any more pressing to figure out a solution?

Ah, I see what you mean. Yeah, that's not good and is probably a release blocker.

rfratto avatar Sep 15 '22 13:09 rfratto

Singleton components aren't allowed in the context of modules, which would prevent remove configuration of components like prometheus.exporter.unix. That's all the more reason, IMO, to replace all singleton components with non-singletons and remove the flag.

rfratto avatar Mar 02 '23 03:03 rfratto

After merging cadvisor and unix flow components removing Singleton, I think we can close this issue. I didn't see any references of Singleton in main.

marctc avatar Oct 09 '23 10:10 marctc