Repository navigation
implement CloudWatch metrics - #69
Conversation
| MetricPublisher( | ||
| event.awsAccountId, event.resourceType, provider_sess | ||
| ) | ||
| ) |
There was a problem hiding this comment.
this looks like a case to me where breaking from the Java interface could be beneficial? we could backport changes or simply say it's different; the metrics stuff is old and the log delivery was patched in hastily from what i can see. for example, seems like the only difference between the publishers is the session. that means the proxy could take the awsAccountId and resourceType, and maybe add_metrics_publisher would just be called with credentials to add another one?
it's up to you. either one of us could do this in a separate PR, too. more of a brainstorm, since it's perfectly functional as is.
There was a problem hiding this comment.
let's get this merged to get the functionality available and can improve in future pr's
|
Aside from travis failing on python 3.8, it looks like rebasing on the current master has caused some issues when I test end to end, investigating. Update: e2e tests just needed |
tobywf
left a comment
tobywf
left a comment
There was a problem hiding this comment.
friendly reminder to not rebase PRs but merge in master instead as it breaks almost all links and reviewing tools and makes spotting any merge issues a lot harder. i guess if it builds it's probably okay
3 participants
Issue #, if available: Fixes #22
Description of changes:
Wires up metrics to publish to cwl in platfom acc and provider acc (if configured).
refactored
metrics.pyto handler multiple boto3 clients per publishing event and address a few gaps with the java plugin.Tested end to end, metrics showing up as expected in provider account.
Resource.__call__is becoming unwieldy, could use a refactor, but didn't want to muddy this pr with more refactors, so will address in a future pr.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.