Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[processor/metricsgeneration] Metrics should be calculated when second metric value is 0 #35533

Closed
crobert-1 opened this issue Oct 1, 2024 · 3 comments · Fixed by #35537
Closed
Labels
processor/metricsgeneration Metrics Generation processor

Comments

@crobert-1
Copy link
Member

Component(s)

processor/metricsgeneration

Describe the issue you're reporting

Bug
The metrics generation processor is used to create new metrics by using calculations on existing metrics. As explained in the configuration example here, the config specifies the first metric, second metric, and operation type for calculations.

Existing logic for this processor simply skips the calculation when the value of the second metric's datapoint is 0 (reference). This means that even when it's a valid operation, and the result may be relevant for users, no metric is created.

Example showing bug
Relevant example configuration:

experimental_metricsgeneration:
  rules:
    - name: available_capacity
      unit: bytes
      type: calculate
      metric1: total_capacity
      metric2: used_capacity
      operation: subtract

This configuration should create a new metric called available_capacity, which will be the result of total_capacity - used_capacity. If the value of used_capacity is 0, the processor won't create a new metric called available_capacity, when I believe it should.

Proposed Solution
I think the solution here is to allow the calculation to be done and to create the resulting metric.

Open Question
How should we handle the divide and percent operations for the case of dividing by 0? We can either add a data point with a value of 0 or not add any data point at all. Since dividing by 0 is an invalid operation, I believe the right thing to do would be log an error and not add a data point in that case. I'm open to feedback from others though.

@crobert-1 crobert-1 added the needs triage New item requiring triage label Oct 1, 2024
Copy link
Contributor

github-actions bot commented Oct 1, 2024

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@dmitryax
Copy link
Member

dmitryax commented Oct 2, 2024

Since dividing by 0 is an invalid operation, I believe the right thing to do would be log an error and not add a data point in that case.

I agree

@crobert-1
Copy link
Member Author

Had a discussion with Dmitrii offline, we also agreed that if no valid data points are generated for the new metric, the new metric should not be created. I'll add a note in the README to clarify this for users.

dmitryax pushed a commit that referenced this issue Oct 2, 2024
…etric's value is 0 (#35537)

**Description:**
Originally, this processor would not generate a new metric if the value
of the metric configured as `metric2` was `0`. There are valid reasons
why this calculation should still be done, and a new metric should be
calculated.

**Link to tracking Issue:** Resolves #35533

**Testing:**
Added tests for each calculation type to show expected results when the
second metric's value is 0.
jriguera pushed a commit to springernature/opentelemetry-collector-contrib that referenced this issue Oct 4, 2024
…etric's value is 0 (open-telemetry#35537)

**Description:**
Originally, this processor would not generate a new metric if the value
of the metric configured as `metric2` was `0`. There are valid reasons
why this calculation should still be done, and a new metric should be
calculated.

**Link to tracking Issue:** Resolves open-telemetry#35533

**Testing:**
Added tests for each calculation type to show expected results when the
second metric's value is 0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
processor/metricsgeneration Metrics Generation processor
Projects
None yet
3 participants