From dev-return-105721-archive-asf-public=cust-asf.ponee.io@kafka.apache.org Mon Jul 15 16:07:16 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 379AF180595 for ; Mon, 15 Jul 2019 18:07:16 +0200 (CEST) Received: (qmail 44822 invoked by uid 500); 15 Jul 2019 16:07:14 -0000 Mailing-List: contact dev-help@kafka.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@kafka.apache.org Delivered-To: mailing list dev@kafka.apache.org Received: (qmail 44806 invoked by uid 99); 15 Jul 2019 16:07:13 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd1-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 15 Jul 2019 16:07:13 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd1-us-west.apache.org (ASF Mail Server at spamd1-us-west.apache.org) with ESMTP id 56F12C06CA for ; Mon, 15 Jul 2019 16:07:13 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: -0.199 X-Spam-Level: X-Spam-Status: No, score=-0.199 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=disabled Authentication-Results: spamd1-us-west.apache.org (amavisd-new); dkim=pass (2048-bit key) header.d=confluent.io Received: from mx1-ec2-va.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id WXgB7wrJJP-t for ; Mon, 15 Jul 2019 16:07:11 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=209.85.221.52; helo=mail-wr1-f52.google.com; envelope-from=john@confluent.io; receiver= Received: from mail-wr1-f52.google.com (mail-wr1-f52.google.com [209.85.221.52]) by mx1-ec2-va.apache.org (ASF Mail Server at mx1-ec2-va.apache.org) with ESMTPS id 2674ABC7A5 for ; Mon, 15 Jul 2019 16:07:11 +0000 (UTC) Received: by mail-wr1-f52.google.com with SMTP id y4so17747560wrm.2 for ; Mon, 15 Jul 2019 09:07:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=confluent.io; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=n9zQtmfa0h/Bb8aTZYUFfL6MmtU3KNsaIrRZ+iWE+qo=; b=m2XY8VovR3HjobEq/MKTd5geghoIidw0gCE2fYGwusmexivpPL5PWIR+AnuktktH+9 HOg2Ud+czfpDZyT7WOK+wlnG0pL0JFAOsN2vaau3OB+D050GYTfY1/Ndav+vkgc/xbuC cI5RrAlj+vGkSV9sLYRT6NjAqxOwa76ov8rFi5ET53+uJplfvl8hTRPRJsHtxI+Vy0hk XGVD8420JEUP84zsYYjErEm6xqs9Psug8w+AH6V0xEOpXh7GeYWGXCro016uCg19UmzK w/U9tMAtgLKldIMOrWQ8ilkFNdBvanOILXksnZLlsOdrjdYcydCKLobXl24qd8Wn6LMf F2sQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=n9zQtmfa0h/Bb8aTZYUFfL6MmtU3KNsaIrRZ+iWE+qo=; b=Vt1+S2l2xq/oAmKEWafr+8AhxADOt85n4gRl2oxOixany9yZOCmjByW+P9Jq8LCqqp t7d7I/tplZ8hOeW4+yLzJdoJnIB7NGM+0wSGixFQtKjnyeMSh7n8y95u9cBvUFxrgwRk j7D1sQjy6bVwg+TdRcrSYG8p7hAR+e7egayCHCGUw609ZyWVFZPsdxaJaGHWyQo0cpwQ amtewy+1mkyM7IqKZvHEjShICe2yEX3/g8UMJtzPBv3VkBmtot6HJ4dKwF63Vp05kCdA 4ZfOjex2tYnvK23v+suXJZ0punnFx2cIFvXlF6jSpXKXv9bFKJXhIibYhKV7omlDFTRg M/zg== X-Gm-Message-State: APjAAAUfXpzYH2EMHHvdmZlw7Woqie3hMI50WeVFbMuavElb1SRELXHV klEXVADMvb0XC79cB+QMcsDqZz5con7RBVsZFTa+HEUdGfs= X-Google-Smtp-Source: APXvYqw+glAmXfurrUC7Gr4blM35uIdLyWOhhlvxq5SIoIH6FjZKxuqzkSjctRcND0xghtTI2OerCb421UARYS7TiXQ= X-Received: by 2002:adf:dd51:: with SMTP id u17mr28676032wrm.218.1563206829720; Mon, 15 Jul 2019 09:07:09 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: John Roesler Date: Mon, 15 Jul 2019 11:06:58 -0500 Message-ID: Subject: Re: [DISCUSS] KIP-488: Clean up Sum,Count,Total Metrics To: dev@kafka.apache.org Content-Type: text/plain; charset="UTF-8" Hey Stanislav, Thanks for the feedback! I do think we should try to keep the scope of this KIP small. The idea you proposed is orthogonal, though, so if you wanted to write up a quick KIP concurrently, I'd be happy to review it. Regarding deprecation: yes, the metrics are in the public interface, because they are not inside a package named `internal`. Therefore, we do need to do deprecations and migrations when considering changes to them. I wish we didn't, since it could be a cleaner transition without them, but we shouldn't risk breaking source compatibility. Thanks, -John On Sat, Jul 13, 2019 at 1:30 PM Stanislav Kozlovski wrote: > > Hello, > > Thanks for the KIP, I'm happy we're converging on implementations. > > I was wondering whether we need to deprecate the old metrics - are those > classes public interfaces? I see we don't build a JavaDoc for them ( > https://kafka.apache.org/22/javadoc/allclasses-noframe.html) and, as far as > I know, that means we don't necessarily need to go with the deprecation > route. > > I think we also have the Measurable and Gauge classes which seem to do the > same thing. From the names, I intuitively thought that Measurable indicates > something that might require some amount of processing to get the value of > and Gauge to be something that is instantly read. Reading their usages, > though, I see that is not the case. Measurable is inherited by > MeasurableStat which itself is inherited by most of the classes you > mentioned in the KIP. Is it worth it converging on those as well (perhaps > deprecating/removing Measurable and opting for Gauge) or do we prefer to > keep the scope of this KIP small? > > Thanks, > Stanislav > > On Fri, Jul 12, 2019 at 7:42 PM John Roesler wrote: > > > Hey, thanks Matthias and Bruno, > > > > I agree, "Cumulative" is a mouthful. "TotalX" sounds fine to me. > > > > Also, yes, I would have liked to not have any modifier for > > "non-sampled", but there is a name conflict with Sum. > > > > I'll update the KIP to reflect "TotalX" and then start the vote thread. > > > > Thanks again, > > -John > > > > On Fri, Jul 12, 2019 at 11:27 AM Bruno Cadonna wrote: > > > > > > OK, makes sense. Then, I am in favour of TotalCount and TotalSum. > > > > > > Best, > > > Bruno > > > > > > On Fri, Jul 12, 2019 at 12:57 AM Matthias J. Sax > > wrote: > > > > > > > > `Sum` is an existing name, for the "sampled sum" metric, that gets > > > > deprecated. Hence, we cannot use it. > > > > > > > > If we cannot use `Sum` and use `TotalSum`, we should also not use > > > > `Count` but `TotalCount` for consistency. > > > > > > > > > > > > -Matthias > > > > > > > > > > > > > > > > On 7/11/19 12:58 PM, Bruno Cadonna wrote: > > > > > Hi John, > > > > > > > > > > Thank you for the KIP. > > > > > > > > > > LGTM > > > > > > > > > > I also do not like CumulativeSum/Count so much. I propose to just > > call > > > > > it Sum and Count. > > > > > > > > > > I understand that you want to unequivocally distinguish the two > > metric > > > > > functions by their names, but I have the feeling the names become > > > > > artificially complex. The exact semantics can also be documented in > > > > > the javadocs, which btw could also be improved in those classes. > > > > > > > > > > Best, > > > > > Bruno > > > > > > > > > > > > > > > > > > > > On Thu, Jul 11, 2019 at 8:25 PM Matthias J. Sax < > > matthias@confluent.io> wrote: > > > > >> > > > > >> Thanks for the KIP. Overall LGTM. > > > > >> > > > > >> The only though I have is, if we may want to use `TotalSum` and > > > > >> `TotalCount` instead of `CumulativeSum/Count` as names? > > > > >> > > > > >> > > > > >> -Matthias > > > > >> > > > > >> > > > > >> On 7/11/19 9:31 AM, John Roesler wrote: > > > > >>> Hi Kafka devs, > > > > >>> > > > > >>> I'd like to propose KIP-488 as a minor cleanup of some of our > > metric > > > > >>> implementations. > > > > >>> > > > > >>> KIP-488: https://cwiki.apache.org/confluence/x/kkAyBw > > > > >>> > > > > >>> Over time, iterative updates to these metrics has resulted in a > > pretty > > > > >>> confusing little collection of classes, and I've personally been > > > > >>> involved in three separate moderately time-consuming iterations of > > me > > > > >>> or someone else trying to work out which metrics are available, and > > > > >>> which ones are desired for a given use case. One of these was > > actually > > > > >>> a long-running bug in Kafka Streams' metrics, so not only has this > > > > >>> confusion been a time sink, but it has also led to bugs. > > > > >>> > > > > >>> I'm hoping this change won't be too controversial. > > > > >>> > > > > >>> Thanks, > > > > >>> -John > > > > >>> > > > > >> > > > > > >