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