Skip to content

Conversation

@randomanderson
Copy link
Contributor

Instrumentation for cassandra 4. The large API changes between versions means not much can be salvaged from the cassandra 3 instrumentation other than the test.

@randomanderson randomanderson added the inst: others All other instrumentations label Dec 2, 2020
@randomanderson randomanderson requested a review from a team as a code owner December 2, 2020 21:16
@randomanderson
Copy link
Contributor Author

#1901

throwable = throwable.getCause();
}
DECORATE.onError(span, throwable);
span.finish();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this span be the active span at this time? If so, this lambda wouldn't need to capture here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems unfortunate that we go to such lengths and quite some cost to propagate context into completable futures, when all we really need to do is capture the span in a closure, but we're doing both here, the CompletableFuture instrumentation should be making sure this would be the active span at the time.

public class CassandraClientInstrumentation extends Instrumenter.Default {

public CassandraClientInstrumentation() {
super("cassandra");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to add a secondary name to distinguish 3 from 4?

@randomanderson randomanderson force-pushed the landerson/datastax-4 branch 3 times, most recently from adb9c26 to c93f1c0 Compare December 3, 2020 03:31
@randomanderson randomanderson merged commit 7d6a41b into master Dec 3, 2020
@randomanderson randomanderson deleted the landerson/datastax-4 branch December 3, 2020 19:16
@github-actions github-actions bot added this to the 0.69.0 milestone Dec 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

inst: others All other instrumentations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants