-
Notifications
You must be signed in to change notification settings - Fork 36
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
ioredis clash #1327
Comments
Hey @a-reuss ! Thanks for your report! We will take a look now. If possible, could you please share parts of your application log? Thanks so much! |
@a-reuss Are you using a cluster or a single connection? |
I will revert the multi/pipeline change. We thought that this is a bug. |
@kirrg001 do you have an idea, how long it will take for the updated version to be available? |
I am already working on the revert. We can release it tomorrow. |
refs #1327 refs #1292 We thought that this is a bug. The tests were not really clear. We revert the multi/pipeline handling for now. Raised https://jsw.ibm.com/browse/INSTA-14540 for further investigation.
Raised PR #1328 Hopefully it resolves the problem 💁♀️ |
Thanks, we will check and give feedback |
refs #1327 refs #1292 We thought that this is a bug. The tests were not really clear. We revert the multi/pipeline handling for now. Raised https://jsw.ibm.com/browse/INSTA-14540 for further investigation.
We have successfully shipped 3.18.1 (except for the AWS Lambda Layer China regions cn-north-1 and cn-northwest-1). If the hot fix does not resolve the problem, please downgrade to v3.17.1 and we will investigate with more time next week. Furthermore: Could you please share some more information about the use case / example code (if you can)?
Thank you so much! |
@kirrg001, I will send details to your email address from my corporate account (not the account I use here) for reasons of compliance. |
Hi, @kirrg001. As written in the mail, we see that the problem persists with 3.18.1 . Do you have a porposal how to proceed. AS recommented, we pinned the version 3.17.1, but that is only viable until a potential bug or CVE will force us to uprade. Thanks so far! |
Thanks for sharing! I am working on another release. |
We have released 3.18.2. |
Branch is created, waiting for pipeline |
Es lebt! ;) |
Thank you. |
Problem Description
In our application, we use ioredis as short term database. Since the update of instana to version 3.18.0 where there were changes to the interaction between instana collector and redis used as instana internal data store, our traces cannot be correctly related to traces recorded in the rest of the system.
Short, Self Contained Example
The reason is that although the necessary header ist correctly transferred through all services involved, the trace ids between our subsystem and the rest of the system do not match anymore. Is it possible that there is a misconfiguration for the ioredis connection instana uses. The most likely candidate for this is the newly introduced code block from line 40 in packages/core/src/tracing/instrumentation/database/ioredis.js
Text
clusterConnectionString = this.startupNodes.map(node =>
${node.host}:${node.port}
).join(',');Additionally, we see issues with parent spans that might come from the change in line 84 in packages/core/src/tracing/instrumentation/database/ioredis.js where 'multi' commands are now treated differently to before. We see error messages of the kind
Cannot start an intermediate span (Foo.bar()) as this requires an active entry (or intermediate) span as parent. But the currently active span is an exit span:
that we did not see before.
We do not know whether the 2 issues are linked to each other at all, or whether they are independent.
Node.js Version
20.x
package.json
package-lock.json
The text was updated successfully, but these errors were encountered: