8000 logs: fix `put_metric_data` client for multi-account/region by sannya-singal · Pull Request #11691 · localstack/localstack · GitHub
[go: up one dir, main page]
Skip to content

logs: fix put_metric_data client for multi-account/region #11691

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
Oct 16, 2024
Merged

logs: fix put_metric_data client for multi-account/region #11691

merged 1 commit into from
Oct 16, 2024

Conversation

sannya-singal
Copy link
Contributor
@sannya-singal sannya-singal commented Oct 15, 2024 •
edited
Loading

Motivation

As an initiative to make the -ext multi-account/multi-region compatible this PR fixes functionality for logs service.
So whenever we use values other than 000000000000 for account ID or us-east-1 for region, it should still create the consequent resources in these accounts and region.

Failing workflow: https://github.com/localstack/localstack-ext/actions/runs/11268466307/job/31335436967

Changes

Add the region and account id from context to the connect_to client in put_log_events used to put the metrics data.
This fixes: tests.aws.services.logs.test_logs.TestCloudWatchLogsPro.test_json_metric_filters in -ext for non-default credentials.

Testing

Set the environment variables
TEST_AWS_ACCOUNT_ID=111111111111, TEST_AWS_ACCESS_KEY_ID=111111111111, TEST_AWS_REGION=us-west-1

@sannya-singal sannya-singal added the semver: patch Non-breaking changes which can be included in patch releases label Oct 15, 2024
@sannya-singal sannya-singal added this to the 4.0 milestone Oct 15, 2024
@sannya-singal sannya-singal self-assigned this Oct 15, 2024
@sannya-singal sannya-singal changed the title logs: fix put_metric_data client for multi account/region logs: fix put_metric_data client for multi-account/region Oct 15, 2024
@github-actions GitHub Actions
Copy link

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   1h 0m 4s ⏱️ - 44m 17s
1 491 tests  - 2 004  1 316 ✅  - 1 766  175 💤  - 238  0 ❌ ±0 
1 493 runs   - 2 004  1 316 ✅  - 1 766  177 💤  - 238  0 ❌ ±0 

Results for commit 7984be4. ± Comparison against base commit 5e8d3a1.

This pull request removes 2004 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]
…

@sannya-singal
Copy link
Contributor Author

Re-ran the Ext Integration Tests pipeline for non-default creds to validate the changes in multi-acc-logs branch: https://github.com/localstack/localstack-ext/actions/runs/11344381436

@sannya-singal sannya-singal marked this pull request as ready for review October 15, 2024 11:56
pinzon
pinzon approved these changes Oct 15, 2024
View reviewed changes
Copy link
Member
@pinzon pinzon left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@sannya-singal sannya-singal merged commit 6679ec9 into master Oct 16, 2024
43 checks passed
@sannya-singal sannya-singal deleted the multi-acc-logs branch October 16, 2024 05:55
macnev2013 pushed a commit that referenced this pull request Oct 22, 2024
@sannya-singal @macnev2013
logs: fix put_metric_data client for multi-account/region (#11691)
d14dc3f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pinzon pinzon pinzon approved these changes

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

Assignees

@sannya-singal sannya-singal

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

Successfully merging this pull request may close these issues.
2 participants
@sannya-singal @pinzon
0