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
sql: equip scalar expressions with telemetry #35616
Conversation
b654789
to
2696ca5
Compare
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.
Would it be possible to get a dump of all generated counter names, just to be able to give it a quick eyeball?
Reviewed 1 of 1 files at r1, 8 of 8 files at r2.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @dt and @knz)
pkg/sql/sem/tree/operators.go, line 33 at r2 (raw file):
// Scalar operators will be counter as sql.ops.<kind>.<lhstype> <opname> <rhstype>. const opCounterNameFmt = "sql.ops.%s.%s"
The constant value doesn't seem to agree with the comment above.
pkg/sql/sem/tree/overload.go, line 58 at r2 (raw file):
// Counter, if non-nil, should be incremented upon successful // type check of expressions using this overload. Counter telemetry.Counter
Counter isn't exported on the other types in sem/tree
, does it need to be?
Release note: None
It's pretty big (~1300 entries), I made a list here: |
2696ca5
to
cae7689
Compare
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.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @bobvawter and @dt)
pkg/sql/sem/tree/operators.go, line 33 at r2 (raw file):
Previously, bobvawter (Bob Vawter) wrote…
The constant value doesn't seem to agree with the comment above.
Clarified in comment.
pkg/sql/sem/tree/overload.go, line 58 at r2 (raw file):
Previously, bobvawter (Bob Vawter) wrote…
Counter isn't exported on the other types in
sem/tree
, does it need to be?
No it doesn't. Good catch!
TFYR! bors r+ |
Canceled (will resume) |
bors r+ |
Not awaiting review |
This patch introduces telemetry for the following scalar expressions: - `array[...]`, `array(...)`, `...[...]` telemetry: `sql.ops.array.cons`, `sql.ops.array.flatten`, `sql.ops.array.ind` - `IFERROR(...)` telemetry: `sql.ops.iferr` - unary, binary and comparison operators; telemetry: `sql.ops.un.<op> <type>`, `sql.ops.bin.<type> <op> <type>`, `sql.ops.cmp.<type> <op> <type>` - casts; telemetry: `sql.ops.cast.<type>::<type>`, `sql.ops.cast.arrays` - built-in function applications. telemetry: `sql.builtins.<name><signature>` For example: ``` > select ('{"a":{"c":"d"},"e":123}'::string::jsonb #- array['a']) - 'e'; > select * from crdb_internal.feature_usage where feature_name like '%json%'; feature_name | usage_count +---------------------------------------------------------------------+-------------+ sql.builtins.json_remove_path.(val: jsonb, path: string[]) -> jsonb | 1 sql.ops.cast.text::jsonb | 1 sql.ops.bin.jsonb - text | 1 (3 rows) ``` Release note (sql change): CockroachDB will now report usage frequency of various SQL scalar operators in telemetry, when telemetry is enabled, so as to guide future optimizations of query performance.
cae7689
to
8d01ef7
Compare
Canceled |
bors r+ |
35616: sql: equip scalar expressions with telemetry r=knz a=knz Informs https://github.com/cockroachlabs/registration/issues/203. This patch introduces telemetry for the following scalar expressions: - `array[...]`, `array(...)`, `...[...]` telemetry: `sql.ops.array.cons`, `sql.ops.array.flatten`, `sql.ops.array.ind` - `IFERROR(...)` telemetry: `sql.ops.iferr` - unary, binary and comparison operators; telemetry: `sql.ops.un.<op> <type>`, `sql.ops.bin.<type> <op> <type>`, `sql.ops.cmp.<type> <op> <type>` - casts; telemetry: `sql.ops.cast.<type>::<type>`, `sql.ops.cast.arrays` - built-in function applications. telemetry: `sql.builtins.<name><signature>` For example: ``` > select ('{"a":{"c":"d"},"e":123}'::string::jsonb #- array['a']) - 'e'; > select * from crdb_internal.feature_usage where feature_name like '%json%'; feature_name | usage_count +---------------------------------------------------------------------+-------------+ sql.builtins.json_remove_path.(val: jsonb, path: string[]) -> jsonb | 1 sql.ops.cast.text::jsonb | 1 sql.ops.bin.jsonb - text | 1 (3 rows) ``` Release note (sql change): CockroachDB will now report usage frequency of various SQL scalar operators in telemetry, when telemetry is enabled, so as to guide future optimizations of query performance. Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
Build succeeded |
Informs https://github.com/cockroachlabs/registration/issues/203.
This patch introduces telemetry for the following scalar expressions:
array[...]
,array(...)
,...[...]
telemetry:
sql.ops.array.cons
,sql.ops.array.flatten
,sql.ops.array.ind
IFERROR(...)
telemetry:
sql.ops.iferr
unary, binary and comparison operators;
telemetry:
sql.ops.un.<op> <type>
,sql.ops.bin.<type> <op> <type>
,sql.ops.cmp.<type> <op> <type>
casts;
telemetry:
sql.ops.cast.<type>::<type>
,sql.ops.cast.arrays
built-in function applications.
telemetry:
sql.builtins.<name><signature>
For example:
Release note (sql change): CockroachDB will now report usage frequency
of various SQL scalar operators in telemetry, when telemetry is
enabled, so as to guide future optimizations of query performance.