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

Fix semantic conventions for PG and PG Pool #467

Merged
merged 7 commits into from
May 8, 2021

Conversation

svrnm
Copy link
Member

@svrnm svrnm commented May 4, 2021

Which problem is this PR solving?

This should fix #116, #296, #297 and #465

Short description of the changes

Replace all official semantic conventions with their values from @opentelemetry/semantic-conventions package.
Leave some of the old custom attributes as they are, but put them into a namespace db.postgresql (while not officialy stated in https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/database.md) there are some other examples with db.<db.system>, e.g. db.mssql.instance_name or db.mongodb.collection

  PG_VALUES = 'db.postgresql.values',
  PG_PLAN = 'db.postgresql.plan',
  IDLE_TIMEOUT_MILLIS = 'db.postgresql.idle.timeout.millis',
  MAX_CLIENT = 'db.postgresql.max.client',

@svrnm svrnm requested a review from a team May 4, 2021 08:55
@codecov
Copy link

codecov bot commented May 4, 2021

Codecov Report

Merging #467 (519fbda) into main (8858e60) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

❗ Current head 519fbda differs from pull request most recent head 0ccc386. Consider uploading reports for the commit 0ccc386 to get more accurate results

@@            Coverage Diff             @@
##             main     #467      +/-   ##
==========================================
- Coverage   95.29%   95.23%   -0.07%     
==========================================
  Files         131      130       -1     
  Lines        8116     8325     +209     
  Branches      751      811      +60     
==========================================
+ Hits         7734     7928     +194     
- Misses        382      397      +15     
Impacted Files Coverage Δ
...node/opentelemetry-instrumentation-pg/src/enums.ts 100.00% <100.00%> (ø)
...ns/node/opentelemetry-instrumentation-pg/src/pg.ts 82.29% <100.00%> (ø)
...node/opentelemetry-instrumentation-pg/src/utils.ts 97.26% <100.00%> (+0.03%) ⬆️
...ntelemetry-instrumentation-pg/test/pg-pool.test.ts 89.51% <100.00%> (+0.08%) ⬆️
...e/opentelemetry-instrumentation-pg/test/pg.test.ts 94.42% <100.00%> (+0.02%) ⬆️
...ce-detector-github/src/detectors/GitHubDetector.ts
...ry-resource-detector-github/src/detectors/index.ts
...ector-github/test/detectors/GitHubDetector.test.ts
...ntelemetry-resource-detector-github/src/version.ts
...umentation-user-interaction/src/userInteraction.ts 93.86% <0.00%> (ø)
... and 2 more

dyladan
dyladan previously requested changes May 4, 2021
Copy link
Member

@dyladan dyladan left a comment

Choose a reason for hiding this comment

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

Blocked because your local path is in the example package.json

examples/postgres/package.json Outdated Show resolved Hide resolved
Copy link
Member

@obecny obecny left a comment

Choose a reason for hiding this comment

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

lgtm

examples/postgres/package.json Outdated Show resolved Hide resolved
@vmarchaud vmarchaud force-pushed the fix-pg-semantic-conventions branch from d5c4397 to a7bcd61 Compare May 8, 2021 08:13
@vmarchaud vmarchaud merged commit 1386d75 into open-telemetry:main May 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PostgreSQL instrumentation uses wrong attributes [pg + pg-pool] Update DB semantic conventions
4 participants