-
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(redis): serialize non sensitive arguments into db.statement attribute #1299
fix(redis): serialize non sensitive arguments into db.statement attribute #1299
Conversation
….statement attribute
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1299 +/- ##
==========================================
- Coverage 96.08% 93.72% -2.36%
==========================================
Files 14 21 +7
Lines 893 1338 +445
Branches 191 263 +72
==========================================
+ Hits 858 1254 +396
- Misses 35 84 +49
|
…ase (removed by mistake)
…ase (removed by mistake)
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.
Great contribution as usual 🥇
Added a few minor comments
plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts
Outdated
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-redis-4/src/utils.ts
Outdated
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-redis/examples/package.json
Outdated
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts
Outdated
Show resolved
Hide resolved
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 💯
…lemetry-js-contrib into redis-db-statement
Which problem is this PR solving?
This issue: #1294
Short description of the changes
The code in ioredis that serialize the non-sensitive data (here: #1052) was exported to another new package (called "redis-common"), and now all redis instrumentations (redis, redis-4, ioredis) use this common code to serialize the
db.statement
attribute while considering the sensitive data.