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

etcdserver: strip patch version in cluster version metrics #11254

Merged

Conversation

jingyih
Copy link
Contributor

@jingyih jingyih commented Oct 14, 2019

@gyuho We discussed about stripping patch version in cluster version metrics.

Example output:

# TYPE etcd_cluster_version gauge
etcd_cluster_version{cluster_version="3.5"} 1

@jingyih jingyih requested a review from gyuho October 14, 2019 20:50
Copy link
Contributor

@gyuho gyuho left a comment

Choose a reason for hiding this comment

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

lgtm

@gyuho
Copy link
Contributor

gyuho commented Oct 14, 2019

@jpbetz @jingyih I think we can backport this to 3.4, as long as we highlight the change. What do you think? Including patch version in cluster version is very confusing anyways.

@gyuho
Copy link
Contributor

gyuho commented Oct 14, 2019

@jingyih Looks like some e2e tests are breaking from this change?

@jingyih
Copy link
Contributor Author

jingyih commented Oct 14, 2019

I am in favor of backporting to 3.4.

e2e metrics test failed due to this change, I will fix it.

Strip patch version in cluster version metrics.
@jingyih jingyih force-pushed the strip_patch_version_in_cluster_version_metrics branch from b86e008 to 1333abc Compare October 14, 2019 23:59
Copy link
Contributor

@gyuho gyuho left a comment

Choose a reason for hiding this comment

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

lgtm, yes then let's backport.

@gyuho gyuho merged commit bcc1471 into etcd-io:master Oct 15, 2019
@jpbetz
Copy link
Contributor

jpbetz commented Oct 15, 2019

The patch version was always 0 before this anyway, right? Just making sure I understand.

@jingyih
Copy link
Contributor Author

jingyih commented Oct 15, 2019

The patch version was always 0 before this anyway, right? Just making sure I understand.

This is also my understanding.

Also, in the log output we never included patch version in cluster version.

zap.String("cluster-version", version.Cluster(c.version.String())),

@jpbetz
Copy link
Contributor

jpbetz commented Oct 15, 2019 via email

gyuho added a commit that referenced this pull request Oct 15, 2019
…#11254-upstream-release-3.4

Automated cherry pick of #11233 #11254 on release 3.4
wenjiaswe added a commit to wenjiaswe/etcd that referenced this pull request Oct 16, 2019
wenjiaswe added a commit to wenjiaswe/etcd that referenced this pull request Oct 16, 2019
wenjiaswe added a commit to wenjiaswe/etcd that referenced this pull request Oct 16, 2019
wenjiaswe added a commit to wenjiaswe/etcd that referenced this pull request Oct 16, 2019
wenjiaswe added a commit to wenjiaswe/etcd that referenced this pull request Oct 16, 2019
wenjiaswe added a commit to wenjiaswe/etcd that referenced this pull request Oct 16, 2019
gyuho added a commit that referenced this pull request Oct 17, 2019
…257-upstream-release-3.3

cherry pick "etcd_cluster_version" metric" (#10257, #11233, #11254, #11265) to release-3.3
wenjiaswe added a commit to wenjiaswe/etcd that referenced this pull request Oct 17, 2019
gyuho added a commit that referenced this pull request Oct 17, 2019
…261-upstream-release-3.2

cherry pick "etcd_cluster_version" metric" (#10257, #11233, #11254, #11265) to release-3.2
hexfusion pushed a commit to openshift/etcd that referenced this pull request Mar 20, 2020
hexfusion pushed a commit to openshift/etcd that referenced this pull request Mar 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants