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

release-20.2: backupccl: add more backup telemetry #54863

Merged
merged 2 commits into from
Sep 29, 2020

Conversation

pbardea
Copy link
Contributor

@pbardea pbardea commented Sep 28, 2020

Backport:

Please see individual PRs for details.

/cc @cockroachdb/release

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@pbardea pbardea mentioned this pull request Sep 28, 2020
40 tasks
@pbardea pbardea requested a review from dt September 28, 2020 18:58
@pbardea pbardea marked this pull request as ready for review September 28, 2020 18:58
@pbardea pbardea requested a review from a team September 28, 2020 18:58
// countSource emits a telemetry counter and also adds a ".scheduled"
// suffix if the job was created by a schedule.
countSource := func(feature string) {
telemetry.Count(feature + sourceSuffix)
Copy link
Member

Choose a reason for hiding this comment

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

hm. I wonder if we should keep the global counts for all backups -- scheduled or manual -- and have an additional counter for scheduled? otherwise I wonder if it is prone to forgetting to sum the two counters any time we look at this. I guess a question for @mwang1026

Choose a reason for hiding this comment

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

Subtraction is hard in Looker but addition is easy. If we did go with a global counter, I'd want separate ones for manual and scheduled as well. Thinking through, I personally prefer the latter -- a global counter, and then a split one with manual and scheduled separate. That way dashboards won't break as we transition versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just chatted with @mwang1026, it sounds like we can search by a prefix. I think it would be worthwhile updating the default prefix to ".manual" so that there is a way to easily filter telemetry by scheduled and non-scheduled backups.

Put out #54952 for this change.

This commit adds telemetry in backup for the type of encryption used in
backups, as well as if the new INTO syntax was used. In addition, it
also adds telemetry to report which backup features are being used by
scheduled backups.

Release note: None
This commit updates the telemetry for backup to always include the
source (either ".manual" or ".scheduled"). This allows for folks to
search by a prefix to get the telemetry for all backups, and the
specific source can be appened to filter the telemetry between manual
and scheduled backups.

Release note: None
@pbardea
Copy link
Contributor Author

pbardea commented Sep 29, 2020

Updated with #54952.

@pbardea pbardea requested a review from dt September 29, 2020 19:48
@pbardea pbardea merged commit 017bb2b into cockroachdb:release-20.2 Sep 29, 2020
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.

4 participants