Skip to content

fix ESM analytics #11961

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
Nov 29, 2024
Merged

fix ESM analytics #11961

merged 1 commit into from
Nov 29, 2024

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Nov 29, 2024 •
edited
Loading

Motivation

It seems #11883 has broken downstream dependencies, related to Kafka not having the same format (sending a dict when the signature is list[dict]). Tests are failing and are not validated against AWS, so I suppose we're supposed to be able to accept dict directly to process_events_batch as input_events.

Changes

  • add more safety around trying to access first_event

@bentsku bentsku added semver: patch Non-breaking changes which can be included in patch releases aws:lambda:event-source-mapping AWS Lambda Event Source Mapping (ESM) labels Nov 29, 2024
@bentsku bentsku self-assigned this Nov 29, 2024
@bentsku bentsku mentioned this pull request Nov 29, 2024
Merged
2 tasks
@github-actions GitHub Actions
Copy link

LocalStack Community integration with Pro

    2 files  ±  0      2 suites  ±0   1h 28m 51s ⏱️ - 22m 8s
2 829 tests  - 946  2 580 ✅  - 849  249 💤  - 97  0 ❌ ±0 
2 831 runs   - 946  2 580 ✅  - 849  251 💤  - 97  0 ❌ ±0 

Results for commit 22ae4f0. ± Comparison against base commit 892eb4d.

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

thrau
thrau approved these changes Nov 29, 2024
View reviewed changes
Copy link
Member

@thrau thrau left a comment

Choose a reason for hiding this comment

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

LGTM!

@thrau thrau merged commit 5a2a3f8 into master Nov 29, 2024
45 checks passed
@thrau thrau deleted the fix-esm-analytics branch November 29, 2024 07:29
@joe4dev
Copy link
Member

joe4dev commented Nov 29, 2024

Thank you very much @bentsku for this fix 💯

Weirdly, Kafka in Lambda ESM does indeed use a different event structure not properly reflected in the API.

Kafka Lambda ESM example:

{
   "eventSource": "SelfManagedKafka",
   "bootstrapServers":"b-2.demo-cluster-1.a1bcde.c1.kafka.us-east-1.amazonaws.com:9092,b-1.demo-cluster-1.a1bcde.c1.kafka.us-east-1.amazonaws.com:9092",
   "records":{
      "mytopic-0":[
         {
            "topic":"mytopic",
            "partition":0,
            "offset":15,
            "timestamp":1545084650987,
            "timestampType":"CREATE_TIME",
            "key":"abcDEFghiJKLmnoPQRstuVWXyz1234==",
            "value":"SGVsbG8sIHRoaXMgaXMgYSB0ZXN0Lg==",
            "headers":[
               {
                  "headerKey":[
                     104,
                     101,
                     97,
                     100,
                     101,
                     114,
                     86,
                     97,
                     108,
                     117,
                     101
                  ]
               }
            ]
         }
      ]
   }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thrau thrau thrau approved these changes

@joe4dev joe4dev Awaiting requested review from joe4dev

@dominikschubert dominikschubert Awaiting requested review from dominikschubert

@dfangl dfangl Awaiting requested review from dfangl

@gregfurman gregfurman Awaiting requested review from gregfurman

Assignees

@bentsku bentsku

Labels
aws:lambda:event-source-mapping AWS Lambda Event Source Mapping (ESM) 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.
3 participants
@bentsku @joe4dev @thrau