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 >>>> >> >>>> > >>>> >>>
