Maybe leave it out until proven it is needed. ATM counters are used a lot but others are less mainstream so being too fine from the start can just add complexity and bugs in impls IMHO.
Le 12 avr. 2018 08:06, "Robert Bradshaw" <rober...@google.com> a écrit : > 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 >>>>>>>> >> >>>>>>>> > >>>>>>>> >>>>>>>