Callback for tracing sql queries

[smacker]

To solve #1281 I came up with this solution...

[jinzhu]

Hi @smacker

Thank you for your PR, sorry just noticed your issue report, I think you could just make it works with set a new logger, so this might be not necessary.

Refer: https://github.com/jinzhu/gorm/blob/master/logger.go

Thank you.

[smacker]

No, it won't work with a logger. I have to start newrelic transaction before sql started execution and finish it after. It measures not only time, but call stack, how much cpu it burn and other things.

[smacker]

please reconsider trace callback. Code instrumentation is very useful. If you don't like callback, can you suggest any other way to do it? For example go-redis provides WrapProcess method: redis/go-redis#352

[zardak]

IMO, this is a sign of hacky approach. The more or less right way would be to introduce lifecycle hooks, like BeforeX and AfterX callbacks.

[smacker]

agreed. I'll try to improve it.

[ipadavic]

I'm trying to add sql tracing to my app via datadog apm, this PR makes it possible. Any chance this could be merged?

[zardak]

@jinzhu could you take a look at the pr please?

[jinzhu]

Hi @smacker

Sorry for the delay, just have time to review it and your code in https://github.com/smacker/newrelic-context/blob/master/gorm.go

I think for your problem you could resolve it with RawQuery callback, and it will be call in Row, Rows, Count... Refer https://github.com/jinzhu/gorm/blob/master/callback.go#L78

cc @ipadavic @zardak

[smacker]

@jinzhu it was my first intention to use raw callback, but you call it before sql.DB methods, there is no way to know when it finish. For Query, Create, Delete and Update you call sql.DB inside callbacks, so I can add my callback before and after real query. But for RawQuery you do sql query inside scope, not callback. I also tried to change Row, Rows, Count, ... methods and move sql related code from scope to inside callbacks, but code becomes quite messy.

[jinzhu]

Hi @smacker

Thank you for your report, just fixed that problem. jinzhu@c62e9bc

[smacker]

@jinzhu your commit broke the code of anybody who altered the query in row callbacks. (your new callback registered before any existed one). It was the issue that I mentioned in my original question #1281 JFK

[jinzhu]

Yes, you are right, just pushed another commit to fix the behaviour with a warning message jinzhu@7fb9b62

Thank you for the report.

[ipadavic]

Sorry for commenting on closed PR, but I'm having trouble getting this to work with Raw(). Does "row_query" callback include Raw queries?

[jinzhu]

No, Raw query is really raw, no callback included.

Only Row, Rows, Count, Plack... those methods include the callback.