search

pi-hole / web

Pi-hole Dashboard for stats and more
https://pi-hole.net
Other
2.05k stars 559 forks source link

Query log shows less than 100 entries when only blocked/permitted queries are shown #2535

Closed yubiuser closed 5 months ago

yubiuser commented 1 year ago

Actual behavior / bug

When users choose to only show permitted or blocked queries (Settings/Web Interface) the number of domains shown on the query log can be less then 100 domains depending on the number of non-shown item. In the worst case (e.g. only show blocked queries) where the last 100 queries where of the non-shown type (e.g. permitted) the query log is empty.

Reported here: https://discourse.pi-hole.net/t/pihole-query-log-recent-queries-wird-automatisch-geleert

Expected behavior

Query log should always show 100 entries of the desired type (blocked/permitted/both).

pralor-bot commented 1 year ago

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

https://discourse.pi-hole.net/t/pihole-query-log-recent-queries-wird-automatisch-geleert/61311/14

yubiuser commented 1 year ago

@DL6ER

Could you add two pseudo options in [status] in /api/queries for v6 (blocked, permitted) that will aggregate all applicable other status options (e.g. "blocked gravity" -> blocked, "blocked regex" -> blocked). That way we would not need to know all different status types (and forget one in the future) on the font end but could simply filter for blocked/permitted

DL6ER commented 1 year ago

I'll do this. My development Pi-hole is currently busy running a long-term test concerning a few database optimizations I'm trying out so I'll code this only afterwards (expect beginning of next week). If you don't hear anything from me, I may need a reminder....

DL6ER commented 1 year ago

@yubiuser After having checked this, I think we need to do this in Javascript. FTL v6 currently accepts a comma-separated list of integers for status. While FTL could add special interpretation for blocked and permitted as strings instead, the multi-selects on the query log page would could not be set by this else than that the Javascript itself knows which IDs belong to these special status strings. And if Javascript knows there is no need to duplicate this in FTL (it would actually be quite some code).

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Please comment or update this issue or it will be closed in 5 days.

pralor-bot commented 1 year ago

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

https://discourse.pi-hole.net/t/merging-ipv4-and-ipv6-dns-lookups-for-a-given-client/63267/23

pralor-bot commented 1 year ago

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

https://discourse.pi-hole.net/t/merging-ipv4-and-ipv6-dns-lookups-for-a-given-client/63267/25

dovecode commented 9 months ago

@yubiuser After having checked this, I think we need to do this in Javascript. FTL v6 currently accepts a comma-separated list of integers for status. While FTL could add special interpretation for blocked and permitted as strings instead, the multi-selects on the query log page would could not be set by this else than that the Javascript itself knows which IDs belong to these special status strings. And if Javascript knows there is no need to duplicate this in FTL (it would actually be quite some code).

Can this even be done reliably client-side? I'm not familiar with the v6 codebase, but in v5, the query log uses the FTL API's getAllQueries request which is designed to consider a fixed number of recent requests and then filters it, resulting in less than the requested number potentially being returned.

One way I can think of improving this would be to change the loop in https://github.com/pi-hole/FTL/blob/4f92b48ff27427ff6fc6e1e8a751d6cc6118111f/src/api/api.c#L882 to iterate in reverse order and having the loop invariant be "until I've collected X entries (or run out of recent queries)". This would of course require a different handling on how the data is returned, as it would need to be reversed, or some clever array logic employed.

I'd give it a stab, but I've not tried rebuilding FTL from sources, so am a bit hesitant to mess up my own Pihole...

PromoFaux commented 6 months ago

I'd give it a stab, but I've not tried rebuilding FTL from sources, so am a bit hesitant to mess up my own Pihole...

You could always build locally on a development machine, and then use the v6 docker container build process to try out that compiled binary by placing it in the src directory of the docker repo

DL6ER commented 6 months ago

This should be fixed in FTL v6.0 or I may misunderstand the issue. We keep iterating over database results until we reach the requested limit.

github-actions[bot] commented 5 months ago

This issue is stale because it has been open 30 days with no activity. Please comment or update this issue or it will be closed in 5 days.