fix: forward logs directly when invocation is over

[kruskall]

The lambda extension process data in the background during an invocation:

apm-aws-lambda/app/run.go

Lines 197 to 204 in 3f5620c

	// APM Data Processing
	backgroundDataSendWg.Add(1)
	go func() {
	defer backgroundDataSendWg.Done()
	if err := app.apmClient.ForwardApmData(invocationCtx); err != nil {
	app.logger.Error(err)
	}
	}()

Data is sent to a channel. This is fine during an invocation when the goroutine is running but it's possible that during shutdown the channel will block because there's no goroutine running.

apm-aws-lambda/app/run.go

Line 101 in 3f5620c

app.logsClient.FlushData(ctx, event.RequestID, event.InvokedFunctionArn, app.apmClient.ForwardLambdaData, true)

The solution is to bypass the channel and forward the data directly during shutdown.

[rockdaboot]

Can you describe the issue in the PR description and how this PR solves it?

[kruskall]

Updated! 👍

[dmathieu]

Could we add tests, or is that too unpredictable?

[rockdaboot]

Updated! 👍

Thanks! Maybe I misunderstand the code changes, but it looks to me that you replace direct writes to the channel with a function that writes to the data channel. How is that a difference in the data flow logic and don't we still have the same issue then (channel reader doesn't read any more)?

I somewhat agree with @dmathieu that we should try to create a test that reproduces the issue, and then prove that this code solves the issue. It doesn't have to be a unit test, if that is too involved. Maybe some kind of tool / setup / script, whatever is the simplest way.

[sboomsma]

just nudging here in case it gets forgotten because the related SDH was auto-closed

[kruskall]

Thanks! Maybe I misunderstand the code changes, but it looks to me that you replace direct writes to the channel with a function that writes to the data channel. How is that a difference in the data flow logic and don't we still have the same issue then (channel reader doesn't read any more)?

There are three places that have been updated. The first two are directly forwarding the data bypassing the channel, the last one is unchanged and just send to a channel.
For the third case: It's fine to send to a channel inside processEvent because there's someone reading from it and in the worst case the ctx will make the func exit.

I somewhat agree with @dmathieu that we should try to create a test that reproduces the issue, and then prove that this code solves the issue. It doesn't have to be a unit test, if that is too involved. Maybe some kind of tool / setup / script, whatever is the simplest way.

maybe we can reuse the setup from @xrmx ? I'm not sure how feasible it is to write a unit test for this.

[rockdaboot]

For the third case: It's fine to send to a channel inside processEvent because there's someone reading from it and in the worst case the ctx will make the func exit.

Thanks for the explanation.

maybe we can reuse the setup from @xrmx ? I'm not sure how feasible it is to write a unit test for this.

@xrmx Can you run your reproducer again with the changes here and confirm that it fixes the issue?