-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Update AWS Cloudwatch mappings #14653
Conversation
AWS cloudwatch metrics were being reported under `aws.metrics`, but the mapping definitions in `fields.yml` were expecting these under `aws.cloudwatch.metrics`. This change fixes that and refactors how metrics are handled when used together with Lightweight modules. Metrics should now include the metricset name in the path, example: For cloudwatch metricset, metrics look like: ``` aws.cloudwatch.metrics.CPUUtilization.p10 aws.cloudwatch.dimensions.InstanceId ``` While when used from a lightweight module it produces: ``` aws.ebs.metrics.BurstBalance.avg aws.cloudwatch.dimensions.VolumeId ``` I kept dimensions together as sometimes these can be used for correlation, so having them under the same field helps.
I've played with this while thinking on #14662, would love to hear opinions here |
Thanks @exekias for starting this! One thing I'm concerned about is the inconsistency of ebs/cloudwatch in light module use case:
metrics and dimensions are both concepts from AWS Cloudwatch. It makes sense with metrics from cloudwatch metricset but for light modules, will this difference be confusing a bit? |
Sorry you are right, I actually changed them to |
Thanks! I'm good with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM besides changelog and needs to run mage fmt update
.
closing in favor of #15245 |
AWS cloudwatch metrics were being reported under
aws.metrics
, but themapping definitions in
fields.yml
were expecting these underaws.cloudwatch.metrics
.This change fixes that and refactors how metrics are handled when used
together with Lightweight modules. Metrics should now include the
metricset name in the path, example:
For cloudwatch metricset, metrics look like:
While when used from a lightweight module it produces:
I kept dimensions together as sometimes these can be used for
correlation, so having them under the same field helps.
TODO: