Kafka add gauge v1 #33408

Naireen · 2024-12-17T23:00:23Z

Add per worker gauge support to add per backlog partition for kafka with java legacy worker for Dataflow

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
Update CHANGES.md with noteworthy changes.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.

codecov · 2024-12-18T00:02:39Z

Codecov Report

Attention: Patch coverage is 50.00000% with 43 lines in your changes missing coverage. Please review.

Project coverage is 60.42%. Comparing base (edc4766) to head (949f3f5).
Report is 45 commits behind head on master.

Files with missing lines	Patch %	Lines
...a/org/apache/beam/sdk/metrics/DelegatingGauge.java	0.00%	19 Missing ⚠️
...dataflow/worker/StreamingStepMetricsContainer.java	23.07%	10 Missing ⚠️
...ker/MetricsToPerStepNamespaceMetricsConverter.java	86.95%	3 Missing and 3 partials ⚠️
...in/java/org/apache/beam/sdk/metrics/NoOpGauge.java	0.00%	5 Missing ⚠️
...ners/dataflow/worker/DataflowMetricsContainer.java	0.00%	2 Missing ⚠️
.../org/apache/beam/sdk/metrics/MetricsContainer.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             master   #33408       +/-   ##
=============================================
+ Coverage     57.47%   60.42%    +2.94%     
- Complexity     1474    15172    +13698     
=============================================
  Files           985     2760     +1775     
  Lines        155802   267597   +111795     
  Branches       1076    12161    +11085     
=============================================
+ Hits          89550   161700    +72150     
- Misses        64035    99435    +35400     
- Partials       2217     6462     +4245

Flag	Coverage Δ
java	`64.81% <50.00%> (-3.78%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Naireen · 2024-12-18T04:18:38Z

R: @sjvanrossum for the kafka io part, thanks in advance!

github-actions · 2024-12-18T04:19:55Z

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment assign set of reviewers

Naireen · 2024-12-18T16:50:20Z

Run Java PreCommit

Naireen · 2024-12-18T17:56:46Z

R: @johnjcasey for the sdk portion of it.

.../java/org/apache/beam/runners/dataflow/worker/MetricsToPerStepNamespaceMetricsConverter.java

Naireen · 2024-12-19T19:48:13Z

Run Java_Pulsar_IO_Direct PreCommit

Naireen · 2024-12-19T20:57:30Z

Run Java PreCommit

Naireen · 2024-12-19T20:57:43Z

Run Java_GCP_IO_Direct PreCommit

Naireen · 2024-12-19T21:32:27Z

Run Java PreCommit

Naireen · 2024-12-19T21:53:22Z

Run Java PreCommit

Naireen · 2024-12-19T21:53:31Z

Run Java_GCP_IO_Direct PreCommit

Naireen · 2024-12-19T21:57:51Z

Run Java_GCP_IO_Direct PreCommit

Naireen · 2024-12-19T22:39:42Z

Run Java PreCommit

sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/NoOpGauge.java

...a/org/apache/beam/runners/dataflow/worker/MetricsToPerStepNamespaceMetricsConverterTest.java

sjvanrossum · 2025-01-07T23:41:54Z

sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaMetrics.java

+    /**
+     * @param topicName topicName
+     * @param partitionId partitionId for the topic Only included in the metric key if
+     *     'supportsMetricsDeletion' is enabled.
+     * @param backlog backlog for the topic Only included in the metric key if
+     *     'supportsMetricsDeletion' is enabled.
+     */


" Only" -> ". Only"?

Removed, thanks for catching that.

sjvanrossum · 2025-01-07T23:49:51Z

sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaMetrics.java

@@ -71,11 +79,17 @@ abstract class KafkaMetricsImpl implements KafkaMetrics {

    abstract HashMap<String, ConcurrentLinkedQueue<Duration>> perTopicRpcLatencies();

+    static ConcurrentHashMap<String, Gauge> backlogGauges = new ConcurrentHashMap<String, Gauge>();
+
+    abstract HashMap<String, Long> perTopicPartitionBacklogs();


If an instance of this class may be concurrently updated, then HashMap needs to be replaced (ditto for the existing HashMap fields). Use ConcurrentHashMap instead.

Slightly unrelated, but why doesn't perTopicRpcLatencies use a gauge or sum as the value type?

What would the sum represent? the sum of latencies? but each individual one is important, and a sum would lose information.
A gauge isn't quite clear either, if you have two concurrent rpcs that completed, what value do you return?

A histogram of values provides more information (and allows us to see the spread of values)

sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaSinkMetrics.java

sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaUnboundedReader.java

sjvanrossum · 2025-01-08T00:11:22Z

sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaUnboundedReader.java

@@ -743,6 +747,16 @@ private void reportBacklog() {
    backlogElementsOfSplit.set(splitBacklogMessages);
  }

+  private void reportBacklogMetrics() {


Looks like this can be merged with reportBacklog (potentially rename that method to reportBacklogMetrics updateBacklogMetrics).

No, I explicitly moved it out to be separate, since reportBacklog() is called twice, and we only need to do this once (when we advance to the next record).

Naireen · 2025-01-17T19:10:43Z

Run Java_IOs_Direct PreCommit

Naireen · 2025-01-21T19:26:45Z

Run Java PreCommit

Naireen · 2025-01-22T08:01:56Z

Run Java_Hadoop_IO_Direct PreCommit

Naireen · 2025-01-22T17:36:34Z

Run Java PreCommit

Naireen · 2025-01-23T07:00:59Z

Run Java PreCommit

Naireen · 2025-01-23T17:42:27Z

Run Java_IOs_Direct PreCommit

Naireen · 2025-01-23T17:56:25Z

Run Java PreCommit

* add counter stuff * Address John's comments about separting conversion and validation checks * address Steven's comments * another round of comments --------- Co-authored-by: Naireen <[email protected]>

github-actions bot added java build io runners dataflow kafka labels Dec 17, 2024

Naireen force-pushed the kafka_add_counters_V1 branch 4 times, most recently from 2791b23 to b91a77f Compare December 18, 2024 01:21

Naireen marked this pull request as ready for review December 18, 2024 04:18

Naireen force-pushed the kafka_add_counters_V1 branch from b91a77f to 948dfe6 Compare December 18, 2024 07:34

johnjcasey requested changes Dec 19, 2024

View reviewed changes

.../java/org/apache/beam/runners/dataflow/worker/MetricsToPerStepNamespaceMetricsConverter.java Outdated Show resolved Hide resolved

Naireen force-pushed the kafka_add_counters_V1 branch from 11bee27 to b9d2f2b Compare December 19, 2024 19:54

johnjcasey approved these changes Dec 19, 2024

View reviewed changes

Naireen mentioned this pull request Jan 6, 2025

Kafka add counters v1 uw2 #33503

Draft

3 tasks

sjvanrossum suggested changes Jan 8, 2025

View reviewed changes

Naireen added 2 commits January 17, 2025 18:23

Address John's comments about separting conversion and validation checks

e5cad4e

address Steven's comments

f02a194

Naireen force-pushed the kafka_add_counters_V1 branch from f33b235 to 925290b Compare January 17, 2025 18:23

github-actions bot removed python go website docker extensions learning gcp jdbc yaml prism labels Jan 17, 2025

Naireen force-pushed the kafka_add_counters_V1 branch 2 times, most recently from 5baabd8 to 11137f9 Compare January 21, 2025 18:32

Naireen force-pushed the kafka_add_counters_V1 branch from 11137f9 to 099ab06 Compare January 22, 2025 07:50

Naireen force-pushed the kafka_add_counters_V1 branch 2 times, most recently from 7e1ad4c to 70a5f63 Compare January 22, 2025 19:56

Naireen force-pushed the kafka_add_counters_V1 branch from 70a5f63 to 949f3f5 Compare January 23, 2025 17:09

another round of comments

fc42673

Naireen force-pushed the kafka_add_counters_V1 branch from 949f3f5 to fc42673 Compare January 24, 2025 01:27

johnjcasey merged commit 2064103 into apache:master Jan 24, 2025
28 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kafka add gauge v1 #33408

Kafka add gauge v1 #33408

Naireen commented Dec 17, 2024

codecov bot commented Dec 18, 2024 •

edited

Loading

Naireen commented Dec 18, 2024

github-actions bot commented Dec 18, 2024

Naireen commented Dec 18, 2024

Naireen commented Dec 18, 2024

Naireen commented Dec 19, 2024

Naireen commented Dec 19, 2024

Naireen commented Dec 19, 2024

Naireen commented Dec 19, 2024

Naireen commented Dec 19, 2024

Naireen commented Dec 19, 2024

Naireen commented Dec 19, 2024

Naireen commented Dec 19, 2024

sjvanrossum Jan 7, 2025

Naireen Jan 9, 2025

sjvanrossum Jan 7, 2025

sjvanrossum Jan 8, 2025

Naireen Jan 9, 2025

sjvanrossum Jan 8, 2025

Naireen Jan 9, 2025

sjvanrossum Jan 17, 2025

Naireen commented Jan 17, 2025

Naireen commented Jan 21, 2025

Naireen commented Jan 22, 2025

Naireen commented Jan 22, 2025

Naireen commented Jan 23, 2025

Naireen commented Jan 23, 2025

Naireen commented Jan 23, 2025

Kafka add gauge v1 #33408

Kafka add gauge v1 #33408

Conversation

Naireen commented Dec 17, 2024

Add per worker gauge support to add per backlog partition for kafka with java legacy worker for Dataflow

GitHub Actions Tests Status (on master branch)

codecov bot commented Dec 18, 2024 • edited Loading

Codecov Report

Naireen commented Dec 18, 2024

github-actions bot commented Dec 18, 2024

Naireen commented Dec 18, 2024

Naireen commented Dec 18, 2024

Naireen commented Dec 19, 2024

Naireen commented Dec 19, 2024

Naireen commented Dec 19, 2024

Naireen commented Dec 19, 2024

Naireen commented Dec 19, 2024

Naireen commented Dec 19, 2024

Naireen commented Dec 19, 2024

Naireen commented Dec 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Naireen commented Jan 17, 2025

Naireen commented Jan 21, 2025

Naireen commented Jan 22, 2025

Naireen commented Jan 22, 2025

Naireen commented Jan 23, 2025

Naireen commented Jan 23, 2025

Naireen commented Jan 23, 2025

codecov bot commented Dec 18, 2024 •

edited

Loading