ajamato commented on a change in pull request #13429:
URL: https://github.com/apache/beam/pull/13429#discussion_r533823494
##########
File path: sdks/python/apache_beam/metrics/cells.py
##########
@@ -77,6 +79,13 @@ def reset(self):
# type: () -> None
raise NotImplementedError
+ @property
+ def start_time(self):
Review comment:
I tried doing this, but couldn't make it simpler. I was going to make
the base class return an empty MonitoringInfo except for setting the start
time, then updating it. But since the implementations in the subcalsses are
calling into functions which return a whole monitoring_info (rather than
setting fields on one passed in), I found what I could produce wasn't actually
cleaner.
i.e. calling
mi = monitoring_infos.int64_user_counter(
name.namespace,
name.name,
self.get_cumulative(),
ptransform=transform_id)
If you had something else in mind, please provide a snippet, and ill see
what i can do
##########
File path: model/pipeline/src/main/proto/metrics.proto
##########
@@ -401,6 +402,17 @@ message MonitoringInfo {
// as Stackdriver will be able to aggregate the metrics using a subset of the
// provided labels
map<string, string> labels = 4;
+
+ // This indicates when the first value was populated by the SDK Harness.
Review comment:
Done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
