Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement support for displaying exact type instead of just "OTHER" #1013

Merged
merged 1 commit into from
Jan 15, 2021

Conversation

DL6ER
Copy link
Member

@DL6ER DL6ER commented Jan 3, 2021

By submitting this pull request, I confirm the following:

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

How familiar are you with the codebase?:

10


Implement support for displaying exact type instead of the catch-them-all category OTHER. The OTHER category is still used when it comes to computing statistics to ensure your chart's legend does not explode.

Example:

dig -t TYPE65 google.de
dig -t TYPE66 google.de

results in:
Screenshot from 2021-01-03 21-52-16
(note that type 65 is correctly translated into the named type HTTPS rather than showing TYPE65)

And the database contains the expected (16 = HTTPS mapped status, 166 = TYPE66 with offset 100)

SELECT * FROM queries WHERE domain = 'google.de' ORDER BY id DESC LIMIT 2;
id          timestamp   type        status      domain      client      forward         additional_info
----------  ----------  ----------  ----------  ----------  ----------  --------------  ---------------
10910413    1609706951  166         2           google.de   127.0.0.1   127.0.0.1#5353                 
10910412    1609706943  16          2           google.de   127.0.0.1   127.0.0.1#5353 

No changed to core of AdminLTE are necessary for this to work. A small change to the long-term queries page will be needed to handle these offset types still as OTHER.

…-all category OTHER. The OTHER category is still used when it comes to computing statistics to ensure your chart's legend does not explode.

Signed-off-by: DL6ER <dl6er@dl6er.de>
Copy link
Member

@PromoFaux PromoFaux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reluctantly approving this. It feels dirty, but having exhausted all other options - it is indeed the most efficient way forward.

@DL6ER DL6ER merged commit 00492c1 into development Jan 15, 2021
@DL6ER DL6ER deleted the new/OTHER_types branch January 15, 2021 17:23
@DL6ER DL6ER mentioned this pull request Jan 15, 2021
@pralor-bot
Copy link

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

https://discourse.pi-hole.net/t/pi-hole-core-v5-2-3-web-v5-3-and-ftl-v5-4-released/43009/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement PR: Approval Required Open Pull Request, needs approval
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants