open-telemetry / opentelemetry-collector-contrib

Contrib repository for the OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
3.09k stars 2.38k forks source link

[processor/metricsgeneration] Add option to match metric attributes for calculation operations #35425

Closed crobert-1 closed 1 month ago

crobert-1 commented 1 month ago

Component(s)

processor/metricsgeneration

Describe the issue you're reporting

Context Currently the metrics generation processor simply grabs the first datapoint's value from the metric whose name matches the supplied metric2 configuration option. This value is then used for generating all new metrics and datapoints.

Current implementation's limitation The limitation here is that attributes are ignored when generating a new metric. An example of when this yields unexpected results is for the host metrics receiver's filesystem scraper. Let's say a user wants to calculate total size of the filesystem in bytes using the two metrics system.filesystem.usage and system.filesystem.utilization. This would be done by the following configuration:

experimental_metricsgeneration:
  rules:
    - name: system.filesystem.capacity
       unit: bytes
       type: calculate
       metric1: system.filesystem.usage
       metric2: system.filesystem.utilization
       operation: divide

The problem here is that the attributes of these two metrics may mismatch. One attribute is the filesystem's mountpoint. The resulting metric would incorrectly use the first datapoint's value, regardless of mountpoint. This would result in meaningless generated metrics.

Proposal I propose adding a new configuration option, match_attributes, a bool that's disabled by default. This would preserve current functionality be default.

If this option is enabled, the two datapoints being compared must have identical attributes to create a resulting datapoint in the desired metric. If they're not an identical match, no action will be taken.

Resulting configuration:

experimental_metricsgeneration:
  rules:
    - name: system.filesystem.capacity
       unit: bytes
       type: calculate
       metric1: system.filesystem.usage
       metric2: system.filesystem.utilization
       match_attributes: true
       operation: divide
github-actions[bot] commented 1 month ago

Pinging code owners:

dmitryax commented 1 month ago

I would say that match_attributes should be true by default. match_attributes=false seems like just an incorrect behavior. Maybe we do it via a feature gate instead? If we see users complaining, we can move it to a config option as you suggested, but I wouldn't expect that.

crobert-1 commented 1 month ago

To be honest, I hadn't thought about using a feature gate when I made this proposal. Using a feature gate implies that somewhere down the line users will only be able to do calculations on metrics when all attributes match between two different metrics, as eventually it would be deleted rendering this functionality permanent. So the question becomes: Is there a valid case where attributes don't matter, or simply don't match up exactly that we have to account for?

One edge case here is that in the user may want some attributes to match, but some are irrelevant. For the case I've provided when filing the issue, the match would actually never occur as system.filesystem.usage has an attribute state (enum of value free, reserved, or used) that does not exist for the system.filesystem.utilization metric. Obviously, the datapoint that matters is state: used, and the user would want the match to occur when all overlapping attributes match.

Would it be worthwhile to make the match logic be the "overlapping attributes match" instead of a strict count requirement as well? Maybe this is an invalid use case that we can ignore?

If we did the matching logic this way I could see the possibility of using a feature flag, but I'd be concerned otherwise without more thoroughly vetting possibilities.

dmitryax commented 1 month ago

Yes, I actually meant overlapping. We don't need to require that all attributes match, but if two metrics have any attributes with the same keys but different values, they must not participate in the calculation.

crobert-1 commented 1 month ago

Sounds good, I'll implement using a feature gate. If users present a valid use case for not matching attributes we can revisit accordingly.