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

WIP: Debug overlay #1315

Draft
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

LukasKalbertodt
Copy link
Member

Inspired by #1314 and a recent example where I caught another n+1 problem in a PR review, I think we (devs) need some way to quiclky catch those problems. I found the one in #1314 fairly randomly and it could have stayed undiscovered for quite some more time. It's just an easy mistake to make. Is the n+1 problem really that bad? Well, it's not the worst but having this in the code base is dangerous as it can lead to unexpected slowness. And then you usually notice it in production at an inconvenient time. That would suck.

I also think it would be useful to more clearly see the duration of a request to notice when there are slow SQL queries for example. This information is already logged in trace level, but we usually don't run with that level during development and even then, it's easy to overlook it in the logs.

So I started this small experiment of adding a debug overlay so that we devs cannot escape that information. There are many open questions and todos:

  • Is this information useful enough to warrant this distracting overlay?
  • We devs probably need a way to disable it whenever we do design-heavy work?
  • This should clearly only be enabled for devs: how do we do that? When experimental features are active? When hostname is localhost? When built in debug mode? We might want to see this on test deployments as well?
  • Is there other interesting stuff for a debug overlay?
  • TODO: need to adjust the colors to work with light mode. Didn't want to put work into this before we know if/how we want this.
  • Is this information security sensitive, i.e. should the backend not send it out in production environments?

@LukasKalbertodt LukasKalbertodt added the changelog:dev Changes primarily for developers label Jan 15, 2025
@github-actions github-actions bot added the status:conflicts This PR has conflicts that need to be resolved label Jan 24, 2025
Copy link

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

paddingLeft: 8,
fontSize: 12,
background: COLORS.neutral05,
display: "none",
Copy link
Member

Choose a reason for hiding this comment

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

This is also hiding the info with ms and number of queries, is that intended?

@owi92
Copy link
Member

owi92 commented Feb 11, 2025

Regarding your questions:

  • Is this information useful enough to warrant this distracting overlay?

I like this and think it can be quite useful and not too distracting.

  • We devs probably need a way to disable it whenever we do design-heavy work?

Agreed, this would also help with the (already pretty small) distracting factor.

  • This should clearly only be enabled for devs: how do we do that? When experimental features are active? When hostname is localhost? When built in debug mode? We might want to see this on test deployments as well?

I would probably not tie this to hostname or debug mode or even experimental features (might be annoying when you want to test another experimental feature). So I think it has to be a new config option :/

  • Is there other interesting stuff for a debug overlay?

I can think of a couple of things, but they're all already logged in console. Limiting this to the ones you have now also helps to keep it only mildly distracting at worst. I guess it'd be nice to see which query these bars refer to, but that's too much information to fit into this overlay.

  • TODO: need to adjust the colors to work with light mode. Didn't want to put work into this before we know if/how we want this.

Also adjust colors in dark mode please. While I'm 100% sure it is unintended, this Bildschirmfoto 2025-02-11 um 17 50 06 is giving me too many "Flag of Germany" vibes

  • Is this information security sensitive, i.e. should the backend not send it out in production environments?

In its currect state, I really don't think so. If there's any doubt on your side, maaaybe you could restrict it (but I wouldn't) 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:dev Changes primarily for developers status:conflicts This PR has conflicts that need to be resolved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants