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: expose feature counters via crdb_internal.feature_usage #31334

Open
wants to merge 4 commits into
base: master
from

Conversation

4 participants
@knz
Member

knz commented Oct 13, 2018

First 3 commits from #31332.
Only last commit in this PR; will rebase when the other is merged.

I found myself using this extensively to work on that other PR and related PRs adding markers for unimplemented features and new feature counts.

knz added some commits Oct 13, 2018

telemetry: minor optimization
Release note: None
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
sql: report unimplemented vtables in telemetry
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.

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

@knz knz requested a review from bobvawter Oct 13, 2018

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

@cockroach-teamcity

This comment has been minimized.

Show comment
Hide comment
@cockroach-teamcity

cockroach-teamcity Oct 13, 2018

Member

This change is Reviewable

Member

cockroach-teamcity commented Oct 13, 2018

This change is Reviewable

@knz

This comment has been minimized.

Show comment
Hide comment
@knz

knz Oct 13, 2018

Member

cc @dt

Member

knz commented Oct 13, 2018

cc @dt

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

@bobvawter

:lgtm:

Reviewed 1 of 1 files at r1, 7 of 7 files at r2, 5 of 5 files at r3, 6 of 6 files at r4.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale)


pkg/server/telemetry/features.go, line 96 at r2 (raw file):

// inspection via SQL. They are not quantized! Thus not suitable for
// reporting.
func GetFeatureCounts() map[string]int32 {

Maybe GetFeatureCountsForDisplay() to make the intended use more obvious if you don't read the doc?


pkg/sql/logictest/testdata/logic_test/crdb_internal, line 140 at r4 (raw file):


query TT colnames
SELECT * FROM crdb_internal.feature_usage WHERE feature_name = ''

Might it make sense to select count(usage_count) from feature_usage where feature_name = 'something' yields 1 to ensure that the wiring from telemetry to the virtual table is functioning? I see why you wouldn't want to query for a specific value, but this doesn't verify that the populate struct member is actually populating anything.

@dt

dt approved these changes Oct 16, 2018

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