Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send logs by default from Lambda Extension #8101

Merged
merged 5 commits into from
May 10, 2021

Conversation

nhinsch
Copy link
Contributor

@nhinsch nhinsch commented May 6, 2021

What does this PR do?

Current behavior:

  • In the regular Datadog agent, the logs_enabled config key defaults to false. Setting it to true enables the logs agent.
  • In the Lambda extension, the logs agent is always enabled. However, by default we only use the logs to calculate metrics. If the user wants to send the logs to Datadog, they must set logs_enabled to true.

New behavior:

  • Create a new config key called serverless.logs_enabled which defaults to true. This is used by the Lambda extension to decide whether or not to send logs to Datadog.

Motivation

The vast majority of Lambda extension users will want to send logs to Datadog. We would like this to be the default behavior so that customers can avoid configuring the extension. Because the logs_enabled config key is used by the regular agent and defaults to false, the only way to change the default behavior without affecting the regular agent is to create a separate config key for the Lambda extension.

As a bonus, this removes confusion that may arise because the logs_enabled key previously had a different meaning in the Lambda extension compared to the regular agent.

Additional Notes

This is a breaking change for the Lambda extension:

  • Customers that previously were not sending logs to Datadog will now send logs to Datadog unless they explicitly disable it using the new serverless.logs_enabled config value (which corresponds to the DD_SERVERLESS_LOGS_ENABLED environment variable).
  • The value set by logs_enabled will now be ignored going forward.

I also looked into using IsSet() to override the normal default so that we could continue using the same logs_enabled environment variable. That is, if logs_enabled was not explicitly set to false, we could override the default to true in the Lambda Extension. This did not work because isSet() returns true even when the value is not explicitly set -- a default counts as being "set".

Describe how to test your changes

I manually tested this change in the sandbox environment.

@nhinsch nhinsch requested a review from a team as a code owner May 6, 2021 16:18
Copy link
Contributor

@DarcyRaynerDD DarcyRaynerDD left a comment

Choose a reason for hiding this comment

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

@nhinsch When you say in the PR notes, "This is a breaking change', might be worth clarifying it's only a breaking change to the extension, and makes so different to the regular agent.

pkg/config/config.go Outdated Show resolved Hide resolved
pkg/config/config.go Outdated Show resolved Hide resolved
@@ -828,6 +828,9 @@ func InitConfig(config Config) {
config.BindEnvAndSetDefault("runtime_security_config.remote_tagger", true)
bindEnvAndSetLogsConfigKeys(config, "runtime_security_config.endpoints.")

// Serverless Agent
config.BindEnvAndSetDefault("serverless_logs_enabled", true)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm very sorry I've made a typo in my previous comment, that would be: serverless.logs_enabled...

@nhinsch nhinsch changed the base branch from master to release/lambda-extension-v8 May 10, 2021 20:59
@nhinsch nhinsch merged commit 579044a into release/lambda-extension-v8 May 10, 2021
@nhinsch nhinsch deleted the ngh/default-logs branch May 10, 2021 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants