8000 CloudWatch: Add missing response fields by viren-nadkarni · Pull Request #11411 · localstack/localstack · GitHub
[go: up one dir, main page]
Skip to content

CloudWatch: Add missing response fields #11411

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 26, 2024
Merged

CloudWatch: Add missing response fields #11411

merged 1 commit into from
Aug 26, 2024

Conversation

viren-nadkarni
Copy link
Member

Background

CloudWatch has the /_aws/cloudwatch/metrics/raw endpoint that returns the raw metrics. It was noticed that the v1 and v2 providers do not have the same API response spec. To be specific, the account and region fields were missing in the v1 provider.

Changes

In order to keep the spec consistent and simplify #11025, this PR adds the missing fields to the v1 provider response.

@viren-nadkarni viren-nadkarni self-assigned this Aug 26, 2024
@viren-nadkarni viren-nadkarni mentioned this pull request Aug 26, 2024
Merged
@viren-nadkarni viren-nadkarni added the semver: patch Non-breaking changes which can be included in patch releases label Aug 26, 2024
steffyP
steffyP approved these changes Aug 26, 2024
View reviewed changes
Copy link
Member
@steffyP steffyP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it makes things easier, I am ok with the change 😄 Thanks for unifying 🙌

For some additional context:
As already outlined in #11025 (comment): I think this endpoint especially for v1 is not really usable.
For v2 I already added a comment to the code, that we should either refine how the endpoint could be used (include aggregated statistics, using filters, pagination etc), or deprecate it and remove it all together at some point.

The endpoint was created years ago in response to a customer request, but it has never been documented and in the state that is currently is, I also wouldn't recommend to do so. However, when trying to pin down certain issues, it can be helpful for debugging, so it can definitely be useful in certain cases.

Anyhow, this is out of the scope for what you are trying to accomplish 😄

@github-actions GitHub Actions
Copy link

LocalStack Community integration with Pro

  2 files  ±    0    2 suites  ±0   31m 28s ⏱️ - 1h 4m 21s
840 tests  - 2 499  788 ✅  - 2 157  52 💤  - 342  0 ❌ ±0 
842 runs   - 2 499  788 ✅  - 2 157  54 💤  - 342  0 ❌ ±0 

Results for commit 91ae008. ± Comparison against base commit 0063912.

This pull request removes 2499 tests. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

@viren-nadkarni viren-nadkarni merged commit 0f99af7 into master Aug 26, 2024
36 of 37 checks passed
@viren-nadkarni viren-nadkarni deleted the cloudwatch-api-response branch August 26, 2024 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steffyP steffyP steffyP approved these changes

@pinzon pinzon Awaiting requested review from pinzon pinzon is a code owner

Assignees

@viren-nadkarni viren-nadkarni

Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@viren-nadkarni @steffyP
0