search

yiisoft / yii2-debug

Debug Extension for Yii 2
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
202 stars 149 forks source link

Don't use table-danger to indicate excessive queries #503

Closed steve962 closed 1 year ago

steve962 commented 1 year ago

What steps will reproduce the problem?

Run a page with enough database queries to be considered "excessive" queries by the new code in 2.1.23

What's expected?

Should NOT be indicating the request failed with table-danger

What do you get instead?

Is indicating the request failed somewhere along the line by marking it with table-danger

Additional info

While it's nice to have an indicator of excessive queries, these requests aren't actually failing -- this at most is a warning, and not even that - it's a suggestion to optimize your database queries. When I see a page request in the debugger marked with the table-danger style, my inclination is to go looking into that page's log in detail looking for why the request failed to complete, and if I'm debugging a complex application where some requests are succeeding and others failing, the failed requests get lost in the sea of requests marked as failed because of excessive queries.

I recommend not putting a style on it and simply leaving the little exclamation icon next to the query count as the only indicator -- that's more than sufficient to indicate that optimization ought to be considered, without burying real errors in the noise.

samdark commented 1 year ago

Indeed, that would be better UX-wise. ⚠️ is the way to go.

samdark commented 1 year ago

@rhertogh what do you think?

rhertogh commented 1 year ago

@samdark I agree. Initially I implemented it as a warning, but the criticalQueryThreshold was implemented as an error, so I used that.

So yes, I also think a warning would be better, but for consistency I would then also display an exceeding of the criticalQueryThreshold as a warning.

I've create a PR for both criticalQueryThreshold and excessiveCallerThreshold: #504