Skip to content

Conversation

@zspitzer
Copy link
Member

combine sql queries into a single table
add toggle to display sql source
highlight slow queries, expand slow queries by default, hide sql for all other queries
add a couple of missing var statements

BTW I'd love to add query resultset size to the debug output, it would be really useful for monitoring memory usage

combine sql queries into a single table
add toggle to display sql source
highlight slow queries, expand slow queries by default, hide sql for all other queries
add a couple of missing var statements
@zspitzer
Copy link
Member Author

regarding the earlier question about persisting expanded state, there isn't enough information to identify individual queries and the reference would need to include the page url, so even if we could persist a reliable reference, it would potentially be a rather large cookie

Copy link
Contributor

@isapir isapir left a comment

Choose a reason for hiding this comment

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

What's up with the empty anchor tag?
<a>#queries.name#</a>

If we can't save the toggle the state then IMO we should not collapse certain queries by default.

Did you try to add a Memory size of the Queries similar to the way we do for the Scopes? This is how we show the estimated size of the Scopes:
https://github.com/lucee/Lucee/blob/5.3/core/src/main/java/resource/context/admin/debug/Modern.cfc#L743

@zspitzer
Copy link
Member Author

the empty anchor tag on the query name is to provide a visual indicator that you can click to expand the results to see the sql, but the actual onclick handler is for the whole row.

the query object doesn't include a reference to the actual query, it only contains the following columns,
name | time | sql | src | count | datasource | usage | cacheType

Something needs to be done in java to add size info

Regarding persistence, how about I just add a persistent expand all sql cookie?

@isapir
Copy link
Contributor

isapir commented Sep 25, 2017

Regarding persistence, how about I just add a persistent expand all sql cookie?

I really don't see the benefit in that since the whole section can be toggled. I think that adding some visual signal for the slow queries is a good idea. Only if you have many queries in the Request does it make sense to go through the trouble of collapsing/expanding.

@isapir
Copy link
Contributor

isapir commented Sep 25, 2017

the query object doesn't include a reference to the actual query, it only contains the following columns,
name | time | sql | src | count | datasource | usage | cacheType

You can open a new ticket requesting to add the Query memory footprint to the debug template.

@zspitzer
Copy link
Member Author

done! https://luceeserver.atlassian.net/browse/LDEV-1517

@zspitzer
Copy link
Member Author

I have added a persistent toggle to expand all sql queries

Even if you only have a few queries in a request, the sql can be rather long and span many pages,
this makes the debug output much more concise by default, whilst still preserving the old verbose behavior

@zspitzer
Copy link
Member Author

I also have an implementation of Server timing headers which i could add to this pull request
https://luceeserver.atlassian.net/browse/LDEV-1520

- add support for server timing headers
- change the column order for execution times
Order	Template	Function	Total Time (ms)	Count	Avg Time (ms)	Query
- show function name in separate column for execution time
@micstriit
Copy link
Contributor

this need to be manually merged with the current Modern.cfc in 5.3 that cahnaged a lot.

@zspitzer
Copy link
Member Author

ok, i'll submit a new PR against 5.3

@zspitzer zspitzer closed this Apr 21, 2018
@cfmitrah
Copy link
Member

Hi @zspitzer,

I've added server timings into latest 5.3 branch. Can you please check it

Pull Request: #468

@zspitzer
Copy link
Member Author

would it be ok to refactor modern.cfc a bit, i.e moving each debug section into separate functions?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants