Skip to content

KAFKA-7963: Extract hard-coded Streams metric name strings to centralized place#6355

Merged
bbejeck merged 1 commit intoapache:trunkfrom
ableegoldman:ExtractMetricsStrings
Mar 6, 2019
Merged

KAFKA-7963: Extract hard-coded Streams metric name strings to centralized place#6355
bbejeck merged 1 commit intoapache:trunkfrom
ableegoldman:ExtractMetricsStrings

Conversation

@ableegoldman
Copy link
Copy Markdown
Member

Moved hard-coded 'expired-window-record-drop' and 'late-record-drop' to static Strings in StreamsMetricsImpl

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@ableegoldman
Copy link
Copy Markdown
Member Author

@bbejeck bbejeck added the streams label Mar 2, 2019
@bbejeck bbejeck requested review from guozhangwang and mjsax March 2, 2019 00:26
@bbejeck
Copy link
Copy Markdown
Member

bbejeck commented Mar 2, 2019

\cc @vvcephei

@ableegoldman
Copy link
Copy Markdown
Member Author

First pass found only these two strings appearing in more than one file (in the context of StreamsMetricsImpl#XXLevelSensor() calls) but there may be others worth including in this scope..?

@ableegoldman
Copy link
Copy Markdown
Member Author

Failures unrelated: retest this, please

Copy link
Copy Markdown
Member

@bbejeck bbejeck left a comment

Choose a reason for hiding this comment

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

LGTM

@mjsax
Copy link
Copy Markdown
Member

mjsax commented Mar 5, 2019

Known flaky test failed: https://issues.apache.org/jira/browse/KAFKA-7978

Retest this please.

@guozhangwang
Copy link
Copy Markdown
Contributor

LGTM.

@bbejeck please feel free to merge.

One side note that after we merged #6080 maybe we can do another follow-up to move the constant string names to the StreamsMetricsImpl class as well and let test classes as well as internal classes like MeteredXXStores reference those strings instead.

@bbejeck bbejeck merged commit 14d97aa into apache:trunk Mar 6, 2019
@bbejeck
Copy link
Copy Markdown
Member

bbejeck commented Mar 6, 2019

Merged #6355 into trunk

@ableegoldman ableegoldman deleted the ExtractMetricsStrings branch March 14, 2019 18:44
@ableegoldman ableegoldman restored the ExtractMetricsStrings branch March 14, 2019 18:44
@ableegoldman ableegoldman deleted the ExtractMetricsStrings branch March 15, 2019 23:57
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
…apache#6355)

Moved hard-coded 'expired-window-record-drop' and 'late-record-drop' to static Strings in StreamsMetricsImpl

Reviewers: Guozhang Wang <wangguoz@gmail.com>, Matthias J. Sax <mjsax@apache.org>,  Bill Bejeck <bbejeck@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants