Add database query summary metrics - #22
Conversation
7a50f9c to
4b7c8a1
Compare
niden
left a comment
There was a problem hiding this comment.
Thank you for this work. Some comments that I would like see addressed - some nitpicks but some design decisions.
Thanks again, this looks super!
|
Thanks for the review. I addressed all the requested changes in 2a2bcd6:
The relevant tests and static analysis checks pass locally. Could you please take another look? |
|
@Alistar84 Thank you for the quick turnaround. One note before I go for second pass, reminder https://docs.phalcon.io/5.18/ai-development/. If you need to change the commit messages go for it. |
|
@Alistar84 Also before I forget, this will need a few lines in docs/index because it is new behavior |
niden
left a comment
There was a problem hiding this comment.
A few more issues to resolve.
Also note the AI doc in my other document (main thread) and please make sure to update the PR description as well since it is a bit off after these changes.
Thanks
Assisted-by: Codex
Assisted-by: Codex
Assisted-by: Codex
2a2bcd6 to
d0a78b7
Compare
|
Thanks. The latest feedback has been addressed in d0a78b7:
The focused collector/renderer tests, PHPStan, code style, and collector coverage checks pass locally. |
niden
left a comment
There was a problem hiding this comment.
@Alistar84 Two more nitpics. This is nearly ready for merge.
Thanks again
| tr.appendChild(el('td', 'phalcon-debugbar-key', scalar(row.label))); | ||
| tr.appendChild(el('td', 'phalcon-debugbar-value', scalar(row.message))); | ||
| value.appendChild(el('span', 'phalcon-debugbar-message', scalar(row.message))); | ||
| if (occurrences > 1) { |
There was a problem hiding this comment.
There are two checks for occurences > 1. Could we make this one?
| border-radius: 999px; | ||
| background: #8a4b08; | ||
| color: #fff1d6; | ||
| font-size: 10px; |
There was a problem hiding this comment.
Nitpick but... Can we make this 11px since that is what we use everywhere else
Hello!
In raising this pull request, I confirm the following:
Small description of change:
Adds a structured summary to the Database panel with:
For example, executing the same normalized statement three times contributes two duplicate runs. Statement normalization collapses whitespace and deliberately ignores binding values, so repeated executions of one prepared statement are grouped together.
Each repeated row exposes its total
occurrencesas structured data. The frontend uses that value to apply a duplicate-row CSS class and displayExecuted N timeswithout changing the duration label. The summary remains visible with zero values when no queries are executed.The collector and renderer tests cover repeated statements with different bindings, unique statements, per-row occurrence counts, summary values, and the zero-query state. The behavior is also documented in
docs/index.md.Thanks