SensorsIot / IOTstack

Docker stack for getting started on IOT on the Raspberry PI
GNU General Public License v3.0
1.44k stars 307 forks source link

Telegraf: remove unused mosquitto dependency #554

Open ukkopahis opened 2 years ago

ukkopahis commented 2 years ago

The mosquitto dependency for Telegraf is a misconfiguration.

There is a mosquitto example for reading from from mosquitto (.templates/telegraf/iotstack_defaults/additions/inputs.mqtt_consumer.conf), but this is NOT enabled by default. As such there shouldn't be a default dependency either. Docs updated with instructions to add this dependency if you use the the addition.

Reasons to remove the dependency:

Resolves #550

Paraphraser commented 2 years ago

I really hate to be "All Mr Negative" twice in the space of 24 hours but I also can't endorse this proposed change.

Let's work backwards.

Q1: Why does Telegraf's service definition contain:

depends_on:
  - mosquitto

A1: Because of ❶ in the following:

/home/pi/IOTstack/.templates/telegraf/iotstack_defaults/
├── additions
│   └── inputs.mqtt_consumer.conf ❶
└── auto_include
    ├── inputs.cpu_temp.conf
    └── inputs.docker.conf

Q2: How does inputs.mqtt_consumer.conf create a dependency on Mosquitto?

A2: Because of line 4 in that file:

servers = ["tcp://mosquitto:1883"]

Q3: Why is that line there?

A3: It is the direct descendent of line 171 in Graham Garner's original implementation.

Q4: When did Graham Garner create that dependency?

A4: November 2019. That's the initial commit, unmodified since.

So, the way I see it is, every single IOTstack user who has ever used IOTstack to instantiate a Telegraf container has had a default configuration that assumes Telegraf is capable of subscribing to topics distributed by a broker responding on the URL tcp://mosquitto:1883 which, in practice, means a Mosquitto container running alongside Telegraf on the same host.

I don't see how that logical chain meets any reasonable definition of "unused". The presence of mosquitto in the depends-on clause simply formalises the expectation set by Telegraf's default configuration that the mosquitto broker will be there.

It may not be a Telegraf dependency. But it's an IOTStack implementation of Telegraf dependency.

The dependency is documented in the Telegraf documentation. If there are situations where it would be better for the Telegraf container to not have this linkage with Mosquitto then perhaps the documentation should be expanded to explain the how-to:

  1. Comment-out the lines in inputs.mqtt_consumer.conf; and then
  2. Remove mosquito from the depends_on clause in the service definition.

With the usual caveats that I'm not in charge of Jack Split, and that this is all just my 2¢ …

👎

ukkopahis commented 2 years ago

"using an optional addition absolutely needs this dependency" (my takeaway)

Good point, docs updated with instructions to add this dependency back, if you use the the addition.

Telegraf container to not have this linkage with Mosquitto .. Comment-out the lines in inputs.mqtt_consumer.conf

Umm.. that is an optional addition, not enabled by default.

ukkopahis commented 2 years ago

PR cleaned up to only include changes regarding the dependency. Other important Telegraf changes split into their own pull-requests.