Sounds perfect. Just wanted to make sure that "custom metrics of supported
type" didn't include new ways of aggregating ints. As long as that means we
have a fixed set of aggregations (that align with what what users want and
metrics back end support) it seems like we are doing user metrics right.

- Ben

On Wed, Apr 11, 2018, 11:30 PM Romain Manni-Bucau <rmannibu...@gmail.com>
wrote:

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

Reply via email to