-
Notifications
You must be signed in to change notification settings - Fork 219
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
aioredis instrumentation fix with extra context and statement #1406
aioredis instrumentation fix with extra context and statement #1406
Conversation
/test |
1 similar comment
/test |
Good catch! I'm not sure how this was missed in the original instrumentation. |
@ajaygupta2790 A few failing tests. Keep me posted if you need help resolving them. |
Yes I will need some help here, could you help me understand the test a little bit? |
Hey @ajaygupta2790 The reason that the test is failing is that you're adding a span where we didn't create spans before, namely in More generally speaking, so far we do not capture redis queries in any of our agents. This has been a deliberate choice, as redis keys quite often contain sensitive information like user names, ids, hashed passwords and similar. The risk of leaking such data from your production system to your monitoring system (in this case, Elasticsearch), is quite high. For SQL queries, we can do some sanitization, but this isn't possible for redis keys, which are essentially free-form. I'll take this topic to our agents team to see if we want to change this and capture redis queries going forward. |
/test |
Also add to redis, not just aioredis
🌐 Coverage report
|
I simplified and updated your solution to not add an additional span. Though the more I look at this the more I think it probably belongs in the instrumentation classes which actually create the spans.... I'll think on it. |
/test |
/test linters |
/test full |
What does this pull request do?
aioredis instrumentation is not available in elastic apm
Just like other instrumentations like asyncpg where we show what statement is executed, this PR adds the statement that is executed using the aioredis instrumentation
An example for the same is as follows:
GET service:key
Adds Redis under dependencies as well