feat(pg): support hooking into span when query is started

[ethanresnick]

Which problem is this PR solving?

Right now, it's possible to modify the span generated for a query after results are returned for the query, using the responseHook. However, it isn't possible to modify the span at the start of the query, based on the information available when the query is issued (i.e., the args passed to client.query), because that information isn't passed to the responseHook.

For my use case, I'm using Datadog and am tracing an app that issues relatively few distinct queries. Therefore, it makes sense to identify each query as a distinct Datadog "resource", by adding a resource.name attribute to each span, where the value of the attribute is the query text. However, this is currently impossible, because the query text isn't available in the responseHook, and there's no other hook where it is available.

Short description of the changes

• Add a queryHook param to support the above use case.

NB: this PR shares the first two commits with #1306, so it may be simpler to review that first. It relies on the code cleanup from #1306 to simplify implementing the queryHook here. (The code cleanup now allows the main code path in _getClientQueryPatch to have a single, normalized queryConfig object that can be passed to the queryHook.)

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: ethanresnick / name: Ethan Resnick (ceb8f02, d74d330, 5e6f995, 5e50b92)

[blumamir]

Maybe I am missing something, but the query text is currently always captured into db.statement span attribute here
Are you looking to duplicate this attribute in your app and record the same text twice?
I am good with adding this hook if you need it but just wanna make sure it is needed.

[ethanresnick]

Are you looking to duplicate this attribute in your app and record the same text twice?

Yup, exactly. I need to duplicate the value into the resource.name attribute to make the Datadog UI work well for my use case (see OP).

[blumamir]

Are you looking to duplicate this attribute in your app and record the same text twice?

Yup, exactly. I need to duplicate the value into the resource.name attribute to make the Datadog UI work well for my use case (see OP).

Are you using OpenTelemetry Collector to send spans to DD? there is the attributesprocessor which does just that

If you can't modify this value in collector or prefer to use instrumentation hook that is fine and I can review this PR.

[codecov]

Codecov Report

Merging #1307 (dd3f8ba) into main (8a375f5) will decrease coverage by 0.27%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1307      +/-   ##
==========================================
- Coverage   96.08%   95.80%   -0.28%     
==========================================
  Files          14       18       +4     
  Lines         893     1217     +324     
  Branches      191      259      +68     
==========================================
+ Hits          858     1166     +308     
- Misses         35       51      +16     

Impacted Files	Coverage Δ
...elemetry-instrumentation-pg/src/instrumentation.ts	90.90% <100.00%> (ø)
...node/opentelemetry-instrumentation-pg/src/utils.ts	98.33% <100.00%> (ø)
...try-instrumentation-pg/src/enums/AttributeNames.ts	100.00% <0.00%> (ø)
...tapackages/auto-instrumentations-node/src/utils.ts	98.21% <0.00%> (ø)

[ethanresnick]

Are you using OpenTelemetry Collector to send spans to DD?

Unfortunately I'm using the DD agent's built-in support for receiving Otel data (this approach), so that I don't have to run a standalone instance of the collector. I think that the DD agent doesn't support attribute processors (or it doesn't expose that support), so a hook seems easier.

[ethanresnick]

@blumamir Ok, I think I addressed all your comments. I also ended up renaming the hook from queryHook to requestHook, to match the name of the analogous hook on the redis instrumentation (and for parallelism with responseHook). Honestly, requestHook feels like a bit of a weird name to me, but I figured it was more important to be consistent.

[blumamir]

LGTM 🥇

[ethanresnick]

@rauno56 Given your comment in #1306, I guess this is good to merge too (once CI finishes)? Thanks :)