open-telemetry / opentelemetry-sqlcommenter

SQLCommenter components for various languages
Apache License 2.0
28 stars 13 forks source link

[Javascript] Consider underlying db package instead of ORM #9

Open weyert opened 3 years ago

weyert commented 3 years ago

I have been thinking about sqlcommenter. I was wondering whether it would make more sense to instrument the underlying packages used to talk to the databases instead of instrumenting the ORMs.

For example, if we would instrument the pg-package then it would for all ORM libraries available in Node.js e.g. it would support TypeORM without effort.

What does everyone think? Good idea?

sjs994 commented 3 years ago

Thanks @weyert , It seems like we can give it a try.

Although if i think more of it, It will be like a tradeoff between number of databases drivers and orm frameworks. Currently we are updating code for each framework. And it supports Postgres, MariaDB and MySQL. If we go via the driver approach, we have to instrument every driver.

We might need the orm information if user would want to correlate performance affected by any particular orm. Now, if we go via the driver path, can we get the information regarding orm from the driver ? or maybe we have to store the information regarding orm in some global variable and use it while instrumenting the Query.

weyert commented 3 years ago

@sjs994 Hmm, that's a good point regarding correlate performance. Didn't think of that. I was thinking we already have some of the node packages instrumented and thought maybe it's worth to integrate SQLcommenter support into those. E.g cassandra, pg, mysql etc. [1]

[1] https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/plugins/node

sjs994 commented 3 years ago

@weyert That would be great. Our eventual goal is to integrate the instrumentations in corresponding language repos if possible. But I think opentelemetry-specs SIG has to agree upon sqlcommenter specs before we start adding changes to https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/plugins/node.