Tracy: universal query panel

5 years ago

Mikulas Dite
Member | 756
+
+8
-

Problem

Larger applications might depend on multiple services (relational db, graph db, fulltext search and key value, …). The current practice is each extension implements its own Tracy panel. This RFC argues for one universal panel for generic queries.

Mostly, extension developers now just copy and paste the ndb panel and call it a day.

Solution


Tracy panel which could hold arbitrary events. Events should include target backend (psql vs elasticsearch etc), running time and query and custom info (sql explain, full response, etc.).

Each extension would only implement Event and made sure each onQuery adds new Event to the panel.

Also, showing all queries in one list makes for easier orientation and allows nifty features such as timelines.

(proof of concept implementation https://github.com/…anel-queries)

Issues

Well, that's where you come in. I don't really see a problem here.

Last edited by Mikulas Dite (2014-05-06 14:35)

5 years ago

MartinitCZ
Member | 590
+
0
-

This is good idea. I vote +1.

May, there can be problem in colors. I suggest add auto detection or something like that. (Now two programmers can use same color.)

BTW: I think that there is one issue. It is not compatible with php 5.3 ;)

Last edited by martinit (2014-05-06 15:48)

5 years ago

Filip Procházka
Moderator | 4693
+
0
-

I love it. Are you ready to prepare pullrequest to Tracy (compatible with 5.3)?

5 years ago

Milo
Nette Core | 1149
+
0
-

Personally, I'm using 2 “query” panels at max in my applications, so nothing must-have for me. But I like the idea.

@Mikulas Dite Do you have some demo or saved static HTML page with Tracy? I'm interested how it looks like in real.

5 years ago

enumag
Member | 2128
+
+1
-

The idea is nice but I often connect to multiple databases from one application so I need to distinguish which query is for which database as well. From the screenshots I can't be sure if this is implemented already or not.

5 years ago

Mikulas Dite
Member | 756
+
0
-

@martinit:

Right, that repo is just a proof of concept, the final implementation should be compatible.

@Milo:

Static demo http://dite.pro/tmp/sample.html

@enumag:

Right, that is not implemented in the PoC version. Do you think it would suffice to differentiate it as a separate storage, such as:

5 years ago

enumag
Member | 2128
+
0
-

@Mikulas Dite: Yeah, I think that would be ok. :-)

5 years ago

looky
Member | 100
+
0
-

This is really nice.

I would implement Query as an interface instead of an abstract class full of abstract public methods.

5 years ago

Filip Procházka
Moderator | 4693
+
0
-

@looky good point. Abstract clases are used when you actually wanna add some common functionality to the base class, but here is none. So interface would be probably better.

Idea: the colors could be autogenerated. I wouldn't wanna have to solve that two databases have the same color, or when you add one driver twice.

5 years ago

mishak
Member | 100
+
0
-

Would be hard to use this prototype with some heuristic/recognition at onQuery along with new interface? This way it could be drop-in replacement.

Otherwise +1 :)

5 years ago

bazo
Member | 625
+
0
-

it's nice. if you have a lot of queries from different sources maybe a filter for source would be useful…
and maybe add an option to show times and number of queries separately in the panel, for quick glance?

5 years ago

enumag
Member | 2128
+
0
-

@bazo: Nice ideas. The first table with storages should also show total count of queries for each storage. Also when clicked it could filter the table below.

5 years ago

Mikulas Dite
Member | 756
+
0
-

New demo version (everything pseudorandom; info and result is a simple dump for simlicity but can be arbitrary html):

http://65202.w2.wedos.ws/…-panel/demo/

Implementation:

https://github.com/…anel-queries

This is still a work in progress, but not just a proof of concept like the original repo. Per-storage filtering will be implemented later on. Polishing and refactoring will come later on as well.

I ended up thinking this should not be in Tracy directly.

Last edited by Mikulas Dite (2014-05-18 23:26)

5 years ago

Filip Procházka
Moderator | 4693
+
0
-

Great work! I looked at the colors for a bit.. they seem a lot repetitive. I think you should have prepared a stack of colors, or some sequence that you'll “shift” from when adding panel.

If it won't be in Nette directly, you'll have a hard time standardizing it.

5 years ago

Mikulas Dite
Member | 756
+
0
-

I've updated the panel (repo, demo):

  • removed color coding as the query type is obvious or can be inferred from highlight
  • added filtering
  • removed row count column as the info is in results dump
  • refactored js
  • removed fixed header, fixed overflow and

The color coding was a bit too much. The panel seems more slick without it now.

Could you please skim through the code and do a code review? As this would hopefully end up as a new repo, there won't be a pull request with this to do it on.

Also, should actual IQuery implementations be in this repo? I added a few without including them in autoload just for illustration. Or should the Query be in each extension (ndb, dibi, elasticsearch, …)?

Filip Procházka wrote:

If it won't be in Nette directly, you'll have a hard time standardizing it.

Right, I meant that it should should be separate repository, not directly in nette/tracy repo. It could be in nette/nette dependencies. Any thoughts about this?

Last edited by Mikulas Dite (2014-05-19 17:07)

5 years ago

hrach
Member | 1817
+
0
-

Please remove also coloring the time. It's just your opinion what is fast and slow…

5 years ago

Mikulas Dite
Member | 756
+
0
-

The time coloring is done dynamically, fastest query is green while slowest is red. That is not based on opinion. It makes finding bottleneck easier. I might remove it if most don't like it, but unlike storage type color coding it actually provides new information.

Last edited by Mikulas Dite (2014-05-19 17:45)

5 years ago

Filip Procházka
Moderator | 4693
+
0
-

Also, should actual IQuery implementations be in this repo? I added a few without including them in autoload just for illustration. Or should the Query be in each extension (ndb, dibi, elasticsearch, …)?

Actual implementations belong to concrete bridges (next to dibi panel, nette\database panel, doctrine panel, …)

Right, I meant that it should should be separate repository, not directly in nette/tracy repo. It could be in nette/nette dependencies. Any thoughts about this?

I'm not sure about that :) I would like to hear opinion from @dg on this one.

The time coloring is done dynamically, fastest query is green while slowest is red. That is not based on opinion.

I love it :)

5 years ago

hrach
Member | 1817
+
0
-

Hovadina, opakovaný rychlý dotaz je 1000× horší než jeden s 50ms. Prostě false positive detekce je k ničemu.

edit: Sorry for czech…

Bullshit, a repeated fast quiery is 1000× worse than one query with 50ms time. Too many false positive detection is quite useless.


I prefer readability over this color AI.

Last edited by hrach (2014-05-19 21:06)

5 years ago

Filip Procházka
Moderator | 4693
+
0
-

Well, good point…

5 years ago

Mikulas Dite
Member | 756
+
0
-

Fair enough, that greyish color in middle of the color range is not very legible. And removing code is awesome, so…

Anyway, what is the next step on this?

5 years ago

Filip Procházka
Moderator | 4693
+
0
-

Next step would be response from @dg.

5 years ago

hrach
Member | 1817
+
0
-

dg ping ping

5 years ago

David Grudl
Nette Core | 6864
+
0
-

What?

5 years ago

Filip Procházka
Moderator | 4693
+
0
-

David Grudl any thoughts?

5 years ago

David Grudl
Nette Core | 6864
+
0
-

What is the question?

5 years ago

hrach
Member | 1817
+
0
-

We would like have some universal interface and panel for db queries. Do you agree to include this into tracy/tracy?

5 years ago

enumag
Member | 2128
+
0
-

@dg: Other option is to have it in a separate package like tracy/query-panel as suggested by @Mikulas Dite.

Last edited by enumag (2014-06-12 20:55)

5 years ago

David Grudl
Nette Core | 6864
+
0
-

Do you agree to include this into tracy/tracy?

Whether or not to include interface into Tracy, that is not used by Tracy? No, of course.

5 years ago

Filip Procházka
Moderator | 4693
+
0
-

David Grudl would you please read the proposal and post a relevant answer? This is not about some interface, and it doesn't have to be about implementation.

The main question for you is if you think that having 5 databases where each one has it's own debug bar panel is an issue. I'm not asking if having 5 databases is an issue, but rather if having 5 panels if there could be one is an issue. After we know your opinion on this, there might be an additional question – do you think the solution is good? After that is another – what do you think about the implementation.

Do you understand now what we need from you?

Last edited by Filip Procházka (2014-06-13 12:49)

5 years ago

enumag
Member | 2128
+
0
-

ping @DavidGrudl

5 years ago

hrach
Member | 1817
+
0
-

@DavidGrudl ping ping ping again.

In my opinion, this interface should be definitely included in tracy itself. the final form should be discussed.

5 years ago

Filip Procházka
Moderator | 4693
+
0
-

I've had a face-to-face talk with @DavidGrudl few months ago, and he had very good reasons not to include this in Tracy. You can safely assume that this won't make it into tracy, but it might be a nice extension.

Last edited by Filip Procházka (2014-12-29 23:55)

5 years ago

David Grudl
Nette Core | 6864
+
0
-

if you think that having 5 databases where each one has it's own debug bar panel is an issue.

I don't know. I am using a small number of databases and when each of them has own panel, it is advantage for me.

do you think the solution is good?

Yes, it is nice.

what do you think about the implementation.

Looks good.

Do you agree to include this into tracy/tracy?

No :-) Why? It's Mikulas's extension, he should manage and develop it.

5 years ago

enumag
Member | 2128
+
0
-

@MikulasDite What is the status of this solution? Do you think we should try it for Kdyby/Doctrine and Kdyby/Redis for example?

5 years ago

Mikulas Dite
Member | 756
+
0
-

I still use the PoC I created a year ago and there are too many issues with that implementation for me to just release it. On the other hand it works fine for me and there is not enough demand to warrant all the work. So I'm torn.

It's in my top like 5 project ideas I would like to realise one day, but no promises on that front. If anybody wants to reimplement this, I'm totally encouraging that.

5 years ago

enumag
Member | 2128
+
0
-

Can you elaborate the issues of the current solution that you're aware of? I can only se one on GH.

5 years ago

Mikulas Dite
Member | 756
+
+1
-

Looked it up just now, it's not as bad as I thought. The main issue is creating the query handlers and hooking into the respective event systems, which is a mess now.

I guess I'll polish the panel itself and talk to @hrach about publishing it under Nextras. The queries will be in separate repo or something.

5 years ago

Mikulas Dite
Member | 756
+
0
-

Ok, na zkoušku jsem něco nahodil na github https://github.com/…-query-panel.

3 years ago

Tomáš Jacík
Member | 142
+
0
-

I work with more than one database in almost every application, so I love this idea.

@DavidGrudl It this won't be directly in Tracy, people does not bother implementing it in extensions. Also I'd be very glad if Nette\Database would use this too.