-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[receiver/awscloudwatchmetricsreceiver] Add new receiver #19429
[receiver/awscloudwatchmetricsreceiver] Add new receiver #19429
Conversation
Foresight Summary View More Details⭕ build-and-test-windows workflow has finished in 8 seconds (33 minutes 31 seconds less than
|
Job | Failed Steps | Tests | |
---|---|---|---|
windows-unittest-matrix | - 🔗 | N/A | See Details |
windows-unittest | - 🔗 | N/A | See Details |
✅ telemetrygen workflow has finished in 1 minute 17 seconds and finished at 27th Mar, 2023.
Job | Failed Steps | Tests | |
---|---|---|---|
build-dev | - 🔗 | N/A | See Details |
publish-latest | - 🔗 | N/A | See Details |
publish-stable | - 🔗 | N/A | See Details |
✅ check-links workflow has finished in 1 minute 45 seconds (⚠️ 40 seconds more than main
branch avg.) and finished at 27th Mar, 2023.
Job | Failed Steps | Tests | |
---|---|---|---|
changed files | - 🔗 | N/A | See Details |
check-links | - 🔗 | N/A | See Details |
✅ changelog workflow has finished in 2 minutes 30 seconds and finished at 27th Mar, 2023.
Job | Failed Steps | Tests | |
---|---|---|---|
changelog | - 🔗 | N/A | See Details |
build-and-test workflow has finished in 24 minutes 10 seconds (25 minutes 28 seconds less than main
branch avg.) and finished at 27th Mar, 2023.
Job | Failed Steps | Tests | |
---|---|---|---|
unittest-matrix (1.20, connector) | N/A | ✅ 126 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, connector) | N/A | ✅ 126 ❌ 0 ⏭ 0 🔗 | See Details |
correctness-metrics | N/A | ✅ 2 ❌ 0 ⏭ 0 🔗 | See Details |
correctness-traces | N/A | ✅ 17 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.20, internal) | N/A | ✅ 583 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, extension) | N/A | ✅ 544 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.20, extension) | N/A | ✅ 544 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, internal) | N/A | ✅ 583 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, processor) | N/A | ✅ 1557 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.20, processor) | N/A | ✅ 1557 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, receiver-0) | N/A | ✅ 2631 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.20, receiver-0) | N/A | ✅ 2631 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.20, exporter) | N/A | ✅ 2505 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, receiver-1) | N/A | ✅ 1965 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.20, receiver-1) | N/A | ✅ 1965 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, exporter) | N/A | ✅ 2505 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, other) | N/A | ✅ 4756 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.20, other) | N/A | ✅ 4756 ❌ 0 ⏭ 0 🔗 | See Details |
integration-tests | N/A | ✅ 55 ❌ 0 ⏭ 0 🔗 | See Details |
✅ prometheus-compliance-tests workflow has finished in 10 minutes 15 seconds (⚠️ 3 minutes 54 seconds more than main
branch avg.) and finished at 27th Mar, 2023.
Job | Failed Steps | Tests | |
---|---|---|---|
prometheus-compliance-tests | - 🔗 | ✅ 21 ❌ 0 ⏭ 0 🔗 | See Details |
✅ load-tests workflow has finished in 15 minutes 21 seconds (⚠️ 4 minutes 27 seconds more than main
branch avg.) and finished at 27th Mar, 2023.
Job | Failed Steps | Tests | |
---|---|---|---|
loadtest (TestIdleMode) | - 🔗 | ✅ 1 ❌ 0 ⏭ 0 🔗 | See Details |
loadtest (TestTraceAttributesProcessor) | - 🔗 | ✅ 3 ❌ 0 ⏭ 0 🔗 | See Details |
loadtest (TestMetric10kDPS|TestMetricsFromFile) | - 🔗 | ✅ 6 ❌ 0 ⏭ 0 🔗 | See Details |
loadtest (TestTraceNoBackend10kSPS|TestTrace1kSPSWithAttrs) | - 🔗 | ✅ 8 ❌ 0 ⏭ 0 🔗 | See Details |
loadtest (TestMetricResourceProcessor|TestTrace10kSPS) | - 🔗 | ✅ 12 ❌ 0 ⏭ 0 🔗 | See Details |
loadtest (TestTraceBallast1kSPSWithAttrs|TestTraceBallast1kSPSAddAttrs) | - 🔗 | ✅ 10 ❌ 0 ⏭ 0 🔗 | See Details |
loadtest (TestBallastMemory|TestLog10kDPS) | - 🔗 | ✅ 18 ❌ 0 ⏭ 0 🔗 | See Details |
setup-environment | - 🔗 | N/A | See Details |
✅ e2e-tests workflow has finished in 17 minutes 41 seconds (⚠️ 2 minutes 50 seconds more than main
branch avg.) and finished at 27th Mar, 2023.
Job | Failed Steps | Tests | |
---|---|---|---|
kubernetes-test (v1.26.0) | - 🔗 | N/A | See Details |
kubernetes-test (v1.25.3) | - 🔗 | N/A | See Details |
kubernetes-test (v1.24.7) | - 🔗 | N/A | See Details |
kubernetes-test (v1.23.13) | - 🔗 | N/A | See Details |
*You can configure Foresight comments in your organization settings page.
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
Unsure why CI is failing. Logs are saying it's failing on a Jaeger test 🤔 |
It’s failing on checks too, please take a look |
@atoulme I fixed the issue with Might have just been a random issue |
It's a flaky test: #9113 |
Ok, hopefully a re-run fixes it |
Please add the new module to versions.yaml with |
Cheers, fixed. |
Can someone with write permissions merge this PR please? |
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.
Thanks for this @lewis262626!
I left a few questions / suggestions. It seems like the receiver does not implement any CloudWatch API polling yet? Is this meant to provide just bare bones for the receiver?
(Sorry if that's just my unfamiliarity with process, not sure what is the required minimum viable functionality for a receiver to be merged. I was expecting an initial version to already be capable of receiving metrics).
I was asked to split up my original PR, hence this bare bones receiver. You're more than welcome to check out and build my original |
@lewis262626 thanks for the explanation, let's get this in! ❤️ |
Please rebase, thanks! |
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.
Please resolve/address the remaining comments and we can get this merged!
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.
dependabot conflict solved, but apparently, there are a few unhandled comments. I'll go over this once the comments are taken care of.
I have some free time later this week. Will resolve the outstanding comments and rebase |
281e034
to
b39cb85
Compare
Have rebased changes from |
I can add the polling logic once this PR has been merged |
@jpkrohling please give final review as the sponsor. |
As per GH comment, include the number of metrics you can define in the error message. Lowercase debug messages for start and shutdown receiver method
Co-authored-by: Antoine Toulme <[email protected]>
Co-authored-by: Juraci Paixão Kröhling <[email protected]>
Co-authored-by: Juraci Paixão Kröhling <[email protected]>
934968d
to
afc2113
Compare
Should all be fixed. Appreciate a final review @jpkrohling |
@lewis262626 please take a look at the failing linter |
Fixed the linter issue |
Description: New AWS CloudWatch metrics receiver initial PR
Link to tracking Issue: #15667
Testing: Unit tests for component config and factory
PR supersedes: #19218