search

frdmn / docker-speedtest-grafana

🚢📦Grafana + InfluxDB + Speedtest.net results
MIT License
206 stars 71 forks source link

Switch to official speedtest-cli #4

Closed therealshark closed 4 years ago

therealshark commented 4 years ago

This PR switches from speed-test to the official sppedtest-cli. Sadly the speed-test package produces unreliable results, see: https://github.com/sindresorhus/speed-test/issues/11. In my case this had the effect, that speed-test would report a speed of 0.1 Mbps instead of ~200Mbps rendering the dashboard useless.

This PR also moves the logic from a shell script to nodejs. Its not strictly necessary since the cli isn't actually node based but this feels somehow cleaner to me.

PS: The speedtest image is quite big, but i'm not exactly sure what to do about it. One way could be to download the deb directly without adding the repo but then we would need to implement logic to select the right architecture into the Dockerfile which feels like a hack to me. If you got some ideas, i'm ready for suggestions :)

therealshark commented 4 years ago

PPS: This might also fix https://github.com/frdmn/docker-speedtest/issues/3

karloskalcium commented 4 years ago

Am using this patch and can confirm it works well. I made some additional enhancements in my fork to allow multiple machines to report in and track additional parameters: https://github.com/karlwbrown/docker-speedtest

frdmn commented 4 years ago

@karlwbrown I might have cherry-picked the following commits from your repo into this one: https://github.com/frdmn/docker-speedtest/compare/9a786f6..021eb2e

Hope you don't mind 😊

karlbrown-va commented 4 years ago

hi, no I was planning on making a PR at some point so no worries.

A note however - I have noticed that the new javascript implementation has a hanging issue, especially on OSX laptops. the delay function seems to hang, I don't yet know why. Just a heads up, I've experienced this on several machines already. @therealshark may have some insights into this.

therealshark commented 4 years ago

@karlbrown-va I'm not entirely sure but i think the problem is not delay but the speedtest-cli. I had the problem once too where the cli would not terminate at all. I think it makes sense to add a timeout at the execa call to fix this :)

To clarify: delay is just a wrapper around setTimeout and should not behave differently on different systems.