Skip to content

opentelemetry support or hooks? #461

Open
@JerrySievert

Description

@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!

Activity

porsager

porsager commented on Aug 14, 2022

@porsager
Owner

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

JerrySievert commented on Aug 14, 2022

@JerrySievert
Author

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

tomsanbear

tomsanbear commented on Sep 27, 2022

@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

JerrySievert commented on Oct 6, 2022

@JerrySievert
Author

@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

younanjo commented on Mar 2, 2023

@younanjo

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

porsager

porsager commented on Jun 25, 2023

@porsager
Owner

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

clouedoc commented on Jun 30, 2023

@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

porsager commented on Jul 1, 2023

@porsager
Owner

That's exactly what the above is? 😉

clouedoc

clouedoc commented on Jul 1, 2023

@clouedoc

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

gustavolzangelo

gustavolzangelo commented on Aug 11, 2023

@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

hastebrot commented on Aug 14, 2023

@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).

24 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      Participants

      @stephenh@neerfri@willhoney7@mbrevda@JerrySievert

      Issue actions

        opentelemetry support or hooks? · Issue #461 · porsager/postgres