chore: parse string values for __value filter

[srikanthccv]

Summary

Certain metrics encode the state as the value, such as pod phase https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/k8sclusterreceiver/documentation.md#k8spodphase, making it difficult to aggregate and filter. For pod phase there is no way to filter by pending pods because there is no label but only a special value. To work around this, we added a special filter called "__value". If we find this in the search items. We assume that the user wants to filter on the value and add value condition. It is not working when the value type is a string. This change checks if the string value can be converted, if so add metric value filter with converted. Otherwise, warn and skip the metric values filter.

---

Important

AddMetricValueFilter now supports filtering on string values for __value by converting them to float, with tests added.

• Behavior:
  • AddMetricValueFilter in query_builder.go now supports string values for __value filter by attempting conversion to float.
  • Logs a warning and skips filter if conversion fails.
• Testing:
  • Added test case in query_builder_test.go to verify filtering with string values that can be converted to numbers.

^{This description was created by for 96df934. It will automatically update as commits are pushed.}

[ellipsis-dev]

❌ Changes requested. Reviewed everything up to 96df934 in 59 seconds

More details

• Looked at 70 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 3 drafted comments based on config settings.

1. pkg/query-service/app/metrics/v4/query_builder_test.go:511

• Draft comment:
  Consider adding a negative test case for a __value string that cannot be parsed (non-numeric) so that it verifies the warning log and that the filter is skipped.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50%
  The comment points out a valid gap in test coverage - we only test the success case but not error handling. However, I need to consider if this meets the bar for a required code change. The current test verifies core functionality. Error handling tests would be nice-to-have but may not be essential.
  The comment is speculative - we don't know if there actually is warning log and filter skip behavior implemented. Without seeing the implementation code, we can't be sure this test would be testing real behavior.
  While speculative, the suggestion is reasonable - any robust implementation should handle invalid input gracefully. Testing error cases is generally good practice.
  The comment should be deleted as it is speculative and not clearly tied to a required code change. While the suggestion has merit, it doesn't meet the bar for a must-fix comment.

2. pkg/query-service/app/metrics/query_builder.go:69

• Draft comment:
  Consider including the actual string value in the warn log (e.g. using zap.String("value", v)) for easier debugging.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

3. pkg/query-service/app/metrics/v4/query_builder_test.go:511

• Draft comment:
  Consider adding a test case with a non-numeric string to verify that an invalid __value filter is gracefully skipped.
• Reason this comment was not posted:
  Marked as duplicate.

Workflow ID: wflow_PLeJxv7KwQc33B8K

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.