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 <ryan.vanhuuksl...@shopify.com.invalid> 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&utm_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 > > <ryan.vanhuuksl...@shopify.com.invalid> 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&utm_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 > > > > <ryan.vanhuuksl...@shopify.com.invalid> 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&utm_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&utm_source=hs_email> > > > > > > > > > > > > > > > >
