Re: Flink-Metrics Prometheus - Native Histograms / Native Counters

2023-06-30 Thread Ryan van Huuksloot
Sounds good - I haven't looked too much into the comparison of the old and
new client.

Opened a ticket: https://issues.apache.org/jira/browse/FLINK-32508

I may have some time to support this ticket in 2 weeks but anyone can feel
free to start working on it.

Thanks,
Ryan van Huuksloot
Sr. Production Engineer | Streaming Platform
[image: Shopify]



On Fri, Jun 30, 2023 at 10:34 AM Martijn Visser 
wrote:

> Hi Ryan,
>
> If I look at the changelog for the simpleclient 0.10 [1], they've switched
> their data model. So if you upgrade to the later version, the data model
> for existing Flink Prometheus users would be broken IIRC. That's why I
> think option 1 is more clean: it provides the option to the user to choose
> which package they want to use. Either the new one, with a new data model,
> or the current one, with the existing data model.
>
> Best regards,
>
> Martijn
>
> [1] https://github.com/prometheus/client_java/releases/tag/parent-0.10.0
>
> On Fri, Jun 30, 2023 at 4:23 PM Ryan van Huuksloot
>  wrote:
>
> > I'd have to check but the original plan is to upgrade the client but keep
> > the flink-metrics-prometheus implementation the same. This should keep
> the
> > metrics collection consistent even with the client upgrade - this would
> > need to be verified.
> >
> > But if that is the worry then we could create a new package to keep
> things
> > distinct.
> >
> > Thanks,
> >
> > Ryan van Huuksloot
> > Sr. Production Engineer | Streaming Platform
> > [image: Shopify]
> >  >
> >
> >
> > On Fri, Jun 30, 2023 at 10:02 AM Martijn Visser <
> martijnvis...@apache.org>
> > wrote:
> >
> > > Hi Patrick,
> > >
> > > Yeah, but you would need the latest version of the client, which would
> > > break the implementation for the current, outdated one, wouldn't it?
> > >
> > > Best regards,
> > >
> > > Martijn
> > >
> > > On Fri, Jun 30, 2023 at 3:35 PM Ryan van Huuksloot
> > >  wrote:
> > >
> > > > Hi Martijn,
> > > >
> > > > Option 2 and 3 would use a single client. It would just register the
> > > > metrics differently.
> > > >
> > > > Does that make sense? Does that change your perspective?
> > > >
> > > > Thanks,
> > > >
> > > > Ryan van Huuksloot
> > > > Sr. Production Engineer | Streaming Platform
> > > > [image: Shopify]
> > > > <
> > https://www.shopify.com/?utm_medium=salessignatures_source=hs_email
> > > >
> > > >
> > > >
> > > > On Fri, Jun 30, 2023 at 7:49 AM Martijn Visser <
> > martijnvis...@apache.org
> > > >
> > > > wrote:
> > > >
> > > > > Hi Ryan,
> > > > >
> > > > > I think option 2 and option 3 won't work, because there can be only
> > one
> > > > > version of the client. I don't think we should make a clean break
> on
> > > > > metrics in a minor version, but only in major. All in all, I think
> > > > option 1
> > > > > would be the best. We could deprecate the existing one and remove
> it
> > > > > with Flink 2.0 imho.
> > > > >
> > > > > Best regards,
> > > > >
> > > > > Martijn
> > > > >
> > > > > On Thu, Jun 29, 2023 at 5:56 PM Ryan van Huuksloot
> > > > >  wrote:
> > > > >
> > > > > > Hi Martijn,
> > > > > >
> > > > > > Our team shared the same concern. We've considered a few options:
> > > > > >
> > > > > >
> > > > > > *1. Add a new package such as `flink-metrics-prometheus-native`
> and
> > > > > > eventually deprecate the original.*
> > > > > > *Pros:*
> > > > > > - Supports backward compatibility.
> > > > > > *Cons:*
> > > > > > - Two packages to maintain in the interim.
> > > > > > - Not consistent with other metrics packages.
> > > > > >
> > > > > > *2. Maintain the same logic in flink-metrics-prometheus and write
> > new
> > > > > > natively typed metrics to a different metric name in Prometheus,
> in
> > > > > > addition to the original metric.*
> > > > > >
> > > > > > *Pros:*
> > > > > > - Supports backward compatibility.
> > > > > > *Cons:*
> > > > > > - Nearly doubles the metrics being captured by default.
> > > > > > - The naming convention will permanently differ when the original
> > > names
> > > > > are
> > > > > > deprecated.
> > > > > > - The original names will likely be deprecated at some point.
> > > > > >
> > > > > > *3. Maintain the same logic in flink-metrics-prometheus. However,
> > if
> > > > you
> > > > > > use a flink-conf option, natively typed metrics would be written
> to
> > > the
> > > > > > same names instead of the original metric types.*
> > > > > >
> > > > > > *Pros:*
> > > > > > - Supports backwards compatibility
> > > > > > - No double metrics
> > > > > > *Cons:*
> > > > > > - Increases the maintenance burden.
> > > > > > - Would require future migrations
> > > > > >
> > > > > > *4. Make a clean break and swap the types in
> > > flink-metrics-prometheus,
> > > > > > releasing it in 1.18 or 1.19 with a note.*
> > > > > >
> > > > > > *Pros:*
> > > > > > - Avoids duplicate metrics and packages.
> > > > > > - No 

Re: Flink-Metrics Prometheus - Native Histograms / Native Counters

2023-06-30 Thread Martijn Visser
Hi Ryan,

If I look at the changelog for the simpleclient 0.10 [1], they've switched
their data model. So if you upgrade to the later version, the data model
for existing Flink Prometheus users would be broken IIRC. That's why I
think option 1 is more clean: it provides the option to the user to choose
which package they want to use. Either the new one, with a new data model,
or the current one, with the existing data model.

Best regards,

Martijn

[1] https://github.com/prometheus/client_java/releases/tag/parent-0.10.0

On Fri, Jun 30, 2023 at 4:23 PM Ryan van Huuksloot
 wrote:

> I'd have to check but the original plan is to upgrade the client but keep
> the flink-metrics-prometheus implementation the same. This should keep the
> metrics collection consistent even with the client upgrade - this would
> need to be verified.
>
> But if that is the worry then we could create a new package to keep things
> distinct.
>
> Thanks,
>
> Ryan van Huuksloot
> Sr. Production Engineer | Streaming Platform
> [image: Shopify]
> 
>
>
> On Fri, Jun 30, 2023 at 10:02 AM Martijn Visser 
> wrote:
>
> > Hi Patrick,
> >
> > Yeah, but you would need the latest version of the client, which would
> > break the implementation for the current, outdated one, wouldn't it?
> >
> > Best regards,
> >
> > Martijn
> >
> > On Fri, Jun 30, 2023 at 3:35 PM Ryan van Huuksloot
> >  wrote:
> >
> > > Hi Martijn,
> > >
> > > Option 2 and 3 would use a single client. It would just register the
> > > metrics differently.
> > >
> > > Does that make sense? Does that change your perspective?
> > >
> > > Thanks,
> > >
> > > Ryan van Huuksloot
> > > Sr. Production Engineer | Streaming Platform
> > > [image: Shopify]
> > > <
> https://www.shopify.com/?utm_medium=salessignatures_source=hs_email
> > >
> > >
> > >
> > > On Fri, Jun 30, 2023 at 7:49 AM Martijn Visser <
> martijnvis...@apache.org
> > >
> > > wrote:
> > >
> > > > Hi Ryan,
> > > >
> > > > I think option 2 and option 3 won't work, because there can be only
> one
> > > > version of the client. I don't think we should make a clean break on
> > > > metrics in a minor version, but only in major. All in all, I think
> > > option 1
> > > > would be the best. We could deprecate the existing one and remove it
> > > > with Flink 2.0 imho.
> > > >
> > > > Best regards,
> > > >
> > > > Martijn
> > > >
> > > > On Thu, Jun 29, 2023 at 5:56 PM Ryan van Huuksloot
> > > >  wrote:
> > > >
> > > > > Hi Martijn,
> > > > >
> > > > > Our team shared the same concern. We've considered a few options:
> > > > >
> > > > >
> > > > > *1. Add a new package such as `flink-metrics-prometheus-native` and
> > > > > eventually deprecate the original.*
> > > > > *Pros:*
> > > > > - Supports backward compatibility.
> > > > > *Cons:*
> > > > > - Two packages to maintain in the interim.
> > > > > - Not consistent with other metrics packages.
> > > > >
> > > > > *2. Maintain the same logic in flink-metrics-prometheus and write
> new
> > > > > natively typed metrics to a different metric name in Prometheus, in
> > > > > addition to the original metric.*
> > > > >
> > > > > *Pros:*
> > > > > - Supports backward compatibility.
> > > > > *Cons:*
> > > > > - Nearly doubles the metrics being captured by default.
> > > > > - The naming convention will permanently differ when the original
> > names
> > > > are
> > > > > deprecated.
> > > > > - The original names will likely be deprecated at some point.
> > > > >
> > > > > *3. Maintain the same logic in flink-metrics-prometheus. However,
> if
> > > you
> > > > > use a flink-conf option, natively typed metrics would be written to
> > the
> > > > > same names instead of the original metric types.*
> > > > >
> > > > > *Pros:*
> > > > > - Supports backwards compatibility
> > > > > - No double metrics
> > > > > *Cons:*
> > > > > - Increases the maintenance burden.
> > > > > - Would require future migrations
> > > > >
> > > > > *4. Make a clean break and swap the types in
> > flink-metrics-prometheus,
> > > > > releasing it in 1.18 or 1.19 with a note.*
> > > > >
> > > > > *Pros:*
> > > > > - Avoids duplicate metrics and packages.
> > > > > - No future maintenance burden.
> > > > > *Cons:*
> > > > > -Introduces a breaking change.
> > > > > - Metrics may silently fail in dashboards if the graphs do not
> > support
> > > > the
> > > > > new data type (I would need to conduct more testing to determine
> how
> > > > often
> > > > > this occurs).
> > > > >
> > > > > I lean towards option 4, and we would communicate the change
> > internally
> > > > as
> > > > > part of a minor version upgrade. I'm open to other ideas and would
> > > > welcome
> > > > > further discussion on what the OSS community prefers.
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Ryan van Huuksloot
> > > > > Sr. Production Engineer | Streaming Platform
> > > > > [image: Shopify]
> > > > > <
> > >
> 

Re: Flink-Metrics Prometheus - Native Histograms / Native Counters

2023-06-30 Thread Ryan van Huuksloot
I'd have to check but the original plan is to upgrade the client but keep
the flink-metrics-prometheus implementation the same. This should keep the
metrics collection consistent even with the client upgrade - this would
need to be verified.

But if that is the worry then we could create a new package to keep things
distinct.

Thanks,

Ryan van Huuksloot
Sr. Production Engineer | Streaming Platform
[image: Shopify]



On Fri, Jun 30, 2023 at 10:02 AM Martijn Visser 
wrote:

> Hi Patrick,
>
> Yeah, but you would need the latest version of the client, which would
> break the implementation for the current, outdated one, wouldn't it?
>
> Best regards,
>
> Martijn
>
> On Fri, Jun 30, 2023 at 3:35 PM Ryan van Huuksloot
>  wrote:
>
> > Hi Martijn,
> >
> > Option 2 and 3 would use a single client. It would just register the
> > metrics differently.
> >
> > Does that make sense? Does that change your perspective?
> >
> > Thanks,
> >
> > Ryan van Huuksloot
> > Sr. Production Engineer | Streaming Platform
> > [image: Shopify]
> >  >
> >
> >
> > On Fri, Jun 30, 2023 at 7:49 AM Martijn Visser  >
> > wrote:
> >
> > > Hi Ryan,
> > >
> > > I think option 2 and option 3 won't work, because there can be only one
> > > version of the client. I don't think we should make a clean break on
> > > metrics in a minor version, but only in major. All in all, I think
> > option 1
> > > would be the best. We could deprecate the existing one and remove it
> > > with Flink 2.0 imho.
> > >
> > > Best regards,
> > >
> > > Martijn
> > >
> > > On Thu, Jun 29, 2023 at 5:56 PM Ryan van Huuksloot
> > >  wrote:
> > >
> > > > Hi Martijn,
> > > >
> > > > Our team shared the same concern. We've considered a few options:
> > > >
> > > >
> > > > *1. Add a new package such as `flink-metrics-prometheus-native` and
> > > > eventually deprecate the original.*
> > > > *Pros:*
> > > > - Supports backward compatibility.
> > > > *Cons:*
> > > > - Two packages to maintain in the interim.
> > > > - Not consistent with other metrics packages.
> > > >
> > > > *2. Maintain the same logic in flink-metrics-prometheus and write new
> > > > natively typed metrics to a different metric name in Prometheus, in
> > > > addition to the original metric.*
> > > >
> > > > *Pros:*
> > > > - Supports backward compatibility.
> > > > *Cons:*
> > > > - Nearly doubles the metrics being captured by default.
> > > > - The naming convention will permanently differ when the original
> names
> > > are
> > > > deprecated.
> > > > - The original names will likely be deprecated at some point.
> > > >
> > > > *3. Maintain the same logic in flink-metrics-prometheus. However, if
> > you
> > > > use a flink-conf option, natively typed metrics would be written to
> the
> > > > same names instead of the original metric types.*
> > > >
> > > > *Pros:*
> > > > - Supports backwards compatibility
> > > > - No double metrics
> > > > *Cons:*
> > > > - Increases the maintenance burden.
> > > > - Would require future migrations
> > > >
> > > > *4. Make a clean break and swap the types in
> flink-metrics-prometheus,
> > > > releasing it in 1.18 or 1.19 with a note.*
> > > >
> > > > *Pros:*
> > > > - Avoids duplicate metrics and packages.
> > > > - No future maintenance burden.
> > > > *Cons:*
> > > > -Introduces a breaking change.
> > > > - Metrics may silently fail in dashboards if the graphs do not
> support
> > > the
> > > > new data type (I would need to conduct more testing to determine how
> > > often
> > > > this occurs).
> > > >
> > > > I lean towards option 4, and we would communicate the change
> internally
> > > as
> > > > part of a minor version upgrade. I'm open to other ideas and would
> > > welcome
> > > > further discussion on what the OSS community prefers.
> > > >
> > > > Thanks,
> > > >
> > > > Ryan van Huuksloot
> > > > Sr. Production Engineer | Streaming Platform
> > > > [image: Shopify]
> > > > <
> > https://www.shopify.com/?utm_medium=salessignatures_source=hs_email
> > > >
> > > >
> > > >
> > > > On Thu, Jun 29, 2023 at 4:23 AM Martijn Visser <
> > martijnvis...@apache.org
> > > >
> > > > wrote:
> > > >
> > > > > Hi Ryan,
> > > > >
> > > > > I think there definitely is an interest in the
> > > > > flink-metrics-prometheus, but I do see some challenges as well.
> Given
> > > > > that the Prometheus simpleclient doesn't yet have a major version,
> > > > > there are breaking changes happening in that. If we would update
> > this,
> > > > > it can/probably breaks the metrics for users, which is an
> undesirable
> > > > > situation. Any thoughts on how we could avoid that situation?
> > > > >
> > > > > Best regards,
> > > > >
> > > > > Martijn
> > > > >
> > > > > On Tue, Jun 20, 2023 at 3:53 PM Ryan van Huuksloot
> > > > >  wrote:
> > > > > >
> > > > > > Following up, any interest in flink-metrics-prometheus? It is
> > quite a
> > > > > stale
> 

Re: Flink-Metrics Prometheus - Native Histograms / Native Counters

2023-06-30 Thread Martijn Visser
Hi Patrick,

Yeah, but you would need the latest version of the client, which would
break the implementation for the current, outdated one, wouldn't it?

Best regards,

Martijn

On Fri, Jun 30, 2023 at 3:35 PM Ryan van Huuksloot
 wrote:

> Hi Martijn,
>
> Option 2 and 3 would use a single client. It would just register the
> metrics differently.
>
> Does that make sense? Does that change your perspective?
>
> Thanks,
>
> Ryan van Huuksloot
> Sr. Production Engineer | Streaming Platform
> [image: Shopify]
> 
>
>
> On Fri, Jun 30, 2023 at 7:49 AM Martijn Visser 
> wrote:
>
> > Hi Ryan,
> >
> > I think option 2 and option 3 won't work, because there can be only one
> > version of the client. I don't think we should make a clean break on
> > metrics in a minor version, but only in major. All in all, I think
> option 1
> > would be the best. We could deprecate the existing one and remove it
> > with Flink 2.0 imho.
> >
> > Best regards,
> >
> > Martijn
> >
> > On Thu, Jun 29, 2023 at 5:56 PM Ryan van Huuksloot
> >  wrote:
> >
> > > Hi Martijn,
> > >
> > > Our team shared the same concern. We've considered a few options:
> > >
> > >
> > > *1. Add a new package such as `flink-metrics-prometheus-native` and
> > > eventually deprecate the original.*
> > > *Pros:*
> > > - Supports backward compatibility.
> > > *Cons:*
> > > - Two packages to maintain in the interim.
> > > - Not consistent with other metrics packages.
> > >
> > > *2. Maintain the same logic in flink-metrics-prometheus and write new
> > > natively typed metrics to a different metric name in Prometheus, in
> > > addition to the original metric.*
> > >
> > > *Pros:*
> > > - Supports backward compatibility.
> > > *Cons:*
> > > - Nearly doubles the metrics being captured by default.
> > > - The naming convention will permanently differ when the original names
> > are
> > > deprecated.
> > > - The original names will likely be deprecated at some point.
> > >
> > > *3. Maintain the same logic in flink-metrics-prometheus. However, if
> you
> > > use a flink-conf option, natively typed metrics would be written to the
> > > same names instead of the original metric types.*
> > >
> > > *Pros:*
> > > - Supports backwards compatibility
> > > - No double metrics
> > > *Cons:*
> > > - Increases the maintenance burden.
> > > - Would require future migrations
> > >
> > > *4. Make a clean break and swap the types in flink-metrics-prometheus,
> > > releasing it in 1.18 or 1.19 with a note.*
> > >
> > > *Pros:*
> > > - Avoids duplicate metrics and packages.
> > > - No future maintenance burden.
> > > *Cons:*
> > > -Introduces a breaking change.
> > > - Metrics may silently fail in dashboards if the graphs do not support
> > the
> > > new data type (I would need to conduct more testing to determine how
> > often
> > > this occurs).
> > >
> > > I lean towards option 4, and we would communicate the change internally
> > as
> > > part of a minor version upgrade. I'm open to other ideas and would
> > welcome
> > > further discussion on what the OSS community prefers.
> > >
> > > Thanks,
> > >
> > > Ryan van Huuksloot
> > > Sr. Production Engineer | Streaming Platform
> > > [image: Shopify]
> > > <
> https://www.shopify.com/?utm_medium=salessignatures_source=hs_email
> > >
> > >
> > >
> > > On Thu, Jun 29, 2023 at 4:23 AM Martijn Visser <
> martijnvis...@apache.org
> > >
> > > wrote:
> > >
> > > > Hi Ryan,
> > > >
> > > > I think there definitely is an interest in the
> > > > flink-metrics-prometheus, but I do see some challenges as well. Given
> > > > that the Prometheus simpleclient doesn't yet have a major version,
> > > > there are breaking changes happening in that. If we would update
> this,
> > > > it can/probably breaks the metrics for users, which is an undesirable
> > > > situation. Any thoughts on how we could avoid that situation?
> > > >
> > > > Best regards,
> > > >
> > > > Martijn
> > > >
> > > > On Tue, Jun 20, 2023 at 3:53 PM Ryan van Huuksloot
> > > >  wrote:
> > > > >
> > > > > Following up, any interest in flink-metrics-prometheus? It is
> quite a
> > > > stale
> > > > > package. I would be interested in contributing - time permitting.
> > > > >
> > > > > Ryan van Huuksloot
> > > > > Sr. Production Engineer | Streaming Platform
> > > > > [image: Shopify]
> > > > > <
> > >
> https://www.shopify.com/?utm_medium=salessignatures_source=hs_email
> > > > >
> > > > >
> > > > >
> > > > > On Thu, Jun 15, 2023 at 2:16 PM Ryan van Huuksloot <
> > > > > ryan.vanhuuksl...@shopify.com> wrote:
> > > > >
> > > > > > Hello,
> > > > > >
> > > > > > Internally we use the flink-metrics-prometheus jar and we noticed
> > > that
> > > > the
> > > > > > code is a little out of date. Primarily, there are new metric
> types
> > > in
> > > > > > Prometheus that would allow for the exporter to write Counters
> and
> > > > > > Histograms as Native metrics in prometheus (vs writing as
> Gauges).
> > > > > >
> > > > > 

Re: Flink-Metrics Prometheus - Native Histograms / Native Counters

2023-06-30 Thread Ryan van Huuksloot
Hi Martijn,

Option 2 and 3 would use a single client. It would just register the
metrics differently.

Does that make sense? Does that change your perspective?

Thanks,

Ryan van Huuksloot
Sr. Production Engineer | Streaming Platform
[image: Shopify]



On Fri, Jun 30, 2023 at 7:49 AM Martijn Visser 
wrote:

> Hi Ryan,
>
> I think option 2 and option 3 won't work, because there can be only one
> version of the client. I don't think we should make a clean break on
> metrics in a minor version, but only in major. All in all, I think option 1
> would be the best. We could deprecate the existing one and remove it
> with Flink 2.0 imho.
>
> Best regards,
>
> Martijn
>
> On Thu, Jun 29, 2023 at 5:56 PM Ryan van Huuksloot
>  wrote:
>
> > Hi Martijn,
> >
> > Our team shared the same concern. We've considered a few options:
> >
> >
> > *1. Add a new package such as `flink-metrics-prometheus-native` and
> > eventually deprecate the original.*
> > *Pros:*
> > - Supports backward compatibility.
> > *Cons:*
> > - Two packages to maintain in the interim.
> > - Not consistent with other metrics packages.
> >
> > *2. Maintain the same logic in flink-metrics-prometheus and write new
> > natively typed metrics to a different metric name in Prometheus, in
> > addition to the original metric.*
> >
> > *Pros:*
> > - Supports backward compatibility.
> > *Cons:*
> > - Nearly doubles the metrics being captured by default.
> > - The naming convention will permanently differ when the original names
> are
> > deprecated.
> > - The original names will likely be deprecated at some point.
> >
> > *3. Maintain the same logic in flink-metrics-prometheus. However, if you
> > use a flink-conf option, natively typed metrics would be written to the
> > same names instead of the original metric types.*
> >
> > *Pros:*
> > - Supports backwards compatibility
> > - No double metrics
> > *Cons:*
> > - Increases the maintenance burden.
> > - Would require future migrations
> >
> > *4. Make a clean break and swap the types in flink-metrics-prometheus,
> > releasing it in 1.18 or 1.19 with a note.*
> >
> > *Pros:*
> > - Avoids duplicate metrics and packages.
> > - No future maintenance burden.
> > *Cons:*
> > -Introduces a breaking change.
> > - Metrics may silently fail in dashboards if the graphs do not support
> the
> > new data type (I would need to conduct more testing to determine how
> often
> > this occurs).
> >
> > I lean towards option 4, and we would communicate the change internally
> as
> > part of a minor version upgrade. I'm open to other ideas and would
> welcome
> > further discussion on what the OSS community prefers.
> >
> > Thanks,
> >
> > Ryan van Huuksloot
> > Sr. Production Engineer | Streaming Platform
> > [image: Shopify]
> >  >
> >
> >
> > On Thu, Jun 29, 2023 at 4:23 AM Martijn Visser  >
> > wrote:
> >
> > > Hi Ryan,
> > >
> > > I think there definitely is an interest in the
> > > flink-metrics-prometheus, but I do see some challenges as well. Given
> > > that the Prometheus simpleclient doesn't yet have a major version,
> > > there are breaking changes happening in that. If we would update this,
> > > it can/probably breaks the metrics for users, which is an undesirable
> > > situation. Any thoughts on how we could avoid that situation?
> > >
> > > Best regards,
> > >
> > > Martijn
> > >
> > > On Tue, Jun 20, 2023 at 3:53 PM Ryan van Huuksloot
> > >  wrote:
> > > >
> > > > Following up, any interest in flink-metrics-prometheus? It is quite a
> > > stale
> > > > package. I would be interested in contributing - time permitting.
> > > >
> > > > Ryan van Huuksloot
> > > > Sr. Production Engineer | Streaming Platform
> > > > [image: Shopify]
> > > > <
> > https://www.shopify.com/?utm_medium=salessignatures_source=hs_email
> > > >
> > > >
> > > >
> > > > On Thu, Jun 15, 2023 at 2:16 PM Ryan van Huuksloot <
> > > > ryan.vanhuuksl...@shopify.com> wrote:
> > > >
> > > > > Hello,
> > > > >
> > > > > Internally we use the flink-metrics-prometheus jar and we noticed
> > that
> > > the
> > > > > code is a little out of date. Primarily, there are new metric types
> > in
> > > > > Prometheus that would allow for the exporter to write Counters and
> > > > > Histograms as Native metrics in prometheus (vs writing as Gauges).
> > > > >
> > > > > I noticed that there was a closed PR for the simpleclient:
> > > > > https://github.com/apache/flink/pull/21047 - which has what is
> > > required
> > > > > for the native metrics but may cause other maintenance tickets.
> > > > >
> > > > > Is there any appetite from the community to update this exporter?
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Ryan van Huuksloot
> > > > > Sr. Production Engineer | Streaming Platform
> > > > > [image: Shopify]
> > > > > <
> > >
> https://www.shopify.com/?utm_medium=salessignatures_source=hs_email>
> > > > >
> > >
> >
>


Re: Flink-Metrics Prometheus - Native Histograms / Native Counters

2023-06-30 Thread Martijn Visser
Hi Ryan,

I think option 2 and option 3 won't work, because there can be only one
version of the client. I don't think we should make a clean break on
metrics in a minor version, but only in major. All in all, I think option 1
would be the best. We could deprecate the existing one and remove it
with Flink 2.0 imho.

Best regards,

Martijn

On Thu, Jun 29, 2023 at 5:56 PM Ryan van Huuksloot
 wrote:

> Hi Martijn,
>
> Our team shared the same concern. We've considered a few options:
>
>
> *1. Add a new package such as `flink-metrics-prometheus-native` and
> eventually deprecate the original.*
> *Pros:*
> - Supports backward compatibility.
> *Cons:*
> - Two packages to maintain in the interim.
> - Not consistent with other metrics packages.
>
> *2. Maintain the same logic in flink-metrics-prometheus and write new
> natively typed metrics to a different metric name in Prometheus, in
> addition to the original metric.*
>
> *Pros:*
> - Supports backward compatibility.
> *Cons:*
> - Nearly doubles the metrics being captured by default.
> - The naming convention will permanently differ when the original names are
> deprecated.
> - The original names will likely be deprecated at some point.
>
> *3. Maintain the same logic in flink-metrics-prometheus. However, if you
> use a flink-conf option, natively typed metrics would be written to the
> same names instead of the original metric types.*
>
> *Pros:*
> - Supports backwards compatibility
> - No double metrics
> *Cons:*
> - Increases the maintenance burden.
> - Would require future migrations
>
> *4. Make a clean break and swap the types in flink-metrics-prometheus,
> releasing it in 1.18 or 1.19 with a note.*
>
> *Pros:*
> - Avoids duplicate metrics and packages.
> - No future maintenance burden.
> *Cons:*
> -Introduces a breaking change.
> - Metrics may silently fail in dashboards if the graphs do not support the
> new data type (I would need to conduct more testing to determine how often
> this occurs).
>
> I lean towards option 4, and we would communicate the change internally as
> part of a minor version upgrade. I'm open to other ideas and would welcome
> further discussion on what the OSS community prefers.
>
> Thanks,
>
> Ryan van Huuksloot
> Sr. Production Engineer | Streaming Platform
> [image: Shopify]
> 
>
>
> On Thu, Jun 29, 2023 at 4:23 AM Martijn Visser 
> wrote:
>
> > Hi Ryan,
> >
> > I think there definitely is an interest in the
> > flink-metrics-prometheus, but I do see some challenges as well. Given
> > that the Prometheus simpleclient doesn't yet have a major version,
> > there are breaking changes happening in that. If we would update this,
> > it can/probably breaks the metrics for users, which is an undesirable
> > situation. Any thoughts on how we could avoid that situation?
> >
> > Best regards,
> >
> > Martijn
> >
> > On Tue, Jun 20, 2023 at 3:53 PM Ryan van Huuksloot
> >  wrote:
> > >
> > > Following up, any interest in flink-metrics-prometheus? It is quite a
> > stale
> > > package. I would be interested in contributing - time permitting.
> > >
> > > Ryan van Huuksloot
> > > Sr. Production Engineer | Streaming Platform
> > > [image: Shopify]
> > > <
> https://www.shopify.com/?utm_medium=salessignatures_source=hs_email
> > >
> > >
> > >
> > > On Thu, Jun 15, 2023 at 2:16 PM Ryan van Huuksloot <
> > > ryan.vanhuuksl...@shopify.com> wrote:
> > >
> > > > Hello,
> > > >
> > > > Internally we use the flink-metrics-prometheus jar and we noticed
> that
> > the
> > > > code is a little out of date. Primarily, there are new metric types
> in
> > > > Prometheus that would allow for the exporter to write Counters and
> > > > Histograms as Native metrics in prometheus (vs writing as Gauges).
> > > >
> > > > I noticed that there was a closed PR for the simpleclient:
> > > > https://github.com/apache/flink/pull/21047 - which has what is
> > required
> > > > for the native metrics but may cause other maintenance tickets.
> > > >
> > > > Is there any appetite from the community to update this exporter?
> > > >
> > > > Thanks,
> > > >
> > > > Ryan van Huuksloot
> > > > Sr. Production Engineer | Streaming Platform
> > > > [image: Shopify]
> > > > <
> > https://www.shopify.com/?utm_medium=salessignatures_source=hs_email>
> > > >
> >
>


Re: Flink-Metrics Prometheus - Native Histograms / Native Counters

2023-06-29 Thread Ryan van Huuksloot
Hi Martijn,

Our team shared the same concern. We've considered a few options:


*1. Add a new package such as `flink-metrics-prometheus-native` and
eventually deprecate the original.*
*Pros:*
- Supports backward compatibility.
*Cons:*
- Two packages to maintain in the interim.
- Not consistent with other metrics packages.

*2. Maintain the same logic in flink-metrics-prometheus and write new
natively typed metrics to a different metric name in Prometheus, in
addition to the original metric.*

*Pros:*
- Supports backward compatibility.
*Cons:*
- Nearly doubles the metrics being captured by default.
- The naming convention will permanently differ when the original names are
deprecated.
- The original names will likely be deprecated at some point.

*3. Maintain the same logic in flink-metrics-prometheus. However, if you
use a flink-conf option, natively typed metrics would be written to the
same names instead of the original metric types.*

*Pros:*
- Supports backwards compatibility
- No double metrics
*Cons:*
- Increases the maintenance burden.
- Would require future migrations

*4. Make a clean break and swap the types in flink-metrics-prometheus,
releasing it in 1.18 or 1.19 with a note.*

*Pros:*
- Avoids duplicate metrics and packages.
- No future maintenance burden.
*Cons:*
-Introduces a breaking change.
- Metrics may silently fail in dashboards if the graphs do not support the
new data type (I would need to conduct more testing to determine how often
this occurs).

I lean towards option 4, and we would communicate the change internally as
part of a minor version upgrade. I'm open to other ideas and would welcome
further discussion on what the OSS community prefers.

Thanks,

Ryan van Huuksloot
Sr. Production Engineer | Streaming Platform
[image: Shopify]



On Thu, Jun 29, 2023 at 4:23 AM Martijn Visser 
wrote:

> Hi Ryan,
>
> I think there definitely is an interest in the
> flink-metrics-prometheus, but I do see some challenges as well. Given
> that the Prometheus simpleclient doesn't yet have a major version,
> there are breaking changes happening in that. If we would update this,
> it can/probably breaks the metrics for users, which is an undesirable
> situation. Any thoughts on how we could avoid that situation?
>
> Best regards,
>
> Martijn
>
> On Tue, Jun 20, 2023 at 3:53 PM Ryan van Huuksloot
>  wrote:
> >
> > Following up, any interest in flink-metrics-prometheus? It is quite a
> stale
> > package. I would be interested in contributing - time permitting.
> >
> > Ryan van Huuksloot
> > Sr. Production Engineer | Streaming Platform
> > [image: Shopify]
> >  >
> >
> >
> > On Thu, Jun 15, 2023 at 2:16 PM Ryan van Huuksloot <
> > ryan.vanhuuksl...@shopify.com> wrote:
> >
> > > Hello,
> > >
> > > Internally we use the flink-metrics-prometheus jar and we noticed that
> the
> > > code is a little out of date. Primarily, there are new metric types in
> > > Prometheus that would allow for the exporter to write Counters and
> > > Histograms as Native metrics in prometheus (vs writing as Gauges).
> > >
> > > I noticed that there was a closed PR for the simpleclient:
> > > https://github.com/apache/flink/pull/21047 - which has what is
> required
> > > for the native metrics but may cause other maintenance tickets.
> > >
> > > Is there any appetite from the community to update this exporter?
> > >
> > > Thanks,
> > >
> > > Ryan van Huuksloot
> > > Sr. Production Engineer | Streaming Platform
> > > [image: Shopify]
> > > <
> https://www.shopify.com/?utm_medium=salessignatures_source=hs_email>
> > >
>


Re: Flink-Metrics Prometheus - Native Histograms / Native Counters

2023-06-29 Thread Martijn Visser
Hi Ryan,

I think there definitely is an interest in the
flink-metrics-prometheus, but I do see some challenges as well. Given
that the Prometheus simpleclient doesn't yet have a major version,
there are breaking changes happening in that. If we would update this,
it can/probably breaks the metrics for users, which is an undesirable
situation. Any thoughts on how we could avoid that situation?

Best regards,

Martijn

On Tue, Jun 20, 2023 at 3:53 PM Ryan van Huuksloot
 wrote:
>
> Following up, any interest in flink-metrics-prometheus? It is quite a stale
> package. I would be interested in contributing - time permitting.
>
> Ryan van Huuksloot
> Sr. Production Engineer | Streaming Platform
> [image: Shopify]
> 
>
>
> On Thu, Jun 15, 2023 at 2:16 PM Ryan van Huuksloot <
> ryan.vanhuuksl...@shopify.com> wrote:
>
> > Hello,
> >
> > Internally we use the flink-metrics-prometheus jar and we noticed that the
> > code is a little out of date. Primarily, there are new metric types in
> > Prometheus that would allow for the exporter to write Counters and
> > Histograms as Native metrics in prometheus (vs writing as Gauges).
> >
> > I noticed that there was a closed PR for the simpleclient:
> > https://github.com/apache/flink/pull/21047 - which has what is required
> > for the native metrics but may cause other maintenance tickets.
> >
> > Is there any appetite from the community to update this exporter?
> >
> > Thanks,
> >
> > Ryan van Huuksloot
> > Sr. Production Engineer | Streaming Platform
> > [image: Shopify]
> > 
> >


Re: Flink-Metrics Prometheus - Native Histograms / Native Counters

2023-06-20 Thread Ryan van Huuksloot
Following up, any interest in flink-metrics-prometheus? It is quite a stale
package. I would be interested in contributing - time permitting.

Ryan van Huuksloot
Sr. Production Engineer | Streaming Platform
[image: Shopify]



On Thu, Jun 15, 2023 at 2:16 PM Ryan van Huuksloot <
ryan.vanhuuksl...@shopify.com> wrote:

> Hello,
>
> Internally we use the flink-metrics-prometheus jar and we noticed that the
> code is a little out of date. Primarily, there are new metric types in
> Prometheus that would allow for the exporter to write Counters and
> Histograms as Native metrics in prometheus (vs writing as Gauges).
>
> I noticed that there was a closed PR for the simpleclient:
> https://github.com/apache/flink/pull/21047 - which has what is required
> for the native metrics but may cause other maintenance tickets.
>
> Is there any appetite from the community to update this exporter?
>
> Thanks,
>
> Ryan van Huuksloot
> Sr. Production Engineer | Streaming Platform
> [image: Shopify]
> 
>


Re: Flink Metrics

2016-10-18 Thread Aljoscha Krettek
https://ci.apache.org/projects/flink/flink-docs-release-1.2/monitoring/metrics.html
Or this:
https://ci.apache.org/projects/flink/flink-docs-release-1.1/apis/metrics.html
if
you prefer Flink 1.1

On Mon, 17 Oct 2016 at 19:16 amir bahmanyari <amirto...@yahoo.com> wrote:

> Hi colleagues,
> Is there a link that described Flink Matrices & provides example on how to
> utilize it pls?
> I really appreciate it...
> Cheers
>
> --
> *From:* Till Rohrmann <trohrm...@apache.org>
> *To:* u...@flink.apache.org
> *Cc:* dev@flink.apache.org
> *Sent:* Monday, October 17, 2016 12:52 AM
> *Subject:* Re: Flink Metrics
>
> Hi Govind,
>
> I think the DropwizardMeterWrapper implementation is just a reference
> implementation where it was decided to report the minute rate. You can
> define your own meter class which allows to configure the rate interval
> accordingly.
>
> Concerning Timers, I think nobody requested this metric so far. If you
> want, then you can open a JIRA issue and contribute it. The community would
> really appreciate that.
>
> Cheers,
> Till
> ​
>
> On Mon, Oct 17, 2016 at 5:26 AM, Govindarajan Srinivasaraghavan <
> govindragh...@gmail.com> wrote:
>
> > Hi,
> >
> > I am currently using flink 1.2 snapshot and instrumenting my pipeline
> with
> > flink metrics. One small suggestion I have is currently the Meter
> interface
> > only supports getRate() which is always the one minute rate.
> >
> > It would great if all the rates (1 min, 5 min & 15 min) are exposed to
> get
> > a better picture in terms of performance.
> >
> > Also is there any reason why timers are not part of flink metrics core?
> >
> > Regards,
> > Govind
> >
>
>
>


Re: Flink Metrics

2016-10-17 Thread amir bahmanyari
Hi colleagues,Is there a link that described Flink Matrices & provides example 
on how to utilize it pls?I really appreciate it...Cheers

  From: Till Rohrmann <trohrm...@apache.org>
 To: u...@flink.apache.org 
Cc: dev@flink.apache.org
 Sent: Monday, October 17, 2016 12:52 AM
 Subject: Re: Flink Metrics
   
Hi Govind,

I think the DropwizardMeterWrapper implementation is just a reference
implementation where it was decided to report the minute rate. You can
define your own meter class which allows to configure the rate interval
accordingly.

Concerning Timers, I think nobody requested this metric so far. If you
want, then you can open a JIRA issue and contribute it. The community would
really appreciate that.

Cheers,
Till
​

On Mon, Oct 17, 2016 at 5:26 AM, Govindarajan Srinivasaraghavan <
govindragh...@gmail.com> wrote:

> Hi,
>
> I am currently using flink 1.2 snapshot and instrumenting my pipeline with
> flink metrics. One small suggestion I have is currently the Meter interface
> only supports getRate() which is always the one minute rate.
>
> It would great if all the rates (1 min, 5 min & 15 min) are exposed to get
> a better picture in terms of performance.
>
> Also is there any reason why timers are not part of flink metrics core?
>
> Regards,
> Govind
>

   

Re: Flink Metrics

2016-10-17 Thread Till Rohrmann
Hi Govind,

I think the DropwizardMeterWrapper implementation is just a reference
implementation where it was decided to report the minute rate. You can
define your own meter class which allows to configure the rate interval
accordingly.

Concerning Timers, I think nobody requested this metric so far. If you
want, then you can open a JIRA issue and contribute it. The community would
really appreciate that.

Cheers,
Till
​

On Mon, Oct 17, 2016 at 5:26 AM, Govindarajan Srinivasaraghavan <
govindragh...@gmail.com> wrote:

> Hi,
>
> I am currently using flink 1.2 snapshot and instrumenting my pipeline with
> flink metrics. One small suggestion I have is currently the Meter interface
> only supports getRate() which is always the one minute rate.
>
> It would great if all the rates (1 min, 5 min & 15 min) are exposed to get
> a better picture in terms of performance.
>
> Also is there any reason why timers are not part of flink metrics core?
>
> Regards,
> Govind
>