Tweak API history endpoint and fix cached counting

[DL6ER]

What does this implement/fix?

Two improvements for the main activity, and one bug fix for the clients activity graphs API endpoints:

• Include forwarded queries in /api/history
• Count stale-cache queries as cached queries in /api/history
• Fix duplicated clients in /api/history/clients

[!NOTE] Needs https://github.com/pi-hole/web/pull/2956 to be checked out

---

Related issue or feature (if applicable): https://discourse.pi-hole.net/t/dashboard-graphs-dont-match/68263

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:

• [x] The code change is tested and works locally.
• [x] I based my code and PRs against the repositories developmental branch.
• [x] I signed off all commits. Pi-hole enforces the DCO for all contributions
• [x] I signed all my commits. Pi-hole requires signatures to verify authorship
• [x] 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/dashboard-graphs-dont-match/68263/6

[DL6ER]

Has been confirmed working on the related Discourse thread

[sawsanders]

While the initial double counting is fixed, I noticed that the client graphs show clients with zero queries in some time periods while still having many queries reported in the "other" category. I only saw this after I set the number of max clients back to 10 from 100.

IMHO, it shouldn't show zero query clients when it has clients in the "other" category to use instead.

And please forgive me is this is posted in the wrong area. :)

[DL6ER]

The problem we are facing here is limited number of distinct (and also distinguishable as in "by humans") colors. If we'd starting showing different clients the way your suggest (which is surely meaningful in some way), we'd quickly reach 20 or more colors even if the limit is set to 10 - or even less.

It would mean that the color palette starts repeating at some point and - in my personal opinion - if the red client is the printer in the morning, and then some random smart home devices the rest of the day, this would possibly cause much more confusion that it'd be useful in the end.

What do you think?

[sawsanders]

The problem we are facing here is limited number of distinct (and also distinguishable as in "by humans") colors. If we'd starting showing different clients the way your suggest (which is surely meaningful in some way), we'd quickly reach 20 or more colors even if the limit is set to 10 - or even less.

It would mean that the color palette starts repeating at some point and - in my personal opinion - if the red client is the printer in the morning, and then some random smart home devices the rest of the day, this would possibly cause much more confusion that it'd be useful in the end.

What do you think?

I see your point and it all really comes down to a matter of preference.

I'd rather see the true top 10 (or whatever is configured) when I hover over the bars; it allows me a quick glance on client activity over that period. Sometimes the "others" category squishes down the lower frequency ones, making them hard to discern the colors.

You (and others I'm sure) feel that consistent colors are the way to go. That's fine too and I'm ok with it. It's hard to make everyone happy.

[DL6ER]

As you know: Pi-hole always tries to make the users happy (if the request is reasonable). I thought: How difficult could it be and came to the conclusion: Well, it wasn't as simple as I first assumed but after an hour of work its done.

The most recent commit adds a new config option webserver.api.client_history_global_max controlling if the client's activities chart should sort and show the global (integrated over 24 hours) or the local (measured individually in each time slot) most active clients.

This new option defaults to true (my interpretation) but if you set to to false you should get what you are looking for. This required me to redefine the data format returned by this API endpoint because returning sparse-data (not every of the clients shown is also present in every time frame) was not possible so far. This is no issue in itself but it obviously makes this PR substantially larger (and hence more difficult to review). Whatever testing you could contribute would likely help the reviewing process a lot.

Note: The previous format was using a dense format which comes close to the most compact format possible. The new format needed to use keys to reference clients and, hence, has some overhead. To give an example: assume maxClients = 5, the previous format returned about 6 KB of data while the new is about 13 KB of data. Not a show stopper by any means (this endpoint is queried every 10 minutes) but I still thought a reviewer would like to know the size implications of the new response schema.

[sawsanders]

Wow. That was not expected, but thank you and I’ll do my best to put this through its paces.

[sawsanders]

So far so good. No show stoppers that I can tell yet.

The only downside I notice is that zooming and scrolling the graphs is much slower when webserver.api.client_history_global_max is set to false. It was never speedy for me before, but now it’s almost painfully slow. I rarely use that feature anyway.

[DL6ER]

now it’s almost painfully slow

It is dependent on mainly two factors (1) the device your browsers runs on and (2) the number of clients shown in total.

While (1) is obviously always the same and much worse on a phone than, e.g., on a modern laptop, (2) is strongly influences by the setting being either true (maximum 10 clients) or false (possibly hundreds).

There is nothing we could really do about it, it's Javascript driven and only UI-side.

So far so good. No show stoppers that I can tell yet.

Thank you very much for your support in testing this feature!

[sawsanders]

I noticed a one time mis-sorting of the "Top Permitted Domains" section of the dashboard.

There was one domain with 2 hits in third place on the list while the rest of the top 10 had over 1000. I went to make a screen shot, messed up, reloaded the page and the problem dissapeared after the page reloaded.

Edit: Just happened again but on both Top Permitted and Top Blocked:

[DL6ER]

@sawsanders Thanks! Fixed with the last commit.

[sawsanders]

18 hours and counting. Everything working as expected.

EDIT: 30+ hours later and things look great still.

[sawsanders]

Everything is still working as expected. I feel confident that the issue is solved with this PR.

Thank you!

[Gontier-Julien]

Tested it and it working as expected too after a 4 days 👍🏻

[DL6ER]

Waiting for https://github.com/pi-hole/web/pull/2956 to be approved before merging this one