Skip to content

Show names in top clients list from long-term data#2245

Merged
yubiuser merged 2 commits into
develfrom
db_toplist_names
Jul 4, 2022
Merged

Show names in top clients list from long-term data#2245
yubiuser merged 2 commits into
develfrom
db_toplist_names

Conversation

@yubiuser
Copy link
Copy Markdown
Member

@yubiuser yubiuser commented Jul 4, 2022

  • What does this PR aim to accomplish?:

Replaces the IPs in the Top Clients list (in long-term data) with names if applicable. Since pi-hole/FTL#1255 we store more data in the long-term database, e.g. the name of the client which made the query. We can use that to fetch the name when generating the top list.

Bildschirmfoto zu 2022-07-04 20-43-05

  • How does this PR accomplish the above?:

Re-uses the code from #2202 to get the name of the client if one is set.


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 and I have tested my changes.
  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)

  • I have read the above and my PR is ready for review. Check this box to confirm

Signed-off-by: Christian König <ckoenig@posteo.de>
@yubiuser yubiuser added the PR: Approval Required Open Pull Request, needs approval label Jul 4, 2022
@yubiuser yubiuser requested a review from a team July 4, 2022 18:53
@pralor-bot
Copy link
Copy Markdown

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

https://discourse.pi-hole.net/t/top-clients-long-term-data/56325/7

@Bucking-Horn
Copy link
Copy Markdown
Member

As this is about evaluating long term queries, does that cover for

  • a client of the same name showing up with different IPs over time?
  • the same IP being assigned or claimed by differently named hosts over time?

@rdwebdesign
Copy link
Copy Markdown
Member

rdwebdesign commented Jul 4, 2022

This is using client_by_id table.
(I'm not sure about the second item).

Comment thread api_db.php Outdated
Co-authored-by: RD WebDesign <github@rdwebdesign.com.br>

Signed-off-by: yubiuser <ckoenig@posteo.de>
@yubiuser
Copy link
Copy Markdown
Member Author

yubiuser commented Jul 4, 2022

a client of the same name showing up with different IPs over time?

Our reference is still the IP. If a client (long discussion what a client is) changes IP, this will create a new entry in the client_by_id table and therefore show up as different client in the top list.

the same IP being assigned or claimed by differently named hosts over time

Again this will add a new entry in the database and show up as different client in the top list. The same happens if the client changes its name.

If the same IP/name combination is used by a different "real" client this will still be attributed to the same database client.
https://github.com/pi-hole/FTL/blob/ed35b17fe795b10006b789b7c09159a239dcd0e9/src/database/query-table.c#L169

@yubiuser yubiuser merged commit 66e141c into devel Jul 4, 2022
@yubiuser yubiuser deleted the db_toplist_names branch July 4, 2022 21:43
@pralor-bot
Copy link
Copy Markdown

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

https://discourse.pi-hole.net/t/pi-hole-ftl-v5-16-web-v5-13-and-core-v5-11-1-released/56384/1

@pralor-bot
Copy link
Copy Markdown

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

https://discourse.pi-hole.net/t/resolve-client-ip-adresses-for-the-long-term-data-section/55655/2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: Approval Required Open Pull Request, needs approval

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants