/cockroach

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,telemetry: fold unimplemented/error telemetry into feature counters #31356

Merged
merged 2 commits into from Oct 15, 2018

Conversation

Reviewers

@dt dt

@cockroachdb/core-prs core-prs

@cockroachdb/sql-execution-prs sql-execution-prs

@cockroachdb/sql-planning-prs sql-planning-prs

Assignees
No one assigned
Labels
None yet
Projects
  
Finished (milestone r2.1) in SQL Front-end, Lang & Semantics
Milestone
No milestone
3 participants
@knz @cockroach-teamcity @dt
@knz
Member

knz commented Oct 15, 2018

Forked off #31332.
@knz
telemetry: minor optimization 
Release note: None
6f72a17

@knz knz requested a review from dt Oct 15, 2018

@knz knz added this to Triage in SQL Front-end, Lang & Semantics via automation Oct 15, 2018

@knz knz requested review from cockroachdb/core-prs as code owners Oct 15, 2018

@cockroach-teamcity

This comment has been minimized.

Show comment
Hide comment
@cockroach-teamcity

cockroach-teamcity Oct 15, 2018

Member

This change is Reviewable
Member

cockroach-teamcity commented Oct 15, 2018

This change is Reviewable

@knz knz referenced this pull request Oct 15, 2018

Merged

sql: report unimplemented vtables in telemetry #31332

@knz knz moved this from Triage to Current milestone in SQL Front-end, Lang & Semantics Oct 15, 2018

@knz knz referenced this pull request Oct 15, 2018

Merged

release-2.1: assorted SQL telemetry fixes #31357

@dt

dt approved these changes Oct 15, 2018
View changes

Reviewed 1 of 1 files at r1, 1 of 7 files at r2.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

pkg/sql/conn_executor.go, line 316 at r2 (raw file):

	if pgErr, ok := pgerror.GetPGCause(err); ok {
		telemetry.Count("pgerror." + pgErr.Code)

During recording in the telemetry collection server, when reported in the error count field, these are currently written to the general usage tracking table with the prefix prefix "errorcodes." .

If we want to change that to pgerror, we should change the collector server's code too, and probably update all collected data as well.

https://github.com/cockroachlabs/registration/blob/master/db.go#L222

@knz
sql: fold unimplemented/error telemetry into feature counters 
This was discussed with @dt: use the feature counter machinery
instead of using separate maps. This simplifies the code.

Release note: None
Loading status checks…
7d9920e
@knz

This comment has been minimized.

Show comment
Hide comment
@knz

knz Oct 15, 2018

Member

done renaming prefix

bors r+
Member

knz commented Oct 15, 2018

done renaming prefix

bors r+

craig bot pushed a commit that referenced this pull request Oct 15, 2018

craig[bot] @knz
Merge #31333 #31356 
31333: pgerror: hint what to do upon encountering unimplemented errors r=knz a=knz

Requested by @awoods187 . Inspired by @tschottdorf 

Before:

```
root@127.0.0.1:28070/defaultdb> select * from information_schema.view_table_usage;
pq: virtual schema table not implemented: information_schema.view_table_usage
```

After:

```
root@127.0.0.1:28070/defaultdb> select * from information_schema.view_table_usage;
pq: virtual schema table not implemented: information_schema.view_table_usage
HINT: This feature is not yet implemented in CockroachDB.

Please check https://github.com/cockroachdb/cockroach/issues to check
whether this feature is already tracked. If you cannot find it there,
please report this error with example query at:

    https://github.com/cockroachdb/cockroach/issues/new/choose

If you would rather not post publicly, please contact us directly at:

    support@cockroachlabs.com

The Cockroach Labs team appreciates your feedback.
```

Release note: None

31356: sql,telemetry: fold unimplemented/error telemetry into feature counters r=knz a=knz

Forked off  #31332.


Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
Loading status checks…
b74a347
@craig

This comment has been minimized.

Show comment
Hide comment
@craig

craig bot Oct 15, 2018

Build succeeded

craig bot commented Oct 15, 2018

Build succeeded

@craig craig bot merged commit 7d9920e into cockroachdb:master Oct 15, 2018
2 of 3 checks passed

2 of 3 checks passed

GitHub CI (Cockroach) TeamCity build started
Details
bors Build succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@knz knz deleted the knz:20181015-telemetry branch Oct 15, 2018

craig bot pushed a commit that referenced this pull request Oct 15, 2018

craig[bot] @knz
Merge #31332 
31332: sql: report unimplemented vtables in telemetry r=knz a=knz

Requested by @awoods187 
First two commits from  #31356.

Release note (sql change): CockroachDB will now collect references to
tables in `information_schema` and `pg_catalog` which are not yet
implemented, and report them as telemetry if statistics reporting is
enabled. This will help determine which features should be implemented
next for compatibility.

Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
Loading status checks…
93a48b3

@knz knz moved this from Current milestone to Finished (milestone r2.1) in SQL Front-end, Lang & Semantics Oct 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment