-
Notifications
You must be signed in to change notification settings - Fork 540
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(mysql): add enhancedDatabaseReporting to mysql #1337
fix(mysql): add enhancedDatabaseReporting to mysql #1337
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1337 +/- ##
==========================================
- Coverage 96.08% 95.89% -0.20%
==========================================
Files 14 18 +4
Lines 893 1168 +275
Branches 191 232 +41
==========================================
+ Hits 858 1120 +262
- Misses 35 48 +13
|
plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts
Outdated
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts
Outdated
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts
Outdated
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts
Outdated
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts
Outdated
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts
Outdated
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-mysql/test/index.test.ts
Outdated
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-mysql/test/index.test.ts
Outdated
Show resolved
Hide resolved
…lemetry-js-contrib into mysql-db-statement
plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts
Outdated
Show resolved
Hide resolved
Thanks @haddasbronfman , Maybe I am missing something, but wouldn't all the potentially sensitive values still be written to the |
You're right. |
The instrumentation actually receives the plain SQL statement without values, and it inject the values here by calling the if (typeof query === 'string') {
return values ? format(query, values) : query;
} else {
// According to https://github.com/mysqljs/mysql#performing-queries
// The values argument will override the values in the option object.
return values || query.values
? format(query.sql, values || query.values)
: query.sql;
} This transforms a query such as I think that if we remove the |
…lemetry-js-contrib into mysql-db-statement
Which problem is this PR solving?
fixes #1237
Short description of the changes
I added a config object to
MySQLInstrumentation
with a field calleddbStatementSerializer
. This is a function which the user can set and decide how to parse his db.statement.