opentelemetry support or hooks?

[JerrySievert]

hi,

wondering if you've considered either adding opentelemetry support directly, or adding hooks for pre/post query execution?

just looking for a good way to get telemetry into your awesome library - thanks!

[porsager]

Hi @JerrySievert .. Thank you for the kind words..

I think it's a great idea to add some hooks so this can be achieved. I'll give some thoughts to how that could be achieved best.

[JerrySievert]

fantastic! I look forward to seeing the addition, thanks for considering it.

[tomsanbear]

Hey @JerrySievert @porsager

I ended up taking a stab at instrumenting this library over here:
https://github.com/tomsanbear/opentelemetry-instrumentation-postgres

This is really just a result of a few hours trying to grasp this library's internals and finding a hacky way to get the query statement, along with start and end span markers. I'd love some recommendations @porsager if you have better thoughts on how to hook into this library. So far it works, but I wouldn't put it anywhere close to production without some more testing.

[JerrySievert]

@tomsanbear that's awesome, but is there some trick to getting it to work?

const {
  PostgresInstrumentation
} = require('opentelemetry-instrumentation-postgres');

and then in my instrumentations when instantiating the sdk:

  instrumentations: [
    getNodeAutoInstrumentations(),
    new HapiInstrumentation(),
    new PostgresInstrumentation(),
    new NetInstrumentation()
  ]

is there something else I need to do?

[younanjo]

@porsager checking to see if there is any updates on supporting OpenTelemetry?

[porsager]

I'm thinking the following to cover various use cases:

options.stats: true or sql.stats() gives you the following properties on the query result

result.duration // the complete duration of the query
result.waiting // the time the query waited to be sent
result.execution // the actual execution time (query sent to pg until response)

Now that's fine for hooking up or logging if required, but for the opentelemetry case, I'm thinking an option like this:

const sql = postgres({
  ...,
  onrequest() {
    // query queued
    return () => {
      // query sent
      return () => // query finished
    }
  }
})

I've got a branch with that setup here if you want to test it out https://github.com/porsager/postgres/tree/query-stats .. Let me know what you think:

[clouedoc]

What might be interesting would be to also provide hooks for different lifecycle events of the query;

i.e., for instance, there might be a high "time to connect to postgres" compared to "time of query execution"

[porsager]

That's exactly what the above is? 😉

[clouedoc]

Oh, haha, yes, sorry about that; I didn't pay enough attention 🤦‍♂️
Thanks for your library, by the way!

[gustavolzangelo]

I'm thinking the following to cover various use cases:

options.stats: true or sql.stats() gives you the following properties on the query result

result.duration // the complete duration of the query
result.waiting // the time the query waited to be sent
result.execution // the actual execution time (query sent to pg until response)

Now that's fine for hooking up or logging if required, but for the opentelemetry case, I'm thinking an option like this:

const sql = postgres({
  ...,
  onrequest() {
    // query queued
    return () => {
      // query sent
      return () => // query finished
    }
  }
})

I've got a branch with that setup here if you want to test it out https://github.com/porsager/postgres/tree/query-stats .. Let me know what you think:

@porsager this is perfect! I think these 3 stats are exactly what is needed. Is there any plan to implement this feature?

[hastebrot]

My approach for manual instrumentation would be to wrap the sql function returned by postgres() in another function that first calls tracer.startSpan(), then awaits the original sql function, and then calls span.end() with optional error handling.

Then the trace exporter would receive startTimeUnixNano and endTimeUnixNano (as defined in otlp-transformer/src/trace/types.ts).