[
https://issues.apache.org/jira/browse/BEAM-4775?focusedWorklogId=201125&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-201125
]
ASF GitHub Bot logged work on BEAM-4775:
----------------------------------------
Author: ASF GitHub Bot
Created on: 20/Feb/19 07:40
Start Date: 20/Feb/19 07:40
Worklog Time Spent: 10m
Work Description: ryan-williams commented on pull request #7876:
[BEAM-4775] Clean up metric protos; support integer distributions, gauges
URL: https://github.com/apache/beam/pull/7876#discussion_r258357699
##########
File path:
runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/MonitoringInfoMetricName.java
##########
@@ -113,11 +136,15 @@ public boolean equals(Object o) {
}
@Override
- public String toString() {
+ public String toString(String delimiter) {
+ if (getNamespace() != null && getName() != null) {
+ return super.toString(delimiter);
+ }
StringBuilder builder = new StringBuilder();
- builder.append(this.urn.toString());
- builder.append(" ");
- builder.append(this.labels.toString());
+ if (labels.containsKey(PCOLLECTION_LABEL)) {
Review comment:
I wrote things that way in several places initially, but ran into
interesting problems that we'll need to discuss eventually, so I'll describe
them below.
Here's what happens in this PR if I naively append the label kv-pairs:
https://github.com/ryan-williams/beam/commit/557628627ae7438a6b688514e81205594238ed0f.
Note that Flink receives a metric with the unseemly name
[`step.PTRANSFORM.step.PCOLLECTION.pcoll.beam.metric.element_count.v1`](https://github.com/ryan-williams/beam/commit/557628627ae7438a6b688514e81205594238ed0f#diff-682be67e606ae40ded3e0087df5f9f8bR136)
(which goes e.g. to the Flink web UI, which we've taken pains to make more
human-readable).
The first `step` there is added by `MetricKey.toString`, which calls this
`MetricName.toString` routine, expecting info about the metric's "name" (URN)
only.
## MetricKey vs MetricName
The issue, In my view, is that `MonitoringInfoMetricName` is actually more
of a `MetricKey` implementation:
- `MetricKey` ≈ `MonitoringInfoMetricName` ({URN, labels})
- `stepName` ≈ [labels map with just a `PTRANSFORM` entry]
- `MetricName` ≈ URN
- `MetricResult` ≈ `MonitoringInfo` ({key, metric value})
I ultimately lay things out in this manner in #7823, folding MIMN into
MetricKey.
## Special-casing single-label ptransform/pcollection-scoped metrics
A bigger issue is that I think runners and endpoints are going to want to
hard-code [the only 2 cases that we will have for the foreseeable future]:
`<ptransform>.<namespace>.<name>` and `<pcollection>.<urn>`
(`<ptransform>.<urn>` folds into this nicely as well).
They'll do this for backwards-compat reasons, and/or to make things look
nice in e.g. web UIs. I've touched code like this in at least the Samza, Flink,
and Spark runners (and that `MetricsHttpSink` we were looking at).
I think it's great that the wire format is extensible to many+new labels,
but ended up feeling that the SDK should provide the APIs that deal
specifically with the shapes all calling code will care about atm.
Sorry that's a lot! I don't care much what we do in this case, but I think
we'll have some harder similar choices shortly 😲 lmk what you think.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 201125)
Time Spent: 14h (was: 13h 50m)
> JobService should support returning metrics
> -------------------------------------------
>
> Key: BEAM-4775
> URL: https://issues.apache.org/jira/browse/BEAM-4775
> Project: Beam
> Issue Type: Bug
> Components: beam-model
> Reporter: Eugene Kirpichov
> Assignee: Ryan Williams
> Priority: Major
> Labels: triaged
> Time Spent: 14h
> Remaining Estimate: 0h
>
> [https://github.com/apache/beam/blob/master/model/job-management/src/main/proto/beam_job_api.proto]
> currently doesn't appear to have a way for JobService to return metrics to a
> user, even though
> [https://github.com/apache/beam/blob/master/model/fn-execution/src/main/proto/beam_fn_api.proto]
> includes support for reporting SDK metrics to the runner harness.
>
> Metrics are apparently necessary to run any ValidatesRunner tests because
> PAssert needs to validate that the assertions succeeded. However, this
> statement should be double-checked: perhaps it's possible to somehow work
> with PAssert without metrics support.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
