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

ddl, metrics: remove DDL version in metrics #7737

Merged
merged 3 commits into from
Sep 23, 2018

Conversation

zimulala
Copy link
Contributor

@zimulala zimulala commented Sep 18, 2018

What problem does this PR solve?

Related #7472
When the updating schema is frequent, the DDL version will be very much. But we'd better not overuse labels in prometheus. Refer to https://prometheus.io/docs/practices/instrumentation/#do-not-overuse-labels

What is changed and how it works?

Remove the label of the version in metrics.

Check List

Tests

  • No code

ddl/syncer.go Outdated
@@ -325,8 +324,7 @@ func (s *schemaVersionSyncer) OwnerCheckAllVersions(ctx context.Context, latestV

var err error
defer func() {
ver := strconv.FormatInt(latestVer, 10)
metrics.OwnerHandleSyncerHistogram.WithLabelValues(metrics.OwnerGetGlobalVersion, ver, metrics.RetLabel(err)).Observe(time.Since(startTime).Seconds())
metrics.OwnerHandleSyncerHistogram.WithLabelValues(metrics.OwnerGetGlobalVersion, metrics.RetLabel(err)).Observe(time.Since(startTime).Seconds())
Copy link
Contributor

Choose a reason for hiding this comment

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

s/OwnerGetGlobalVersion/OwnerCheckAllVersions ?

@zimulala
Copy link
Contributor Author

PTAL @crazycs520 @ciscoxll @winkyao

@zimulala
Copy link
Contributor Author

/run-all-tests

Copy link
Contributor

@lysu lysu left a comment

Choose a reason for hiding this comment

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

LGTM

@lysu lysu added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 21, 2018
@shenli
Copy link
Member

shenli commented Sep 23, 2018

LGTM

@shenli shenli added status/LGT2 Indicates that a PR has LGTM 2. status/all tests passed and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 23, 2018
@shenli shenli merged commit 16864f9 into pingcap:master Sep 23, 2018
@zimulala zimulala deleted the metrics-ver branch November 8, 2018 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/metrics status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants