feat: add job level options and labels_to_snake_case to CW exporter #5015
Conversation
| Period time.Duration `alloy:"period,attr,optional"` | ||
| Length time.Duration `alloy:"length,attr,optional"` | ||
| Delay time.Duration `alloy:"delay,attr,optional"` | ||
| //TODO: Remove NilToZero, because it is deprecated upstream. |
There was a problem hiding this comment.
thought: I actually could not find any reference to this being a deprecated options, not in the YACE docs nor in the code itself so decided to remove these TODOs.
| DimensionNameRequirements: rj.DimensionNameRequirements, | ||
| // By setting RoundingPeriod to nil, the exporter will align the start and end times for retrieving CloudWatch | ||
| // metrics, with the smallest period in the retrieved batch. | ||
| RoundingPeriod: nil, |
There was a problem hiding this comment.
thought: removed RoundingPeriod as an option as this IS deprecated according to YACE: https://github.com/prometheus-community/yet-another-cloudwatch-exporter/blob/0c9677d91836f0a4150a55172a0ce5081574b407/pkg/config/config.go#L258
There was a problem hiding this comment.
thought: during writing and fixing of tests I often found myself switching a lot between the the alloy config and the expected model so I decided to:
- re-order the tests a bit more logically
- Move the expected model close to the input Alloy config
IMHO this makes it bit easier to find the relation between the config and expected model and makes it easier to debug when a test is failing.
Let me know if y'all have other thoughts.
| } | ||
|
|
||
| func toYACEMetrics(ms []Metric, jobNilToZero *bool) []*yaceConf.Metric { | ||
| func toYACEMetrics(ms []Metric, jobPeriod time.Duration, jobLength time.Duration) []*yaceConf.Metric { |
There was a problem hiding this comment.
question/thought: AFAICT, basically to only reason we still need this function is because in Alloy we decided to automatically align the length with the period window if the length is not defined.
As this is something that YACE does not do, do we actively want to patch a different behavior from upstream in Alloy?
|
Hey @kgeckhart hope you don't mind me tagging you directly: If you wouldn't mind, could you give me your thoughts on this PR? Is this something Grafana Alloy would like as a contribution? This PR still requires some final polish, but would like a general opinion on the direction. |
1 participant
PR Description
This PR adds job level
period,length, andadd_cloudwatch_timestampsupport to both discovery and custom namespace jobs. Futhermore it expose a top levellabels_snake_caseoption.Which issue(s) this PR fixes
Closes #398
Notes to the Reviewer
PR Checklist