scwhittle commented on code in PR #33503:
URL: https://github.com/apache/beam/pull/33503#discussion_r1975424830
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/LabeledMetricNameUtils.java:
##########
@@ -63,8 +65,18 @@ public void addLabel(String key, String value) {
.append(LABEL_DELIMITER);
}
+ /**
+ * Add a metric label KV pair to the metric. This is not concatenated as a
part of the name, but
+ * merely for adding attributes to the metric.
+ */
+ public void addMetricLabel(String key, String value) {
+ System.out.println("xxx add metric lable");
Review Comment:
rm
##########
sdks/java/extensions/google-cloud-platform-core/src/test/java/org/apache/beam/sdk/extensions/gcp/GcpCoreApiSurfaceTest.java:
##########
@@ -40,6 +40,7 @@ public void testGcpCoreApiSurface() throws Exception {
final ApiSurface apiSurface =
ApiSurface.ofPackage(thisPackage, thisClassLoader)
.pruningPattern("org[.]apache[.]beam[.].*Test.*")
+ .pruningPattern("org[.]apache[.]beam[.].*vendor.*")
Review Comment:
is it correct to prune this? It seemsl like we might not want to expose our
vendored libs. I'm not sure why it is needed though, since where you add
vendor guava deps it seems like they were already there.
##########
model/pipeline/src/main/proto/org/apache/beam/model/pipeline/v1/metrics.proto:
##########
@@ -447,6 +447,7 @@ message MonitoringInfo {
SPANNER_TABLE_ID = 25 [(label_props) = { name: "SPANNER_TABLE_ID" }];
SPANNER_INSTANCE_ID = 26 [(label_props) = { name: "SPANNER_INSTANCE_ID" }];
SPANNER_QUERY_NAME = 27 [(label_props) = { name: "SPANNER_QUERY_NAME" }];
+ PER_WORKER_METRIC = 28 [(label_props) = { name: "PER_WORKER_METRIC" }];
Review Comment:
How about a comment like:
// Label which if has a "true" value indicates that the metric is intended
to be aggregated per-worker.
--
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]
