Skip to content

Conversation

estolfo
Copy link
Contributor

@estolfo estolfo commented Nov 19, 2021

This PR adds a change to signal on a channel when it receives an intake request from an agent containing the query param "flushed=true".
We can change the name of the query param, if that's not clear enough.

As a fallback, it also signals if it receives a runtimeDone event from the logsapi.

There are now three channels used to signal in main. The AgentDoneSignal signals that there was an intake request received with flushed=true, the runtimeDoneSignal channel signals that a runtimeDone event was received via the Logs API. The funcDone channel is closed if a signal is received on AgentDoneSignal or runtimeDoneSignal.
Closing that channel signals to the go routines posting apm data and processing logs API events that they can return.

Resolves #62

@github-actions github-actions bot added the aws-λ-extension AWS Lambda Extension label Nov 19, 2021
@ghost
Copy link

ghost commented Nov 19, 2021

💚 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: 2021-12-01T14:08:10.430+0000

  • Duration: 7 min 40 sec

  • Commit: 5b2295b

Test stats 🧪

Test Results
Failed 0
Passed 78
Skipped 0
Total 78

🤖 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 force-pushed the estolfo/agent-signal branch from 44c8394 to 77d6862 Compare November 25, 2021 13:14
@estolfo estolfo requested a review from astorm November 25, 2021 13:22
Copy link
Contributor

@astorm astorm left a comment

Choose a reason for hiding this comment

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

Overall this looks simple and solid. I see we have a new FuncDone property on the extension package that we're using as a signal only channel, and when this channel receives a message it indicates the function is done with execution.

We send a signal to this channel whenever we see a request to /intake/v2/events that includes a flushed=true query string parameter.

This should work. However, when I tested this against a real agent without the query param implemented the extension crashed and interfered with a Node.js Lambda function (causing it to return null)

However we coordinate with the agents to get this query param in we should (probably?) ensure that lambda functions with older agents attached don't crash. (I've added a discussion topic to the next planning meeting)

@estolfo
Copy link
Contributor Author

estolfo commented Nov 30, 2021

Thanks for the review, @astorm. I'll try to reproduce the error and report back.

@estolfo estolfo marked this pull request as draft November 30, 2021 09:39
@estolfo estolfo force-pushed the estolfo/agent-signal branch from 2116fbd to 3a0ce93 Compare November 30, 2021 21:40
@estolfo estolfo marked this pull request as ready for review November 30, 2021 21:41
@astorm astorm self-requested a review December 1, 2021 13:39
Copy link
Contributor

@astorm astorm left a comment

Choose a reason for hiding this comment

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

Approving 👍

I ran a manual smoke test and the fallback code is working -- data's sent, the extension does not crash.

The query param checking/signaling code looks like it does what it needs to.

@estolfo estolfo merged commit 2980ed0 into main Dec 1, 2021
@estolfo estolfo deleted the estolfo/agent-signal branch December 1, 2021 20:35
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.

Use explicit request from agent to signal completion of lambda function

2 participants