Allow setting timestamp of metrics. #69
Conversation
Adds the MetricsLogger.setTimestamp() call as discussed in awslabs#61
|
Thanks for the PR! I think this is the correct behavior. Preserving timestamp across flushes makes sense. |
README.md
Outdated
|
|
||
| Sets the CloudWatch [timestamp](https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/cloudwatch_concepts.html#about_timestamp) that extracted metrics are associated with. If not set a default value of `new Date()` will be used. | ||
|
|
||
| If set, timestamp will be preserved across calls to flush(). |
There was a problem hiding this comment.
...for a given MetricsLogger instance.
There was a problem hiding this comment.
Clarified in a subsequent commit.
README.md
Outdated
|
|
||
| Examples: | ||
|
|
||
| ```py |
There was a problem hiding this comment.
Have updated, also removed the single other use of ```py that lead to my copypasta fail.
- also change other snippet (unrelated to this change) from py to js
|
@jaredcnance I can't see the CodeBuild failures but given Does the build maybe need a retry? If you find it is something with the code please let me know and I'll try to reproduce and address it. * I know, I know, "works on my machine" 😆 |
|
Build was failing because we are getting throttled by Docker Hub when pulling the CW Agent. We need to switch to using ECR, I believe. I rebuilt it and it passed, but I'm not sure why the status check hasn't been reported back to GitHub. I'm approving and merging the PR. Thanks! |
Issue #, if available: 61
#61
Description of changes:
Adds the MetricsLogger.setTimestamp() method.
One aspect I think core maintainers will have a much better sense of, if I set the timestamp should that be preserved across calls to
flush()? I wasn't 100% sure what would be desired behaviour, so I took a guess and went for "yes, if set explicitly, preserve". For my own use case described in #61, I will be callingsetTimeoutin 100% ofMetricsContextinstances generated, so it would be irrelevant anyway.Another guess I made was that if the timestamp is not set explicitly, the
MetricsContextshould reset themeta.Timestampfield tonew Date(). I think this is less surprising behaviour for users who never callsetTimestamp. Plus if I preserved the timestamp when not explicitly set, that would change the behaviour for all existing users, I thought it would only make sense to change behaviour for callers who are "opting in" by usingsetTimeout.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.