You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "raj14243 (via GitHub)" <gi...@apache.org> on 2026/04/06 23:42:22 UTC
[PR] Add Table field to ObservedQuery and ObservedBatch for table-level observability [cassandra-gocql-driver]
raj14243 opened a new pull request, #1941:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1941
ObservedQuery and ObservedBatch did not expose the target table name, requiring consumers (metrics collectors, loggers, tracers) to parse the CQL statement string to determine which table a query targets.
This change adds a Table field to ObservedQuery and a Tables field to ObservedBatch, populated from prepared statement metadata returned by Cassandra. The table information was already available internally via preparedMetadata (used for routing), so no additional round-trips or CQL parsing are needed.
Patch by Raj Ummadisetty for CASSGO-119
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] Add Table field to ObservedQuery and ObservedBatch for table-level observability [cassandra-gocql-driver]
Posted by "joao-r-reis (via GitHub)" <gi...@apache.org>.
joao-r-reis commented on code in PR #1941:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1941#discussion_r3102027896
##########
query_executor.go:
##########
@@ -383,17 +383,19 @@ func (q *internalQuery) attempt(keyspace string, end, start time.Time, iter *Ite
if q.qryOpts.observer != nil {
metricsForHost := q.hostMetricsManager.attempt(latency, host)
q.qryOpts.observer.ObserveQuery(q.qryOpts.context, ObservedQuery{
- Keyspace: keyspace,
- Statement: q.qryOpts.stmt,
- Values: q.qryOpts.values,
- Start: start,
- End: end,
- Rows: iter.numRows,
- Host: host,
- Metrics: metricsForHost,
- Err: iter.err,
- Attempt: attempt,
- Query: q.originalQuery,
+ Keyspace: keyspace,
+ Statement: q.qryOpts.stmt,
+ StatementKeyspace: q.routingInfo.getKeyspace(),
Review Comment:
Hmm I don't think we should be adding CQL parsing for something like this, most if not all drivers avoid doing any kind of CQL parsing. What some drivers do is expose a statement builder / mapper API that will create statements and internally set fields so the driver is aware of some metadata like keyspace, table, idempotency without having to parse it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSGO-119 Add Table field to ObservedQuery and ObservedBatch for table-level observability [cassandra-gocql-driver]
Posted by "joao-r-reis (via GitHub)" <gi...@apache.org>.
joao-r-reis commented on PR #1941:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1941#issuecomment-4595077964
Let's hold off on merging this until 2.1.2 is released
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] Add Table field to ObservedQuery and ObservedBatch for table-level observability [cassandra-gocql-driver]
Posted by "worryg0d (via GitHub)" <gi...@apache.org>.
worryg0d commented on PR #1941:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1941#issuecomment-4553295569
Besides CASSGO-120, this PR looks good.
I guess you can get rid of CASSGO-120 in the commit message, and we will handle it separately if it is worth fixing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] Add Table field to ObservedQuery and ObservedBatch for table-level observability [cassandra-gocql-driver]
Posted by "worryg0d (via GitHub)" <gi...@apache.org>.
worryg0d commented on code in PR #1941:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1941#discussion_r3063530308
##########
query_executor.go:
##########
@@ -383,17 +383,19 @@ func (q *internalQuery) attempt(keyspace string, end, start time.Time, iter *Ite
if q.qryOpts.observer != nil {
metricsForHost := q.hostMetricsManager.attempt(latency, host)
q.qryOpts.observer.ObserveQuery(q.qryOpts.context, ObservedQuery{
- Keyspace: keyspace,
- Statement: q.qryOpts.stmt,
- Values: q.qryOpts.values,
- Start: start,
- End: end,
- Rows: iter.numRows,
- Host: host,
- Metrics: metricsForHost,
- Err: iter.err,
- Attempt: attempt,
- Query: q.originalQuery,
+ Keyspace: keyspace,
+ Statement: q.qryOpts.stmt,
+ StatementKeyspace: q.routingInfo.getKeyspace(),
Review Comment:
`routingInfo` could not have any information about keyspaces/tables for DDL queries that are unprepareable, so you probably should use `internalQuery.Keyspace()` here. It has some predefined defaulting behavior.
We should probably think about adding a kind of parsing of DDL queries to determine keyspace and table names from it before defaulting to session-level keyspace.
Also, there is an alternative for `internalBatch.Keyspace()` as well, but it just calls the underlying routingInfo, which is fine because it is impossible to batch DDL queries.
@joao-r-reis, could you please take a look at this?
##########
session.go:
##########
@@ -2374,6 +2374,14 @@ type ObservedQuery struct {
Keyspace string
Statement string
+ // StatementKeyspace is the keyspace of the table targeted by the query.
+ // It is populated from prepared statement metadata returned by Cassandra.
+ StatementKeyspace string
Review Comment:
I think it is fine to populate the existing `Keyspace` field here - it is unclear to see the difference between `Keyspace` and `StatementKeyspace`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSGO-119 Add Table field to ObservedQuery and ObservedBatch for table-level observability [cassandra-gocql-driver]
Posted by "joao-r-reis (via GitHub)" <gi...@apache.org>.
joao-r-reis commented on PR #1941:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1941#issuecomment-4742098086
@raj14243 can you add both of us as reviewers to the commit message and fix the git conflict? Will merge afterwards
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSGO-119 Add Table field to ObservedQuery and ObservedBatch for table-level observability [cassandra-gocql-driver]
Posted by "joao-r-reis (via GitHub)" <gi...@apache.org>.
joao-r-reis commented on PR #1941:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1941#issuecomment-4743043565
Or if you don't mind github not recognizing the PR as "merged" then I can do that for you and set you as author of the commit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] Add Table field to ObservedQuery and ObservedBatch for table-level observability [cassandra-gocql-driver]
Posted by "raj14243 (via GitHub)" <gi...@apache.org>.
raj14243 commented on code in PR #1941:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1941#discussion_r3251530097
##########
query_executor.go:
##########
@@ -383,17 +383,19 @@ func (q *internalQuery) attempt(keyspace string, end, start time.Time, iter *Ite
if q.qryOpts.observer != nil {
metricsForHost := q.hostMetricsManager.attempt(latency, host)
q.qryOpts.observer.ObserveQuery(q.qryOpts.context, ObservedQuery{
- Keyspace: keyspace,
- Statement: q.qryOpts.stmt,
- Values: q.qryOpts.values,
- Start: start,
- End: end,
- Rows: iter.numRows,
- Host: host,
- Metrics: metricsForHost,
- Err: iter.err,
- Attempt: attempt,
- Query: q.originalQuery,
+ Keyspace: keyspace,
+ Statement: q.qryOpts.stmt,
+ StatementKeyspace: q.routingInfo.getKeyspace(),
Review Comment:
I will check the `internalQuery.Keyspace()` I agree with @joao-r-reis on not having CQL parsing. I started this PR as we were internally parsing statement to extract keyspace and wanted to avoid that :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSGO-119 Add Table field to ObservedQuery and ObservedBatch for table-level observability [cassandra-gocql-driver]
Posted by "raj14243 (via GitHub)" <gi...@apache.org>.
raj14243 commented on PR #1941:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1941#issuecomment-4756065614
> @raj14243 can you add both of us as reviewers to the commit message and fix the git conflict? Will merge afterwards
@joao-r-reis done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] Add Table field to ObservedQuery and ObservedBatch for table-level observability [cassandra-gocql-driver]
Posted by "worryg0d (via GitHub)" <gi...@apache.org>.
worryg0d commented on PR #1941:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1941#issuecomment-4368864599
> Wdyt?
Yeah I guess it would work, but API looks a bit gross. As alternative to exposing those fields to ObservedXY structs, you may call this method: https://github.com/apache/cassandra-gocql-driver/blob/c65c762b83eccdb6a6a083c123a5935c4302745c/session.go#L756
It basically allows to retrieve internally cached statement metadata. But this requires observers to be able to call session methods
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSGO-119 Add Table field to ObservedQuery and ObservedBatch for table-level observability [cassandra-gocql-driver]
Posted by "joao-r-reis (via GitHub)" <gi...@apache.org>.
joao-r-reis merged PR #1941:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1941
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] Add Table field to ObservedQuery and ObservedBatch for table-level observability [cassandra-gocql-driver]
Posted by "joao-r-reis (via GitHub)" <gi...@apache.org>.
joao-r-reis commented on code in PR #1941:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1941#discussion_r3102041604
##########
CHANGELOG.md:
##########
@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [2.1.1]
+### Added
Review Comment:
Add a new 2.2.0 section
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] Add Table field to ObservedQuery and ObservedBatch for table-level observability [cassandra-gocql-driver]
Posted by "raj14243 (via GitHub)" <gi...@apache.org>.
raj14243 commented on code in PR #1941:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1941#discussion_r3251523794
##########
session.go:
##########
@@ -2374,6 +2374,14 @@ type ObservedQuery struct {
Keyspace string
Statement string
+ // StatementKeyspace is the keyspace of the table targeted by the query.
+ // It is populated from prepared statement metadata returned by Cassandra.
+ StatementKeyspace string
Review Comment:
The existing Keyspace field is populated with Keyspace the session is initialized with. I am concerned changing that might break if someone is using it with that assumption.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] Add Table field to ObservedQuery and ObservedBatch for table-level observability [cassandra-gocql-driver]
Posted by "worryg0d (via GitHub)" <gi...@apache.org>.
worryg0d commented on code in PR #1941:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1941#discussion_r3309884860
##########
session.go:
##########
@@ -2374,6 +2374,14 @@ type ObservedQuery struct {
Keyspace string
Statement string
+ // StatementKeyspace is the keyspace of the table targeted by the query.
+ // It is populated from prepared statement metadata returned by Cassandra.
+ StatementKeyspace string
Review Comment:
Yeah, this is why I filed CASSGO-120, I'm unsure if this is the correct behavior. At least I was expecting it to be based on the query, if it is presented there
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] Add Table field to ObservedQuery and ObservedBatch for table-level observability [cassandra-gocql-driver]
Posted by "joao-r-reis (via GitHub)" <gi...@apache.org>.
joao-r-reis commented on code in PR #1941:
URL: https://github.com/apache/cassandra-gocql-driver/pull/1941#discussion_r3102041604
##########
CHANGELOG.md:
##########
@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [2.1.1]
+### Added
Review Comment:
Add a new 2.2.0 section and move this under there
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org