Fully custom metric types is the "more speculative and difficult" feature that I was proposing we kick down the road (and may never get to). What I'm suggesting is that we support custom metrics of standard type.
On Wed, Apr 11, 2018 at 5:52 PM Ben Chambers <bchamb...@apache.org> wrote: > The metric api is designed to prevent user defined metric types based on > the fact they just weren't used enough to justify support. > > Is there a reason we are bringing that complexity back? Shouldn't we just > need the ability for the standard set plus any special system metrivs? > On Wed, Apr 11, 2018, 5:43 PM Robert Bradshaw <rober...@google.com> wrote: > >> Thanks. I think this has simplified things. >> >> One thing that has occurred to me is that we're conflating the idea of >> custom metrics and custom metric types. I would propose the MetricSpec >> field be augmented with an additional field "type" which is a urn >> specifying the type of metric it is (i.e. the contents of its payload, as >> well as the form of aggregation). Summing or maxing over ints would be a >> typical example. Though we could pursue making this opaque to the runner in >> the long run, that's a more speculative (and difficult) feature to tackle. >> This would allow the runner to at least aggregate and report/return to the >> SDK metrics that it did not itself understand the semantic meaning of. (It >> would probably simplify much of the specialization in the runner itself for >> metrics that it *did* understand as well.) >> >> In addition, rather than having UserMetricOfTypeX for every type X one >> would have a single URN for UserMetric and it spec would designate the type >> and payload designate the (qualified) name. >> >> - Robert >> >> >> >> On Wed, Apr 11, 2018 at 5:12 PM Alex Amato <ajam...@google.com> wrote: >> >>> Thank you everyone for your feedback so far. >>> I have made a revision today which is to make all metrics refer to a >>> primary entity, so I have restructured some of the protos a little bit. >>> >>> The point of this change was to futureproof the possibility of allowing >>> custom user metrics, with custom aggregation functions for its metric >>> updates. >>> Now that each metric has an aggregation_entity associated with it (e.g. >>> PCollection, PTransform), we can design an approach which forwards the >>> opaque bytes metric updates, without deserializing them. These are >>> forwarded to user provided code which then would deserialize the metric >>> update payloads and perform the custom aggregations. >>> >>> I think it has also simplified some of the URN metric protos, as they do >>> not need to keep track of ptransform names inside themselves now. The >>> result is simpler structures, for the metrics as the entities are pulled >>> outside of the metric. >>> >>> I have mentioned this in the doc now, and wanted to draw attention to >>> this particular revision. >>> >>> >>> >>> On Tue, Apr 10, 2018 at 9:53 AM Alex Amato <ajam...@google.com> wrote: >>> >>>> I've gathered a lot of feedback so far and want to make a decision by >>>> Friday, and begin working on related PRs next week. >>>> >>>> Please make sure that you provide your feedback before then and I will >>>> post the final decisions made to this thread Friday afternoon. >>>> >>>> >>>> On Thu, Apr 5, 2018 at 1:38 AM Ismaël Mejía <ieme...@gmail.com> wrote: >>>> >>>>> Nice, I created a short link so people can refer to it easily in >>>>> future discussions, website, etc. >>>>> >>>>> https://s.apache.org/beam-fn-api-metrics >>>>> >>>>> Thanks for sharing. >>>>> >>>>> >>>>> On Wed, Apr 4, 2018 at 11:28 PM, Robert Bradshaw <rober...@google.com> >>>>> wrote: >>>>> > Thanks for the nice writeup. I added some comments. >>>>> > >>>>> > On Wed, Apr 4, 2018 at 1:53 PM Alex Amato <ajam...@google.com> >>>>> wrote: >>>>> >> >>>>> >> Hello beam community, >>>>> >> >>>>> >> Thank you everyone for your initial feedback on this proposal so >>>>> far. I >>>>> >> have made some revisions based on the feedback. There were some >>>>> larger >>>>> >> questions asking about alternatives. For each of these I have added >>>>> a >>>>> >> section tagged with [Alternatives] and discussed my recommendation >>>>> as well >>>>> >> as as few other choices we considered. >>>>> >> >>>>> >> I would appreciate more feedback on the revised proposal. Please >>>>> take >>>>> >> another look and let me know >>>>> >> >>>>> >> >>>>> https://docs.google.com/document/d/1MtBZYV7NAcfbwyy9Op8STeFNBxtljxgy69FkHMvhTMA/edit >>>>> >> >>>>> >> Etienne, I would appreciate it if you could please take another >>>>> look after >>>>> >> the revisions I have made as well. >>>>> >> >>>>> >> Thanks again, >>>>> >> Alex >>>>> >> >>>>> > >>>>> >>>>