-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[metrics_generation_processor] Add metrics generation logic #3433
[metrics_generation_processor] Add metrics generation logic #3433
Conversation
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Will address the PR feedbacks. |
Need to improve unit test coverage. Do not merge. |
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
@hossain-rayhan did you improve the unit-tests? Looks like this is on you |
I marking as draft since the author intends to add unit tests. |
Sorry, I was little bit distracted. Yes, I am back on this. Will send the update soon. Thanks @bogdandrutu and @tigrannajaryan |
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Ahh, this one is also on me! Couldn't prioritize this week. Sorry for the delay and I am taking all the blames. Hopefully I will update the PR early Monday. |
Signed-off-by: Rayhan Hossain <[email protected]>
Signed-off-by: Rayhan Hossain <[email protected]>
6f362f3
to
0fd5e73
Compare
Signed-off-by: Rayhan Hossain <[email protected]>
Hi @bogdandrutu, finally I finished adding unit tests and improve coverage for my code here. Would you please take a look? Also, I am not sure if I have the permission to change the status from |
Updates #3104 Signed-off-by: Bogdan Drutu [email protected]
…alculations (#35428) **Description:** The metrics generation processor would previously hit a panic when the calculation rule was operating on a `sum` metric, instead of a `gauge`. The [component proposal](#2722), [initial implementation](#3433), nor the [README](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/processor/metricsgenerationprocessor) state that it's a requirement for the metric to be a `gauge`, nor do I think this requirement makes sense. I've updated the logic to account for sum metrics being used and added a test. **Testing:** Added a test for this situation, the test panics before the change and passes after.
…alculations (open-telemetry#35428) **Description:** The metrics generation processor would previously hit a panic when the calculation rule was operating on a `sum` metric, instead of a `gauge`. The [component proposal](open-telemetry#2722), [initial implementation](open-telemetry#3433), nor the [README](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/processor/metricsgenerationprocessor) state that it's a requirement for the metric to be a `gauge`, nor do I think this requirement makes sense. I've updated the logic to account for sum metrics being used and added a test. **Testing:** Added a test for this situation, the test panics before the change and passes after.
Description:
This PR adds the actual metrics generation logic based on configured rules in the
experimental_metrics_generation
processor.Link to tracking Issue:
#2722
Testing:
Manually tested.
I am adding unit tests to improve coverage. Created the PR to have some early feedback.
Documentation:
README updated.