Skip to content

fix(instrumentation-pg): connection string parsing #2715

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

nikmel2803
Copy link
Contributor

Which problem is this PR solving?

Fixes #1415

Short description of the changes

  • Handle the connectionString param in the PgPool constructor

The changes have also been tested in our org's telemetry setup.

@nikmel2803 nikmel2803 requested a review from a team as a code owner February 13, 2025 18:00
Copy link

linux-foundation-easycla bot commented Feb 13, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link

codecov bot commented Feb 14, 2025

Codecov Report

Attention: Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.42%. Comparing base (675ec97) to head (f640f30).

Files with missing lines Patch % Lines
...node/opentelemetry-instrumentation-pg/src/utils.ts 95.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2715   +/-   ##
=======================================
  Coverage   92.41%   92.42%           
=======================================
  Files         171      171           
  Lines        8150     8169   +19     
  Branches     1653     1660    +7     
=======================================
+ Hits         7532     7550   +18     
- Misses        618      619    +1     
Files with missing lines Coverage Δ
...telemetry-instrumentation-pg/src/internal-types.ts 100.00% <ø> (ø)
...node/opentelemetry-instrumentation-pg/src/utils.ts 97.27% <95.00%> (-0.38%) ⬇️

@nikmel2803
Copy link
Contributor Author

hey @maryliag!

can I please get a review on this PR?

@deejay1
Copy link
Contributor

deejay1 commented Feb 19, 2025

@nikmel2803 linter fails

Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @nikmel2803, thank you for your contribution! I added a few comments/questions!

@nikmel2803
Copy link
Contributor Author

hey @deejay1!
my bad, i thought it was complaining about other modules.
fixed now

@@ -138,13 +159,24 @@ export function getSemanticAttributesFromConnection(
}

export function getSemanticAttributesFromPool(params: PgPoolOptionsParams) {
// const connectionParams = getConnectionParams(params);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like you forgot to remove this line

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, right! fixed

Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution!

@pichlermarc pichlermarc merged commit b520d04 into open-telemetry:main Feb 28, 2025
4 checks passed
@dyladan dyladan mentioned this pull request Feb 28, 2025
deejay1 pushed a commit to deejay1/opentelemetry-js-contrib that referenced this pull request Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

instrumentation-pg records wrong db.connection_string when pg.Pool is configured by connectionString
4 participants