Skip to content

Commit fd0c3e4

Browse files
jrmccluskeygemini-code-assist[bot]damccorm
authored
Migrate to Google Cloud Dataflow Client (#37639)
* [WIP] Migrate to Google Cloud Dataflow Client rebase * Trigger relevant postcommits * base image update * fix camel case * update dataflow runner + tests * slide import to avoid triggering unit tests * yapf stuff * remove extra print * further spec structs, fix incorrect piplineUrl option, remove old client code * suppress line-too-longs * formatting * linting, tweak metrics tests * Proto-specific changes to metric processing tests * try to dump logging * handle more straightforward metrics values * add skips since the unit tests now depend on the proto library * testing if there's a disconnect between proto behavior locally and in tests * correct scalar access * clean up dist accesses * linting, various fixes * fix unit test setup for direct accesses * linting * more linting * re-enable histograms * Bump dataflow client version, restore pausing/paused concept * formatting * fix enum selection * handle the proto hash PR * remove unnecessary try/except block * re-delete old messsages * fix disk_provisioned_iops/throughput_mibps tests * code bot suggestions * revert pipeline options interaction * leftover update * yapf * swap credential loading * Fix message importance parsing * fix apiclient_test case w/ cloud deps and no auth * Local runner SSL fix * last local change * apitools test fix * yapf * Apply suggestions from code review Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * gemini suggestions * fix message importance * fix local runner case * remove legacy translation maps * revert container requirements changes * missing import * fix bad trigger file merge * Update sdks/python/apache_beam/runners/dataflow/dataflow_metrics.py Co-authored-by: Danny McCormick <dannymccormick@google.com> * fix merge problem with runnerv2 disabled tests * fix state breakage --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Danny McCormick <dannymccormick@google.com>
1 parent 54accc3 commit fd0c3e4

14 files changed

Lines changed: 506 additions & 10401 deletions
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
22
"comment": "Modify this file in a trivial way to cause this test suite to run.",
33
"pr": "38069",
4-
"modification": 42
4+
"modification": 43
55
}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
{
22
"comment": "Modify this file in a trivial way to cause this test suite to run",
3-
"modification": 2
3+
"modification": 3
44
}

sdks/python/apache_beam/runners/dataflow/dataflow_metrics.py

Lines changed: 17 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ def _get_match(proto, filter_fn):
5757

5858

5959
# V1b3 MetricStructuredName keys to accept and copy to the MetricKey labels.
60-
STEP_LABEL = 'step'
6160
STRUCTURED_NAME_LABELS = set(
6261
['execution_step', 'original_name', 'output_user_name'])
6362

@@ -112,9 +111,7 @@ def _translate_step_name(self, internal_name):
112111
try:
113112
step = _get_match(
114113
self._job_graph.proto.steps, lambda x: x.name == internal_name)
115-
user_step_name = _get_match(
116-
step.properties.additionalProperties,
117-
lambda x: x.key == 'user_name').value.string_value
114+
user_step_name = step.properties.get('user_name')
118115
except ValueError:
119116
pass # Exception is handled below.
120117
if not user_step_name:
@@ -135,24 +132,17 @@ def _get_metric_key(self, metric):
135132
# step name (only happens for unstructured-named metrics).
136133
# 2. Unable to unpack [step] or [namespace]; which should only happen
137134
# for unstructured names.
138-
step = _get_match(
139-
metric.name.context.additionalProperties,
140-
lambda x: x.key == STEP_LABEL).value
135+
step = metric.name.context['step']
141136
step = self._translate_step_name(step)
142-
except ValueError:
137+
except (KeyError, ValueError):
143138
pass
144139

145140
namespace = "dataflow/v1b3" # Try to extract namespace or add a default.
146-
try:
147-
namespace = _get_match(
148-
metric.name.context.additionalProperties,
149-
lambda x: x.key == 'namespace').value
150-
except ValueError:
151-
pass
141+
namespace = metric.name.context.get('namespace', 'dataflow/v1b3')
152142

153-
for kv in metric.name.context.additionalProperties:
154-
if kv.key in STRUCTURED_NAME_LABELS:
155-
labels[kv.key] = kv.value
143+
for key in metric.name.context:
144+
if key in STRUCTURED_NAME_LABELS:
145+
labels[key] = metric.name.context[key]
156146
# Package everything besides namespace and name the labels as well,
157147
# including unmodified step names to assist in integration the exact
158148
# unmodified values which come from dataflow.
@@ -185,10 +175,7 @@ def _populate_metrics(self, response, result, user_metrics=False):
185175
# in the service.
186176
# The second way is only useful for the UI, and should be ignored.
187177
continue
188-
is_tentative = [
189-
prop for prop in metric.name.context.additionalProperties
190-
if prop.key == 'tentative' and prop.value == 'true'
191-
]
178+
is_tentative = metric.name.context.get('tentative') == 'true'
192179
tentative_or_committed = 'tentative' if is_tentative else 'committed'
193180

194181
metric_key = self._get_metric_key(metric)
@@ -209,32 +196,16 @@ def _get_metric_value(self, metric):
209196
return None
210197

211198
if metric.scalar is not None:
212-
return metric.scalar.integer_value
199+
# This will always be a single value if there is any data in the field.
200+
val = metric.scalar
201+
if isinstance(val, float) and val.is_integer():
202+
return int(val)
203+
return val
213204
elif metric.distribution is not None:
214-
dist_count = _get_match(
215-
metric.distribution.object_value.properties,
216-
lambda x: x.key == 'count').value.integer_value
217-
dist_min = _get_match(
218-
metric.distribution.object_value.properties,
219-
lambda x: x.key == 'min').value.integer_value
220-
dist_max = _get_match(
221-
metric.distribution.object_value.properties,
222-
lambda x: x.key == 'max').value.integer_value
223-
dist_sum = _get_match(
224-
metric.distribution.object_value.properties,
225-
lambda x: x.key == 'sum').value.integer_value
226-
if dist_sum is None:
227-
# distribution metric is not meant to use on large values, but in case
228-
# it is, the value can overflow and become double_value, the correctness
229-
# of the value may not be guaranteed.
230-
_LOGGER.info(
231-
"Distribution metric sum value seems to have "
232-
"overflowed integer_value range, the correctness of sum or mean "
233-
"value may not be guaranteed: %s" % metric.distribution)
234-
dist_sum = int(
235-
_get_match(
236-
metric.distribution.object_value.properties,
237-
lambda x: x.key == 'sum').value.double_value)
205+
dist_count = int(metric.distribution['count'])
206+
dist_min = int(metric.distribution['min'])
207+
dist_max = int(metric.distribution['max'])
208+
dist_sum = int(metric.distribution['sum'])
238209
return DistributionResult(
239210
DistributionData(dist_sum, dist_count, dist_min, dist_max))
240211
#TODO(https://github.com/apache/beam/issues/31788) support StringSet after

0 commit comments

Comments
 (0)