-
Notifications
You must be signed in to change notification settings - Fork 240
OpenTelemetry follow up PR #613
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
Conversation
I just realized that the way we store the node level activity might not work if there are multiple requests going to the same node as part of the same request context. I think in normal circumstances this doesn't happen but it really depends on the load balancing policy (and speculative executions probably need to be enabled as well). I'll probably add a |
src/Extensions/Cassandra.OpenTelemetry/OpenTelemetryRequestTracker.cs
Outdated
Show resolved
Hide resolved
src/Extensions/Cassandra.OpenTelemetry/OpenTelemetryRequestTracker.cs
Outdated
Show resolved
Hide resolved
Yeah, I think the |
yeah I just added it now, was a bit of work but it's worth it I think |
Ok, speculative executions are now supported as well, aborted executions will show up with status "unset", while non aborted executions have the status The last thing that I wanted to take care of before releasing this was to add support for Prepared Statements. It would be very useful to have the Prepare round trips on these traces (especially with Mapper and LINQ that automatically prepare stuff internally). I'm running out of time to get this done though so I'll evaluate tomorrow wether it is feasible to add this before the release. |
Looks really cool. Btw, if you pass the number of speculative executions, you can add the attribute db.cassandra.speculative_execution_count. You already did part of the job my marking it The prepared statements would be a really useful feature to add but we can also include in the documentation that is not supported for now, so the users are aware. |
Unfortunately there's no way to tell whether an execution is speculative or not atm (could just be a retry) so I'd have to implement that on the driver. Something to consider in the future though. |
I'm going to merge this when CI finishes. |
db.query.text
support forBoundStatement
andBatchStatement
cc @simaoribeiro