Skip to content
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

Implement a CloudWatch EmbeddedMetricFormat (EMF) registry #5304

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ksletmoe-aws
Copy link

I based this heavily off of the existing CloudWatch2 registry and tried to maintain attribution comments where it made sense.

@pivotal-cla
Copy link

@ksletmoe-aws Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@ksletmoe-aws
Copy link
Author

ksletmoe-aws commented Jul 12, 2024

There should already be a corporate CLA signed by my employer (Amazon), associated with my email domain.

@jonatan-ivanov
Copy link
Member

jonatan-ivanov commented Jul 13, 2024

Thank you for the PR!

CLA:
Could you please check what happens if you click on the first link in the comment of the cla-bot? Do you get an error or does the page says that the CLA is signed?
If the latter, could you please click on sync (second link in the comment).

EMF:
Micrometer supports CloudWatch, you don't need to log your metrics out in EMF if you just want to use CloudWatch. Would the CloudWatchMeterRegistry work for you?
See: #3112

@ksletmoe-aws
Copy link
Author

ksletmoe-aws commented Jul 15, 2024

CLA:
I clicked the sync -- it seems it's not finding our agreement. Is it possible for you to check if it's been signed for @amazon.com domains? Our legal team seems to think that it is.

EMF:
In addition to the concerns Qi Chen raised in #3112, EMF is useful if you are already publishing logs and/or metrics with a CloudWatch Agent or Firelens sidecar in ECS, as there's less configuration changes needed. You also don't need to inject a null object CloudWatch client or modify your code to skip setting up a metric registry in a dev environment with EMF -- you can simply just not run an EMF publisher and metrics will not be sent anywhere. Lastly, EMF gives more flexibility for how to handle failed batches because you are offloading the actual publishing to another process, which can be configured to retry batches, keep metrics on disk, etc in the event CloudWatch is unreachable, which in many cases can prevent lost metrics.

@ksletmoe-aws
Copy link
Author

I spoke with @jonatan-ivanov on Slack, and they were able to help me determine that the CLA has been signed by somebody in the amzn github organization. I joined that organization. Trying to sync the PR via the above link but still no dice on the CLA approval workflow. Suspect there might be some propagation delay

@pivotal-cla
Copy link

@ksletmoe-aws Thank you for signing the Contributor License Agreement!

@ksletmoe-aws
Copy link
Author

Wanted to check in and see if this review has any feedback or will be accepted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants