Support for exposing atttributes as labels

[MaBiConti]

The mbeans that we are working with have a lot of string-type attributes which are not converted into metrics by your project. However, we need to expose the information offered by these string attributes to Prometheus, such that they can be used in queries, or be displayed in graphs. The best way to do this (that we were able to find) is to expose these attributes as labels on the other metrics. We made this behavior configurable in the rules section of the yaml configuration file. Here is an example:

rules:
  - pattern: '{some mbean pattern}'
    name: some_metric
    attributesAsLabels:
       - firstStringAttribute
       - secondStringAttribute

Given an mbean having the attributes:

duration=123
firstStringAttribute="something"
secondStringAttribute="something else"

This produces a metric that looks like this:

some_metric_duration{firstStringAttribute="something",secondStringAttribute="something else"} 123

[dhoard]

@MaBiConti ...

1. I want to discuss this with @fstab since adding pattern matching can negatively impact performance.

private Map<String, String> getAttributesAsLabelsWithValues(ObjectName mBeanName, Attribute attribute, Map<String, Object> attributeMap) {
// ... code omitted ...
}

2, If the functionality/PR is considered for inclusion, it requires both unit and integration tests. (I would hold off on implementation the tests pending the outcome regarding inclusion.)

[karina-calma]

Hi @dhoard, we noted your performance concern, and made a slight improvement by avoiding pattern matching if the feature is not used. Technically, if one does not explicitly configure attributesAsLabels, they should see no impact to performance. However, when the feature is used, we need to match every single rule to every single metric. We noted that this matching is already performed in JmxCollector, but we could not find a way to easily reuse that without an extensive refactoring. If you have any suggestions in this direction, please advise.

[karina-calma]

Hi @dhoard, Have you made a decision regarding this new functionality? Or do you need more time to consider it? Considering that performance will not be impacted if the feature is not used, do you still think that that pattern matching is a problem? We would appreciate an update when you have a chance.

[dhoard]

@karina-calma after discussing with the team, we have chosen not to implement this at this time due to the performance impact.

As a workaround, you can created and register your own collector to expose such metrics.

[karina-calma]

Hi @dhoard ,

We would like to propose an alternative implementation for the functionality shown initially in order to expose to Prometheus the information offered by string-type attributes, which are not converted into metrics by your project. The new functionality would be enabled through a block of metricCustomizers in the yaml configuration file as follows:

startDelaySeconds: 0
jmxUrl: <jmxUrl>
metricCustomizers:
  - mbeanFilter:
       domain: '{domain}'
       properties:
         '{property1}': '{value1}'
    attributesAsLabels:
      - firstStringAttribute
      - secondStringAttribute
rules:
  - pattern: ".*"

Technically, if one does not explicitly configure metricCustomizers, they should see no impact to performance. When the feature is used, we will go through all the elements in the metricCustomizers block and perform string comparison (not pattern matching) on the domain and the (optional) properties with every MBean that is matched by the rules defined in the config file. For those metrics that match, we will add the elements from the attributesAsLabels block as attributes to the existing metrics.

[dhoard]

@karina-calma the method...

private Map<String, String> getAttributesAsLabelsWithValues(ObjectName mBeanName, Attribute attribute, Map<String, Object> attributeMap) {

... is doing regex pattern matching ...

                    Matcher matcher = rule.pattern.matcher(matchName);
                    if (matcher.matches()) {

... which will cause performance issues when configured.

Introducing core/built-in functionality (even if disabled by default) will be configured and used by end users, resulting in performance complaints.

[karina-calma]

Hi @dhoard ,

We just updated the code, when you have time, please take another look.

[MaBiConti]

Hi @dhoard , originally, we just want to get your feedback whether you would accept the approach. I consider a dedicated metricCustomizers configuration block to be not as elegant as a customization directly in the rule, but it has clear performance benefits.

And we did actually see the performance issues you were concerned about on our own system. That's why we went ahead and changed the implementation, which we just pushed.

This code is still missing the automatic tests, but we'd like to get your feedback on the approach first before writing the tests.