gortiz opened a new pull request, #9416: URL: https://github.com/apache/pinot/pull/9416
We currently use Yammer as the metric library, but Yammer is not maintained and it doesn’t have new features other more modern metric libraries (like Drowizard or Micrometer) have. For example, Yammer histograms are misleading in some situations (I strongly recommend to read [this article](https://medium.com/expedia-group-tech/your-latency-metrics-could-be-misleading-you-how-hdrhistogram-can-help-9d545b598374) to learn more about the problem). This issue also affects Dropwizard (in fact the referenced article is focused on Dropwizard) but Dropwizard has evolved in a way that the problem can be fixed by changing the reservoir. We already have a Dropwizard implementation, but we cannot currently use it because it produces metrics with other JMX names, which could break alerts and dashboards our users have right now. At this moment this PR is a draft on which we can discuss, not a ready to merge PR. This PR changes some default values and therefore we would need to test it properly in environments we can break. This PR creates a new metric plugin called _compound_. This metric plugin contains a list of other metric plugins. Each time a metric is registered or unregistered in the _compound_ registry, it is registered or unregistered in all other metric plugins. The metric plugins that are notified by the _compound_ registry can be configured, but by default it notifies all other metric plugins in the classpath. By using this _compound_ metric plugin and including both Dropwizard and Yammer plugins in the classpath, Pinot will register each metric twice: One in Yammer and one in Dropwizard. As said above, each registry produces its own JMX names, so alerts and dashboards created based on the Yammer metrics will continue to work as expected, but given that Dropwizard JMX names will also be published, dashboard and alerts can be migrated to use the new ones and therefore can use the new features. This PR also adds the ability to change the _domain_ on which Dropwizard metrics are published by changing the property `pinot.metrics.dropwizard.domain`, whose default value is `metrics`, following the Dropwizard default domain. This domain is the prefix used by Dropwizard to create its MBean names, which are something like `"<domain>";type="<type>";name="<metric_name>"`. By default a Pinot metric called `myTimer` of type `timer` in the Pinot server is exposed: - by Yammer as `"org.apache.pinot.common.metrics";type="ServerMetrics";name="myTymer"` - by Dropwizard as `"metrics";type="timers";name="myTymer"` When the new `pinot.metrics.dropwizard.domain` property is changed to `org.apache.pinot.common.metrics`, Dropwizard will export the metric as `"org.apache.pinot.common.metrics";type="timer";name="myTymer"`. As far as I know it is not possible to tell Dropwizard to use other value as type. It always use the type of the metric (gauge, timer, histogram, counter, etc). Therefore, with this property we are able to create closer names than the one created with Yammer, but not exactly the same. To palliate that, this PR relax the default prometheus JMX mappers, removing the `BrokerMetrics`, `ControllerMetrics`, `ServerMetrics`, `MinionMetrics` and `ValidationMetrics` type literals and changing them for any word. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
