-
Notifications
You must be signed in to change notification settings - Fork 12
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
Prepare statements support #9
Prepare statements support #9
Conversation
import com.datastax.driver.core.ResultSetFuture; | ||
import com.datastax.driver.core.Session; | ||
import com.datastax.driver.core.Statement; | ||
import com.datastax.driver.core.*; |
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.
please don't use stars in imports
Span span = buildSpan(query); | ||
try { | ||
PreparedStatement resultSet = session.prepare(statement); | ||
finishSpan(span); |
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.
shouldn't span created (started) when prepared statement started to execute.
and span finished when prepared statement finished?
looks like here creation of statement is traced, but not execution.
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.
ha! you're right ;)
I will 👀 into it
After some research and debugging:
|
@tmszdmsk let's start from beginning. |
@malafeev, sorry for the delay I wrote a test that demonstrates the problem, it is actually why the tests are failing: |
@tmszdmsk I mean what is the initial problem? |
I'm closing this PR because it doesn't fix the issue. |
Fixes #8.
One test case added. Under the hood it uses one of
prepareAsync
methods onsession
, I've fixed allprepare*
methods though.