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

MINOR: Fix docs for end-to-end record latency metrics #10449

Merged

Conversation

cadonna
Copy link
Contributor

@cadonna cadonna commented Mar 31, 2021

Committer Checklist (excluded from commit message)

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

Copy link
Contributor Author

@cadonna cadonna left a comment

Choose a reason for hiding this comment

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

@ableegoldman During the preparation of my talk about Kafka Streams metrics, I noticed the following mistake in PR #10251. The e2e record latency metrics are processor node level metrics. So they were in the wrong section. The bean regex was also wrong since the regex depends on the level.
Sorry that I had not found the time to review #10251

@@ -2586,7 +2574,22 @@ <h5 class="anchor-heading"><a id="kafka_streams_node_monitoring" class="anchor-l
<td>The total number of records that have been emitted downstream from suppression operation nodes.</td>
<td>kafka.streams:type=stream-processor-node-metrics,thread-id=([-.\w]+),task-id=([-.\w]+),processor-node-id=([-.\w]+)</td>
</tr>
</tbody>
<tr>
<td>record-e2e-latency-avg</td>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically, the record e2e latency metrics are processor node level metrics that are only available on certain types of nodes. Since the bean regex depends on the level (i.e., the metric group) the regex was wrong before.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, thanks. Tbh I had forgotten how we ultimately ended up doing these metrics, thanks for catching this

@@ -2534,31 +2534,19 @@ <h5 class="anchor-heading"><a id="kafka_streams_task_monitoring" class="anchor-l
</tr>
<tr>
<td>active-process-ratio</td>
<td>The total number of records dropped within this task.</td>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes a copy and paste error coming from the dropped-records-* metrics.

Copy link
Contributor

@ableegoldman ableegoldman left a comment

Choose a reason for hiding this comment

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

LGTM

@ableegoldman
Copy link
Contributor

Unrelated failures in kafka.server.RaftClusterTest

@ableegoldman ableegoldman merged commit 58d4e52 into apache:trunk Mar 31, 2021
Terrdi pushed a commit to Terrdi/kafka that referenced this pull request Apr 1, 2021
Reviewers: Anna Sophie Blee-Goldman <ableegoldman@apache.org>
@cadonna cadonna deleted the fix_e2e_record_latency_metrics_docs branch April 1, 2021 11:55
ijuma added a commit to ijuma/kafka that referenced this pull request Apr 4, 2021
…e-allocations-lz4

* apache-github/trunk: (243 commits)
  KAFKA-12590: Remove deprecated kafka.security.auth.Authorizer, SimpleAclAuthorizer and related classes in 3.0 (apache#10450)
  KAFKA-3968: fsync the parent directory of a segment file when the file is created (apache#10405)
  KAFKA-12283: disable flaky testMultipleWorkersRejoining to stabilize build (apache#10408)
  MINOR: remove KTable.to from the docs (apache#10464)
  MONOR: Remove redudant LocalLogManager (apache#10325)
  MINOR: support ImplicitLinkedHashCollection#sort (apache#10456)
  KAFKA-12587 Remove KafkaPrincipal#fromString for 3.0 (apache#10447)
  KAFKA-12426: Missing logic to create partition.metadata files in RaftReplicaManager (apache#10282)
  MINOR: Improve reproducability of raft simulation tests (apache#10422)
  KAFKA-12474: Handle failure to write new session keys gracefully (apache#10396)
  KAFKA-12593: Fix Apache License headers (apache#10452)
  MINOR: Fix typo in MirrorMaker v2 documentation (apache#10433)
  KAFKA-12600: Remove deprecated config value `default` for client config `client.dns.lookup` (apache#10458)
  KAFKA-12952: Remove deprecated LogConfig.Compact (apache#10451)
  Initial commit (apache#10454)
  KAFKA-12575: Eliminate Log.isLogDirOffline boolean attribute (apache#10430)
  KAFKA-8405; Remove deprecated `kafka-preferred-replica-election` command (apache#10443)
  MINOR: Fix docs for end-to-end record latency metrics (apache#10449)
  MINOR Replaced File with Path in LogSegmentData. (apache#10424)
  KAFKA-12583: Upgrade netty to 4.1.62.Final
  ...
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.

2 participants