-
-
Notifications
You must be signed in to change notification settings - Fork 0
fix(db): Clarify db.query.text and db.query.summary attributes
#208
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
base: main
Are you sure you want to change the base?
Conversation
lcian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, you will need to regenerate and reformat.
Also, example needs to be a string, we can change that but for the purpose of the PR just provide a single example as a string.
|
+1 for being able to add more than one example. For now I removed the second example. Thanks for the review! |
|
@mjq and @Ahmed-Labs should take a look here as well. |
|
Makes sense! We will probably have to update db attribute normalization logic because we currently expect a raw query as opposed to a parametrized one in |
|
Just to add here, JS SDK instrumentations have been sending the parametrized version in |
|
@Ahmed-Labs @cleptric should we move forward with this change? |
| }, | ||
| "is_in_otel": true, | ||
| "example": "SELECT * FROM users", | ||
| "example": "SELECT * FROM users WHERE id = $1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I have this query:
INSERT INTO exec_test (id, name) VALUES (?, ?, ?, ?)Should it become as it is, or:
INSERT INTO exec_test (id, name) VALUES (?)
db.query.text: We previously specified this attribute to contain a full/raw query without parameterization. OTel requires the paramterized query.db.query.summaryWe specified this attribute to contain the paramterized query. OTel specifies a shortened operation/table "grouping".This came up while reviewing getsentry/sentry-javascript#17961