Limit number of clients returned by /api/history/clients

[DL6ER]

What does this implement/fix?

Limit number of clients returned by the /api/history/clients endpoint to 20 (default value).
This can be overwritten at compile-time and at run-time using the query parameter ?N=...

---

Related issue or feature (if applicable): N/A

Pull request in docs with documentation (if applicable): N/A

---

By submitting this pull request, I confirm the following:

1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
2. I have commented my proposed changes within the code.
3. I am willing to help maintain this change if there are issues with it later.
4. It is compatible with the EUPL 1.2 license
5. I have squashed any insignificant commits. (git rebase)

Checklist:

• The code change is tested and works locally.
• I based my code and PRs against the repositories developmental branch.
• I signed off all commits. Pi-hole enforces the DCO for all contributions
• I signed all my commits. Pi-hole requires signatures to verify authorship
• I have read the above and my PR is ready for review.

[pralor-bot]

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pi-hole-for-a-lot-of-ipv6-clients/66987/7

[yubiuser]

Could you implement a "all-other-clients" client with the sum of all their queries for every client exceeding N? Otherwise the two dashboard graphs would not match in terms to queries/timeslot

[DL6ER]

Thanks for the suggestion. I added this and also reduced the default N down to 10 as I had 18 devices in my network over night and the graph already rendered pretty useless. Now with the "other clients" entry compensating for this, it seemed a wise choice.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved.

[yubiuser]

Works as expected, but I have a wish and one point for discussion.

1. Can we make this a config option -> users can set whatever N they want for their web interface to be displayed

2. Now we limit to the most active overall clients. However, the issue we want to solve is: to many clients per time slot (rendering the graph useless). Imagine a situation where we have only 10 clients in the morning and 10 different clients in the afternoon (each within one time slot). Despite both not exceeding N on per-timeslot basis, 10 of them will be removed. How about (I know, much more code) to limit the number of clients returned on per-timeslot basis with a lower N (e.g. 3-5)

[DL6ER]

Sorry, I missed your comment before.

Can we make this a config option -> users can set whatever N they want for their web interface to be displayed

Yes, I did just add this.

How about (I know, much more code) to limit the number of clients returned on per-timeslot basis with a lower N (e.g. 3-5)

I think this will reintroduce part of the problem - even when N is further reduces. We have 240 timeslots in 24 hours, even with N = 3, this is close to 1000. But even when this is surely worst-case thinking, I guess we will get massive confusion with repeated colors as some of your morning clients (smart coffee maker) may be red until 10AM, then they are replaced by some other component (solar system reporting tool) which got the same red color because if limited colors available.

[yubiuser]

I think this will reintroduce part of the problem - even when N is further reduces. We have 240 timeslots in 24 hours, even with N = 3, this is close to 1000. But even when this is surely worst-case thinking, I guess we will get massive confusion with repeated colors as some of your morning clients (smart coffee maker) may be red until 10AM, then they are replaced by some other component (solar system reporting tool) which got the same red color because if limited colors available.

I see your valid point. It's a tricky decision. The current implementation can lead to situations where "other clients" is the biggest/only client within a certain timeslot, making it a bit sensless. However, this should not stop this PR. Depending on use feedback we could change it later.

[yubiuser]

Small nit pick: the order in which objects in the json are returned is clients first, and history second. In the documentation it's the other way round.