[receiver/awsfirehose] Support all possible quantiles of CloudWatch metrics #39687
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
axw
reviewed
Apr 29, 2025
There was a problem hiding this comment.
@yiquanzhou looks good. I'm pretty sure you can specify additional percentiles, but I think that could be handled as a followup. If you'd prefer to defer, maybe just update the changelog note to clarify that this only handles a fixed set of percentiles?
| component: awsfirehosereceiver | ||
|
|
||
| # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
| note: support all possible quantile values (0.7, 0.8, 0.9, 0.95, 0.99) of CloudWatch metrics in addition to quantile 0 (min) and 1 (max) |
There was a problem hiding this comment.
IIANM you can specify arbitrary percentiles, and you're not restricted to the ones in the drop down in CloudWatch
Comment on lines
53
to
62
| // P70 is the 70th percentile of observed values. This value is optional. | ||
| P70 *float64 `json:"p70"` | ||
| // P80 is the 80th percentile of observed values. This value is optional. | ||
| P80 *float64 `json:"p80"` | ||
| // P90 is the 90th percentile of observed values. This value is optional. | ||
| P90 *float64 `json:"p90"` | ||
| // P95 is the 95th percentile of observed values. This value is optional. | ||
| P95 *float64 `json:"p95"` | ||
| // P99 is the 99th percentile of observed values. This value is optional. | ||
| P99 *float64 `json:"p99"` |
There was a problem hiding this comment.
Maybe this is good enough to start with, but to handle arbitrary percentiles we would need to either decode the stats to a map and parse the keys, or otherwise stream parse the stats.
There was a problem hiding this comment.
Thanks for the review. I adjusted the code to support any arbitrary percentile value
atoulme
reviewed
Apr 29, 2025
receiver/awsfirehosereceiver/internal/unmarshaler/cwmetricstream/cwmetric.go
Outdated
Show resolved
Hide resolved
axw
reviewed
Apr 29, 2025
receiver/awsfirehosereceiver/internal/unmarshaler/cwmetricstream/unmarshaler.go
Outdated
Show resolved
Hide resolved
…am/cwmetric.go Co-authored-by: Antoine Toulme <[email protected]>
…am/unmarshaler.go Co-authored-by: Andrew Wilkins <[email protected]>
atoulme
approved these changes
May 2, 2025
vincentfree
pushed a commit
to ing-bank/opentelemetry-collector-contrib
that referenced
this pull request
May 6, 2025
…etrics (open-telemetry#39687) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description This PR is an enhancement to the awsfirehose receiver. By default, CloudWatch metric streams only include four statistics – Minimum, Maximum, Sample Count and Sum. But we can add additional statistics for specific metrics, including p70, p80, p90, p95 and p99 percentiles: <img width="718" alt="image" src="https://github.com/user-attachments/assets/fe17bced-9c00-4082-a0a3-9b520829bd70" /> These additional statistics will be added to the firehose request payload. Example: ``` { "metric_stream_name": "firehose-metrics-stream", "account_id": "1234567890", "region": "eu-west-1", "namespace": "AWS/EC2", "metric_name": "CPUUtilization", "dimensions": { "InstanceId": "i-0a05ddddd2acb83a7" }, "timestamp": 1745849100000, "value": { "max": 31.435, "min": 25.83666666666667, "sum": 140.94833333333332, "count": 5, "p99": 31.40867565167042, "p90": 31.172746310692506, "p95": 31.303598519605032 }, "unit": "Percent" } ``` These values are currently ignored during unmarshalling. This PR adds these fields as optional fields during deserialization and adds the values to the summary quantiles. <!--Describe what testing was performed and which tests were added.--> #### Testing Enrich existing unit test. --------- Co-authored-by: Antoine Toulme <[email protected]> Co-authored-by: Andrew Wilkins <[email protected]>
vincentfree
pushed a commit
to ing-bank/opentelemetry-collector-contrib
that referenced
this pull request
May 20, 2025
…etrics (open-telemetry#39687) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description This PR is an enhancement to the awsfirehose receiver. By default, CloudWatch metric streams only include four statistics – Minimum, Maximum, Sample Count and Sum. But we can add additional statistics for specific metrics, including p70, p80, p90, p95 and p99 percentiles: <img width="718" alt="image" src="https://github.com/user-attachments/assets/fe17bced-9c00-4082-a0a3-9b520829bd70" /> These additional statistics will be added to the firehose request payload. Example: ``` { "metric_stream_name": "firehose-metrics-stream", "account_id": "1234567890", "region": "eu-west-1", "namespace": "AWS/EC2", "metric_name": "CPUUtilization", "dimensions": { "InstanceId": "i-0a05ddddd2acb83a7" }, "timestamp": 1745849100000, "value": { "max": 31.435, "min": 25.83666666666667, "sum": 140.94833333333332, "count": 5, "p99": 31.40867565167042, "p90": 31.172746310692506, "p95": 31.303598519605032 }, "unit": "Percent" } ``` These values are currently ignored during unmarshalling. This PR adds these fields as optional fields during deserialization and adds the values to the summary quantiles. <!--Describe what testing was performed and which tests were added.--> #### Testing Enrich existing unit test. --------- Co-authored-by: Antoine Toulme <[email protected]> Co-authored-by: Andrew Wilkins <[email protected]>
dragonlord93
pushed a commit
to dragonlord93/opentelemetry-collector-contrib
that referenced
this pull request
May 23, 2025
…etrics (open-telemetry#39687) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description This PR is an enhancement to the awsfirehose receiver. By default, CloudWatch metric streams only include four statistics – Minimum, Maximum, Sample Count and Sum. But we can add additional statistics for specific metrics, including p70, p80, p90, p95 and p99 percentiles: <img width="718" alt="image" src="https://github.com/user-attachments/assets/fe17bced-9c00-4082-a0a3-9b520829bd70" /> These additional statistics will be added to the firehose request payload. Example: ``` { "metric_stream_name": "firehose-metrics-stream", "account_id": "1234567890", "region": "eu-west-1", "namespace": "AWS/EC2", "metric_name": "CPUUtilization", "dimensions": { "InstanceId": "i-0a05ddddd2acb83a7" }, "timestamp": 1745849100000, "value": { "max": 31.435, "min": 25.83666666666667, "sum": 140.94833333333332, "count": 5, "p99": 31.40867565167042, "p90": 31.172746310692506, "p95": 31.303598519605032 }, "unit": "Percent" } ``` These values are currently ignored during unmarshalling. This PR adds these fields as optional fields during deserialization and adds the values to the summary quantiles. <!--Describe what testing was performed and which tests were added.--> #### Testing Enrich existing unit test. --------- Co-authored-by: Antoine Toulme <[email protected]> Co-authored-by: Andrew Wilkins <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR is an enhancement to the awsfirehose receiver.
By default, CloudWatch metric streams only include four statistics – Minimum, Maximum, Sample Count and Sum. But we can add additional statistics for specific metrics, including p70, p80, p90, p95 and p99 percentiles:
These additional statistics will be added to the firehose request payload. Example:
These values are currently ignored during unmarshalling. This PR adds these fields as optional fields during deserialization and adds the values to the summary quantiles.
Testing
Enrich existing unit test.