Skip to content
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

postgres: DBM full service fallback w/ prepared statements #3186

Merged
merged 3 commits into from
May 31, 2023

Conversation

tlhunter
Copy link
Member

@tlhunter tlhunter commented May 24, 2023

What does this PR do?

  • when DBM is set to full service
    • the DBM comment falls back to service mode
    • but only when sending a prepared statement

Motivation

Plugin Checklist

@github-actions
Copy link

github-actions bot commented May 24, 2023

Overall package size

Self size: 4.18 MB
Deduped: 58.38 MB
No deduping: 58.43 MB

Dependency sizes

name version self size total size
@datadog/pprof 2.2.1 14.24 MB 15.12 MB
@datadog/native-iast-taint-tracking 1.4.1 14.85 MB 14.86 MB
@datadog/native-appsec 3.2.0 13.38 MB 13.39 MB
protobufjs 7.1.2 2.76 MB 6.55 MB
@datadog/native-iast-rewriter 2.0.1 2.09 MB 2.1 MB
@datadog/native-metrics 2.0.0 898.77 kB 1.3 MB
opentracing 0.14.7 194.81 kB 194.81 kB
semver 7.3.8 88.2 kB 118.6 kB
@datadog/sketches-js 2.1.0 109.9 kB 109.9 kB
lodash.sortby 4.7.0 75.76 kB 75.76 kB
lru-cache 7.14.0 74.95 kB 74.95 kB
ipaddr.js 2.0.1 59.52 kB 59.52 kB
ignore 5.2.0 48.87 kB 48.87 kB
import-in-the-middle 1.3.5 34.34 kB 38.81 kB
istanbul-lib-coverage 3.2.0 29.34 kB 29.34 kB
retry 0.10.1 27.44 kB 27.44 kB
lodash.uniq 4.5.0 25.01 kB 25.01 kB
limiter 1.1.5 23.17 kB 23.17 kB
lodash.kebabcase 4.1.1 17.75 kB 17.75 kB
lodash.pick 4.4.0 16.33 kB 16.33 kB
node-abort-controller 3.0.1 14.33 kB 14.33 kB
crypto-randomuuid 1.0.0 11.18 kB 11.18 kB
diagnostics_channel 1.1.0 7.07 kB 7.07 kB
path-to-regexp 0.1.7 6.78 kB 6.78 kB
koalas 1.0.2 6.47 kB 6.47 kB
methods 1.1.2 5.29 kB 5.29 kB
module-details-from-path 1.0.3 4.47 kB 4.47 kB

🤖 This report was automatically generated by heaviest-objects-in-the-universe

@tlhunter tlhunter force-pushed the tlhunter/dbm-pg-prepared-statements branch from 3dd5e98 to 0864b0f Compare May 24, 2023 17:29
@codecov
Copy link

codecov bot commented May 24, 2023

Codecov Report

Merging #3186 (ed12728) into master (d638b4b) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3186      +/-   ##
==========================================
- Coverage   86.57%   86.54%   -0.03%     
==========================================
  Files         333      331       -2     
  Lines       11934    11894      -40     
  Branches       33       33              
==========================================
- Hits        10332    10294      -38     
+ Misses       1602     1600       -2     
Impacted Files Coverage Δ
packages/datadog-plugin-pg/src/index.js 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@pr-commenter
Copy link

pr-commenter bot commented May 24, 2023

Benchmarks

Comparing candidate commit ed12728 in PR branch tlhunter/dbm-pg-prepared-statements with baseline commit d638b4b in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 446 metrics, 26 unstable metrics.

@tlhunter tlhunter force-pushed the tlhunter/dbm-pg-prepared-statements branch from 0864b0f to 5f2d2b1 Compare May 31, 2023 19:44
const query = {
name: 'pgSelectQuery',
Copy link
Member Author

Choose a reason for hiding this comment

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

This was actually an example of the bug happening. The previous test had a name and was therefore a prepared statement. The test asserts that the traceparent is present in the comment but it should actually not be present.

@tlhunter tlhunter marked this pull request as ready for review May 31, 2023 19:52
@tlhunter tlhunter requested a review from a team as a code owner May 31, 2023 19:52
@tlhunter tlhunter merged commit a1d1e06 into master May 31, 2023
104 checks passed
@tlhunter tlhunter deleted the tlhunter/dbm-pg-prepared-statements branch May 31, 2023 21:29
uurien pushed a commit that referenced this pull request Jun 1, 2023
- when DBM is set to full service
  - the DBM comment falls back to service mode
  - but only when sending a prepared statement
- without this, each prepared statement query is technically different
  - this causes the pg library to fail as it does an exact string check of the query
  - ideally the pg library would somehow parse out and not consider comments
- at any rate this brings parity with other tracer implementations
- see brianc/node-postgres#2735
- previously attempted in 0ff9465
uurien pushed a commit that referenced this pull request Jun 1, 2023
- when DBM is set to full service
  - the DBM comment falls back to service mode
  - but only when sending a prepared statement
- without this, each prepared statement query is technically different
  - this causes the pg library to fail as it does an exact string check of the query
  - ideally the pg library would somehow parse out and not consider comments
- at any rate this brings parity with other tracer implementations
- see brianc/node-postgres#2735
- previously attempted in 0ff9465
uurien pushed a commit that referenced this pull request Jun 1, 2023
- when DBM is set to full service
  - the DBM comment falls back to service mode
  - but only when sending a prepared statement
- without this, each prepared statement query is technically different
  - this causes the pg library to fail as it does an exact string check of the query
  - ideally the pg library would somehow parse out and not consider comments
- at any rate this brings parity with other tracer implementations
- see brianc/node-postgres#2735
- previously attempted in 0ff9465
This was referenced Jun 1, 2023
uurien pushed a commit that referenced this pull request Jun 2, 2023
- when DBM is set to full service
  - the DBM comment falls back to service mode
  - but only when sending a prepared statement
- without this, each prepared statement query is technically different
  - this causes the pg library to fail as it does an exact string check of the query
  - ideally the pg library would somehow parse out and not consider comments
- at any rate this brings parity with other tracer implementations
- see brianc/node-postgres#2735
- previously attempted in 0ff9465
uurien pushed a commit that referenced this pull request Jun 2, 2023
- when DBM is set to full service
  - the DBM comment falls back to service mode
  - but only when sending a prepared statement
- without this, each prepared statement query is technically different
  - this causes the pg library to fail as it does an exact string check of the query
  - ideally the pg library would somehow parse out and not consider comments
- at any rate this brings parity with other tracer implementations
- see brianc/node-postgres#2735
- previously attempted in 0ff9465
uurien pushed a commit that referenced this pull request Jun 2, 2023
- when DBM is set to full service
  - the DBM comment falls back to service mode
  - but only when sending a prepared statement
- without this, each prepared statement query is technically different
  - this causes the pg library to fail as it does an exact string check of the query
  - ideally the pg library would somehow parse out and not consider comments
- at any rate this brings parity with other tracer implementations
- see brianc/node-postgres#2735
- previously attempted in 0ff9465
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.

None yet

3 participants