NEW_RELIC_ENABLED env var not respected

[rittneje]

Description

If we set NEW_RELIC_ENABLED=0, the Go agent won't report anything. However, the extension does not respect this env var. Consequently, it waits for telemetry that will never come, and ends up timing out 200 ms before actual timeout. (From AWS's perspective, the function was running the whole time and we get billed for it.)

newrelic-lambda-extension/main.go

Lines 269 to 300 in a8461c8

	// Set the timeout timer for a smidge before the actual timeout; we can recover from false timeouts.
	timeoutWatchBegins := 200 * time.Millisecond
	timeLimitContext, timeLimitCancel := context.WithDeadline(ctx, timeoutInstant.Add(-timeoutWatchBegins))
	// Before we begin to await telemetry, harvest and ship. Ripe telemetry will mostly be handled here. Even that is a
	// minority of invocations. Putting this here lets us run the HTTP request to send to NR in parallel with the Lambda
	// handler, reducing or eliminating our latency impact.
	pollLogServer(logServer, batch)
	shipHarvest(ctx, batch.Harvest(time.Now()), telemetryClient)
	select {
	case <-timeLimitContext.Done():
	timeLimitCancel()
	// We are about to timeout
	probablyTimeout = true
	continue
	case telemetryBytes := <-telemetryChan:
	timeLimitCancel()
	// We received telemetry
	util.Debugf("Agent telemetry bytes: %s", base64.URLEncoding.EncodeToString(telemetryBytes))
	inv := batch.AddTelemetry(lastRequestId, telemetryBytes)
	if inv == nil {
	util.Logf("Failed to add telemetry for request %v", lastRequestId)
	}
	// Opportunity for an aggressive harvest, in which case, we definitely want to wait for the HTTP POST
	// to complete. Mostly, nothing really happens here.
	pollLogServer(logServer, batch)
	shipHarvest(ctx, batch.Harvest(time.Now()), telemetryClient)
	}

Steps to Reproduce

See above.

Expected Behavior

Extension should be a no-op if NEW_RELIC_ENABLED=0.

Relevant Logs / Console output

Your Environment

Additional context

[workato-integration]

https://new-relic.atlassian.net/browse/NR-277124

[chaudharysaket]

Hi @rittneje
We haveNEW_RELIC_LAMBDA_EXTENSION_ENABLED lambda environment variable to enable/disable extension, as defined here

Currently, we see that different language agent is using a different env var to enable instrumentation, as mentioned here.
env var for controlling whether the agent is enabled differs across agents:
Go: NEW_RELIC_ENABLED
Ruby: NEW_RELIC_AGENT_ENABLED
Node.js: NEW_RELIC_ENABLED

Since lambda extension supports all language runtimes, it should respect all agent specific env vars. We will look into the issue such that lambda extension should respect the agent enable environment variable.
Thank you!

[workato-integration]

Work has been completed on this issue.

[rittneje]

@chaudharysaket Where was this fixed? I don't see any relevant commits, and there is no mention of NEW_RELIC_ENABLED in the source code.

[chaudharysaket]

Hi @rittneje
We can achieve similar results using the Extension specific environment variable NEW_RELIC_LAMBDA_EXTENSION_ENABLED. Since there are variations of NEW_RELIC_ENABLED env variable among different New Relic language agents, we thought of sticking to NEW_RELIC_LAMBDA_EXTENSION_ENABLED for Extension. Introducing new environment variables for different language agents would serve same purpose.

Could you please help us understand cases where NEW_RELIC_LAMBDA_EXTENSION_ENABLED might not be a replacement for NEW_RELIC_ENABLED?
Thank you

[rittneje]

@chaudharysaket As clearly mentioned in the issue description, the bug is that NEW_RELIC_ENABLED=0 causes the agent not to log anything, which in turn causes the extension to block until the lambda timeout. It is required for the extension to abide by this environment variable in order to be compatible.

[chaudharysaket]

Okay. We will look into the issue shortly such that extension will abide by the NEW_RELIC_ENABLED env variable. Thank you!

[rittneje]

@chaudharysaket The other thing I will share is that NEW_RELIC_LAMBDA_EXTENSION_ENABLED=0 on its own is still useful, since that allows us to force it to revert to the old "log group subscription" approach. So NEW_RELIC_ENABLED=0 shouldn't be considered a replacement for it, it just takes priority.

[workato-integration]

Work has been completed on this issue.

[rittneje]

@chaudharysaket This issue needs to be reopened. You only considered “false” not “0”.