Skip to content

Commit

Permalink
fix: only flush logs if logs collection is enabled (#510)
Browse files Browse the repository at this point in the history
* fix: only flush logs if logs collection is enabled

* test: add standard events chain test with logs collection disabled

There is no coverage/test for the app when logs collection is disabled
Add a testcase to ensure standard events chain work even if logs
collection is disabled.
  • Loading branch information
kruskall authored Jul 25, 2024
1 parent f2f7de6 commit b307ea4
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 6 deletions.
12 changes: 8 additions & 4 deletions app/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,10 @@ func (app *App) Run(ctx context.Context) error {
backgroundDataSendWg.Wait()
if event.EventType == extension.Shutdown {
app.logger.Infof("Exiting due to shutdown event with reason %s", event.ShutdownReason)
// Flush buffered logs if any
app.logsClient.FlushData(ctx, event.RequestID, event.InvokedFunctionArn, app.apmClient.LambdaDataChannel, true)
if app.logsClient != nil {
// Flush buffered logs if any
app.logsClient.FlushData(ctx, event.RequestID, event.InvokedFunctionArn, app.apmClient.LambdaDataChannel, true)
}
// Since we have waited for the processEvent loop to finish we
// already have received all the data we can from the agent. So, we
// flush all the data to make sure that shutdown can correctly deduce
Expand Down Expand Up @@ -127,8 +129,10 @@ func (app *App) Run(ctx context.Context) error {
// that the underlying transport is reset for next invocation without
// waiting for grace period if it got to unhealthy state.
flushCtx, cancel := context.WithCancel(ctx)
// Flush buffered logs if any
app.logsClient.FlushData(ctx, event.RequestID, event.InvokedFunctionArn, app.apmClient.LambdaDataChannel, false)
if app.logsClient != nil {
// Flush buffered logs if any
app.logsClient.FlushData(ctx, event.RequestID, event.InvokedFunctionArn, app.apmClient.LambdaDataChannel, false)
}
// Flush APM data now that the function invocation has completed
app.apmClient.FlushAPMData(flushCtx)
cancel()
Expand Down
35 changes: 33 additions & 2 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,29 @@ func TestStandardEventsChain(t *testing.T) {
}
}

// TestStandardEventsChainWithoutLogs checks a nominal sequence of events (fast APM server, only one standard event)
// with logs collection disabled
func TestStandardEventsChainWithoutLogs(t *testing.T) {
l, err := logger.New(logger.WithLevel(zapcore.DebugLevel))
require.NoError(t, err)

eventsChannel := newTestStructs(t)
apmServerInternals, _ := newMockApmServer(t, l)
logsapiAddr := randomAddr()
newMockLambdaServer(t, logsapiAddr, eventsChannel, l)

eventsChain := []MockEvent{
{Type: InvokeStandard, APMServerBehavior: TimelyResponse, ExecutionDuration: 1, Timeout: 5},
}
eventQueueGenerator(eventsChain, eventsChannel)
select {
case <-runAppFull(t, logsapiAddr, true):
assert.Contains(t, apmServerInternals.Data, TimelyResponse)
case <-time.After(timeout):
t.Fatalf("timed out waiting for app to finish")
}
}

// TestFlush checks if the flushed param does not cause a panic or an unexpected behavior
func TestFlush(t *testing.T) {
l, err := logger.New(logger.WithLevel(zapcore.DebugLevel))
Expand Down Expand Up @@ -821,13 +844,21 @@ func TestMetrics(t *testing.T) {
}

func runApp(t *testing.T, logsapiAddr string) <-chan struct{} {
return runAppFull(t, logsapiAddr, false)
}

func runAppFull(t *testing.T, logsapiAddr string, disableLogsAPI bool) <-chan struct{} {
ctx, cancel := context.WithCancel(context.Background())
app, err := app.New(ctx,
opts := []app.ConfigOption{
app.WithExtensionName("apm-lambda-extension"),
app.WithLambdaRuntimeAPI(os.Getenv("AWS_LAMBDA_RUNTIME_API")),
app.WithLogLevel("debug"),
app.WithLogsapiAddress(logsapiAddr),
)
}
if disableLogsAPI {
opts = append(opts, app.WithoutLogsAPI())
}
app, err := app.New(ctx, opts...)
require.NoError(t, err)

go func() {
Expand Down

0 comments on commit b307ea4

Please sign in to comment.