By "type" of metric, I mean both the data types (including their encoding) and accumulator strategy. So sumint would be a type, as would double-distribution.
On Wed, Apr 11, 2018 at 10:39 PM Ben Chambers <bjchamb...@gmail.com> wrote: > When you say type do you mean accumulator type, result type, or > accumulator strategy? Specifically, what is the "type" of sumint, sumlong, > meanlong, etc? > > On Wed, Apr 11, 2018, 9:38 PM Robert Bradshaw <rober...@google.com> wrote: > >> 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 >>>>>>> >> >>>>>>> > >>>>>>> >>>>>>