search

prometheus-community / node-exporter-textfile-collector-scripts

Scripts for node-exporter's textfile collector
Apache License 2.0
488 stars 185 forks source link

`smartmon.sh` vs. `smartmon.py` #216

Closed calestyo closed 2 months ago

calestyo commented 3 months ago

Hey.

Just wondered whether there will ever be one main implementation and the other dropped?

On the long term, it doesn't seem the best two keep two implementations of varying feature levels, people will be confused which to choose.
E.g. Debian's packaging chooses the shell version, but at least for my storage servers (which use all kinds of RAID controllers), the shell version seems to return far less data than the Python version.

Cheers, Chris.

dswarbrick commented 3 months ago

There are a few open issues about missing / differing functionality between the two versions, and there have been some proposals already (probably mostly mine) to drop the smartmon.sh script.

I think that even on resource-constrained systems, the Python-based script is not going to be that demanding, and it unlocks so much more functionality for script developers / maintainers, e.g. especially when parsing a tool's JSON output.

calestyo commented 3 months ago

I see. Then I'll make some PRs for the Debian packaging to replace the shell version with the Python one (or at least allow it to be configurable), if I find some time.

calestyo commented 3 months ago

One more thing. I'm about to start writing an ssacli counterpart to the storcli.py... and would want to get that merged here into node-exporter-textfile-collector-scripts.

Is it okay for you guys to depend on python3-prometheus-client?

dswarbrick commented 3 months ago

Is it okay for you guys to depend on python3-prometheus-client?

If you're referring to this repo, several of the Python scripts already import prometheus_client. It was decided a while ago to forbid homegrown implementations of the text exposition format rendering, and thus require the use of the official prometheus_client library.

If you're referring to the Debian prometheus-node-exporter-collectors package, it already depends on python3-prometheus-client - for the reason stated above.

calestyo commented 3 months ago

I mostly meant this repo here.

Just realised you're the DD ... I've tagged you on some PR for salsa.
Would like to hear your opinion on that.

Once the stuff there is dealt with, I'd make some further PR for at least the Debian package, that make the smartmon backend (more easily) configurable, as long as the shell version is still around.

calestyo commented 2 months ago

Hey.

Other than in the case that this issue shall become the one that tracks replacing the shell with the python implementation, I'd say that all questions have been answered here (thanks :-) btw).

Thus closing.

Cheers, Chris.