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/transform] Add functionality to break up a summary metric into Sum metrics. #11041

Merged
merged 11 commits into from
Jun 21, 2022

Conversation

Mrod1598
Copy link
Contributor

Description:
Resolves #10824

Testing:
Added tests for new functions. Tested processor against a real vault instance as well using the prometheus receiver.

Documentation:
Added To README describing the new functions.

@Mrod1598 Mrod1598 force-pushed the expand-transform-processor branch from 560f5a5 to 0ac6815 Compare June 14, 2022 20:05
@TylerHelmuth
Copy link
Member

@Mrod1598 I haven't reviewed this yet but I noticed the new functions are in functions.go. As part of #10101 all new functions should be in their own files. Checkout #11019 if you want to see the future CONTRIBUTING guide for functions.

@Mrod1598 Mrod1598 marked this pull request as ready for review June 15, 2022 14:06
@Mrod1598 Mrod1598 requested a review from a team June 15, 2022 14:06
Copy link
Member

@TylerHelmuth TylerHelmuth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tackling this. Please add some integration tests to internal/metrics/processor_test.go for the 2 new functions.

Copy link
Member

@TylerHelmuth TylerHelmuth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit

processor/transformprocessor/README.md Outdated Show resolved Hide resolved
@TylerHelmuth
Copy link
Member

/cc @anuraaga

Copy link
Member

@TylerHelmuth TylerHelmuth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Mrod1598
Copy link
Contributor Author

@TylerHelmuth thanks for Reviewing! Can this merge or are we waiting on another review?

@TylerHelmuth
Copy link
Member

We like to get at least 2 approvals for most things. @anuraaga @open-telemetry/collector-contrib-approvers please review.

Copy link
Member

@djaglowski djaglowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@djaglowski djaglowski merged commit b7d9045 into open-telemetry:main Jun 21, 2022
@Mrod1598 Mrod1598 deleted the expand-transform-processor branch June 21, 2022 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[receiver/prometheus] Convert Summary metric into two monotonic sums.
5 participants