Skip to content
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

Ignore forwarding CQL requests for Insights Client #139

Merged
merged 1 commit into from
Feb 28, 2025
Merged

Conversation

lukasz-antoniak
Copy link
Contributor

No description provided.

@lukasz-antoniak lukasz-antoniak marked this pull request as ready for review February 26, 2025 12:41
@@ -213,13 +214,18 @@ func getRequestInfoFromQueryInfo(
}
} else if queryInfo.getStatementType() == statementTypeUse {
sendAlsoToAsync = true
} else if queryInfo.getStatementType() == statementTypeCall {
// RPC client, typically used by "insights reporter" for DSE
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if it's safer to parse the actual function being called, I'm not sure if there are other RPCs being used with DSE, maybe stuff related to Spark and/or search

Copy link
Collaborator

Choose a reason for hiding this comment

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

Storing the first identifier would be enough I think? And then check if that identifier matches InsightsRpc (case insensitive to be safe)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Very good idea. Updated code.

@@ -213,13 +218,20 @@ func getRequestInfoFromQueryInfo(
}
} else if queryInfo.getStatementType() == statementTypeUse {
sendAlsoToAsync = true
} else if queryInfo.getStatementType() == statementTypeCall {
if queryInfo.getCallRpcName() == insightsRpcName {
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this be a case insensitive comparison? Idk if using CALL insightsrpc work so better be safe than sorry?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@joao-r-reis
Copy link
Collaborator

Very nice stuff 👍

@lukasz-antoniak lukasz-antoniak merged commit 57f11de into main Feb 28, 2025
8 checks passed
@lukasz-antoniak lukasz-antoniak deleted the insights branch February 28, 2025 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants