search

tony2001 / pinba_extension

Pinba PHP extension
http://pinba.org
GNU Lesser General Public License v2.1
83 stars 22 forks source link

Is the protocol used by Pinba still compatible with ProtoBuf? #13

Closed a-square closed 10 years ago

a-square commented 10 years ago

There are apparently arrays in the Pinba Request structure that are not in its *.proto file:

request->timer_ru_stime = malloc(sizeof(float) * n);
request->timer_ru_utime = malloc(sizeof(float) * n);

This is disturbing. I'm working on porting the Pinba client to C++ (as a library), and seeing this effectively means I can't recompile the *.proto file to obtain an easy-to-use C++ code.

Are there custom modifications to ProtoBuf-generated code that people should be aware of? Is there hope of seeing the *.proto file retrofitted so that it's useful again?

tony2001 commented 10 years ago

What do you mean not in .proto file? https://github.com/tony2001/pinba_engine/commit/3a8d3838a3d2a2498ffb4bfa93ce0b6db903c416 Of course they are there, but they're not in master yet.

tony2001 commented 10 years ago

Btw, if you take a look at older version of PHP extension, you'll find that it used to be using original Protobuf lib, which is in C++. Hope this helps you. And don't hesitate to ask me if you have any questions.

a-square commented 10 years ago

But the code that I quoted is in master already, e.g. look at pinba.c#L560

tony2001 commented 10 years ago

Right, there's no devel branch for the extension. Adding new fields to the extension doesn't break the server - they are just ignored.

a-square commented 10 years ago

But then shouldn't you update this file? https://github.com/tony2001/pinba_extension/blob/master/pinba.proto

tony2001 commented 10 years ago

I'm going to merge devel to master eventually.