je-ik commented on code in PR #39341:
URL: https://github.com/apache/beam/pull/39341#discussion_r3596004582
##########
runners/kafka-streams/src/main/java/org/apache/beam/runners/kafka/streams/translation/ExecutableStageProcessor.java:
##########
@@ -179,11 +190,25 @@ public <OutputT> FnDataReceiver<OutputT> create(String
pCollectionId) {
};
}
};
+ // Fold the harness's reported metrics into this stage's container when
each bundle completes.
+ // Only the completion response is applied: it carries the bundle's final
cumulative values, and
+ // the container's update() adds counter values, so also applying
mid-bundle progress snapshots
+ // would double-count them. Live mid-bundle metrics can come later if a
use appears.
+ BundleProgressHandler progressHandler =
+ new BundleProgressHandler() {
+ @Override
+ public void onProgress(ProcessBundleProgressResponse progress) {
+ // Deliberately not folded into the container; see comment above.
+ }
+
+ @Override
+ public void onCompleted(ProcessBundleResponse response) {
Review Comment:
Same here, this might come handy in the future.
##########
runners/kafka-streams/src/main/java/org/apache/beam/runners/kafka/streams/translation/ExecutableStageProcessor.java:
##########
@@ -179,11 +190,25 @@ public <OutputT> FnDataReceiver<OutputT> create(String
pCollectionId) {
};
}
};
+ // Fold the harness's reported metrics into this stage's container when
each bundle completes.
+ // Only the completion response is applied: it carries the bundle's final
cumulative values, and
+ // the container's update() adds counter values, so also applying
mid-bundle progress snapshots
+ // would double-count them. Live mid-bundle metrics can come later if a
use appears.
+ BundleProgressHandler progressHandler =
+ new BundleProgressHandler() {
+ @Override
+ public void onProgress(ProcessBundleProgressResponse progress) {
+ // Deliberately not folded into the container; see comment above.
Review Comment:
We might log the response here on DEBUG level using TextFormat and the level
check, i.e.
```java
if (log.isDebugEnabled()) {
log.debug(....);
}
```
##########
runners/kafka-streams/src/main/java/org/apache/beam/runners/kafka/streams/translation/KafkaStreamsTranslationContext.java:
##########
@@ -45,6 +46,10 @@ public class KafkaStreamsTranslationContext {
private final KafkaStreamsPipelineOptions pipelineOptions;
private final Topology topology;
private final Map<String, String> pCollectionIdToProcessorName;
+ // Accumulates the Beam metrics reported by the SDK harness, one container
per executable stage.
+ // Processors update it as bundles complete (in-JVM reference sharing); the
pipeline result
+ // exposes it as MetricResults.
+ private final MetricsContainerStepMap metricsContainerStepMap = new
MetricsContainerStepMap();
Review Comment:
Yeah, the aggregation will have to come via some external tool, e.g.
Prometheus.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
