Added support for more than 100 metrics #31
Conversation
| if should_serialize: | ||
| event_batches.append(json.dumps(body)) | ||
| metric_pointers = [] | ||
| body["_aws"]["CloudWatchMetrics"][0]["Metrics"] = metric_pointers |
There was a problem hiding this comment.
do we ever reset the body? If I'm reading this correctly, all JSON payloads will contain all the metric values just not the pointers? For example, in the node implementation, we recreate a new body every time we flush: https://github.com/awslabs/aws-embedded-metrics-node/blob/730bbaeb341ee3952a404c04fc1de59a6ec65cf5/src/serializers/LogSerializer.ts#L71
There was a problem hiding this comment.
You are correct, JSON payload will contain the metric values. As per my understanding, We need to reset Metrics ( body["_aws"]["CloudWatchMetrics"][0]["Metrics"] ) and use the same value for other fields - Dimensions, Namespace, etc. Please correct me if am wrong.
Thanks,
Abdul
There was a problem hiding this comment.
We need to reset the body as well to prevent the payload from continuing to grow with metrics that aren't referenced. You're removing it from the pointers which is good, but not from the body which get added into ln54.
There was a problem hiding this comment.
Thanks for pointing it out. Fixed.
Issue #: #10
Description of changes:
Added support for more than 100 metrics
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.