Skip to content

Conversation

estolfo
Copy link
Contributor

@estolfo estolfo commented Feb 22, 2022

This is a refactor of the logsapi package as per the review in #124

Changes include:

  • Expose public Subscribe function that subscribes to the logsapi and starts an HTTP server listening for log events
  • Keep client.go but move the creation of the logsapi subscription structs in the client's Subscribe function and change the function signature
  • Remove logsapilistener.go and move the functionality to the Subscribe function in subscribe.go
  • Add subscribe_test.go
  • Remove env variable ELASTIC_APM_LAMBDA_LOGS_LISTENER_ADDRESS

Todo (3 March)

@github-actions github-actions bot added the aws-λ-extension AWS Lambda Extension label Feb 22, 2022
@ghost
Copy link

ghost commented Feb 22, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-03-09T12:28:58.911+0000

  • Duration: 6 min 4 sec

Test stats 🧪

Test Results
Failed 0
Passed 120
Skipped 6
Total 126

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@estolfo estolfo marked this pull request as ready for review February 23, 2022 15:14
@estolfo estolfo force-pushed the estolfo/refactor-logsapi branch from 83315df to a5776fa Compare February 23, 2022 15:19
Copy link
Contributor

@jlvoiseux jlvoiseux left a comment

Choose a reason for hiding this comment

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

Seems great to me ! Aside from reviewing the changes, I used E2E testing and ran this PR with my development Lambda functions and did not face any issue. The collection of platform metrics relies on the logsapi package, but the only major change I see on my end is to move the definition of the platform report model from http_listener.go to subscribe.go.

Copy link
Contributor

@stuartnelson3 stuartnelson3 left a comment

Choose a reason for hiding this comment

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

looking good! left some comments / questions / suggestions

@estolfo estolfo force-pushed the estolfo/refactor-logsapi branch from dbeef11 to 4cc9b37 Compare March 2, 2022 10:35
Copy link
Contributor

@stuartnelson3 stuartnelson3 left a comment

Choose a reason for hiding this comment

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

looks good to me!

@estolfo estolfo force-pushed the estolfo/refactor-logsapi branch from 034b26c to 6ec160d Compare March 9, 2022 11:28
@estolfo estolfo merged commit 4f18179 into main Mar 9, 2022
@estolfo estolfo deleted the estolfo/refactor-logsapi branch March 9, 2022 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aws-λ-extension AWS Lambda Extension

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants