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

Add the ability to exclude functions from being instrumented #135

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

TophrC-dd
Copy link
Contributor

What does this PR do?

This pr introduces the parameter exclude that allows functions to be ignored by the transform command.

Motivation

Customers might encounter a situation where they may not want to instrument a function. This could be for many reason but one is for functions that require low latency

Testing Guidelines

I wrote a test case to ensure the that the function is not modified with layers and ensuring that the handler is not changed.

I also tested with a test deployment that had multiple functions with one function excluded. It instrumented the intended functions while leaving the desired excluded function untouched.

Additional Notes

Types of changes

  • Bug fix
  • New feature
  • Breaking change
  • Misc (docs, refactoring, dependency upgrade, etc.)

Check all that apply

  • This PR's description is comprehensive
  • This PR contains breaking changes that are documented in the description
  • This PR introduces new APIs or parameters that are documented and unlikely to change in the foreseeable future
  • This PR impacts documentation, and it has been updated (or a ticket has been logged)
  • This PR's changes are covered by the automated tests
  • This PR collects user input/sensitive content into Datadog

@TophrC-dd TophrC-dd requested review from a team as code owners September 19, 2024 20:11
Copy link

@brett0000FF brett0000FF left a comment

Choose a reason for hiding this comment

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

Looks good! Just a small suggestion for you to consider.

@@ -121,7 +121,9 @@ To further configure your plugin, use the following custom parameters:
| `nodeLayerVersion` | Version of the Node.js Lambda layer to install, such as "29". Required if you are deploying at least one Lambda function written in Node.js and `addLayers` is true. Find the latest version number from [https://github.com/DataDog/datadog-lambda-js/releases][6]. |
| `dotnetLayerVersion` | Version of the .NET Lambda layer to install, such as "14". Required if you are deploying at least one Lambda function written in .NET and `addLayers` is true. Find the latest version number from [https://github.com/DataDog/dd-trace-dotnet-aws-lambda-layer/releases][9].
| `javaLayerVersion` | Version of the Java Lambda layer to install, such as "12". Required if you are deploying at least one Lambda function written in Java and `addLayers` is true. Find the latest version number from [https://github.com/DataDog/datadog-lambda-java/releases][10].
| `extensionLayerVersion` | Version of the Datadog Lambda Extension layer to install, such as "5". When `extensionLayerVersion` is set, `apiKey` (or if encrypted, `apiKMSKey` or `apiKeySecretArn`) needs to be set as well. While using `extensionLayerVersion` do not set `forwarderArn`. Learn more about the Lambda extension [here][8]. |
| `extensionLayerVersion` | Version of the Datadog Lambda Extension layer to install, such as "5". When `extensionLayerVersion` is set, `apiKey` (or if encrypted, `apiKMSKey` or `apiKeySecretArn`) needs to be set as well. |
| `addExtension` | Whether to add the Lambda extension layer to the Lambda function. Defaults to true. When true, the extension layer is applied which handles sending logs, traces and custom metrics. The Parameter `extensionLayerVersion` must be set. While using `extensionLayerVersion` do not set `forwarderArn`. Learn more about the Lambda extension [here][8]. |

Choose a reason for hiding this comment

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

Suggested change
| `addExtension` | Whether to add the Lambda extension layer to the Lambda function. Defaults to true. When true, the extension layer is applied which handles sending logs, traces and custom metrics. The Parameter `extensionLayerVersion` must be set. While using `extensionLayerVersion` do not set `forwarderArn`. Learn more about the Lambda extension [here][8]. |
| `addExtension` | Whether to add the Lambda extension layer to the Lambda function. Defaults to true. When true, the extension layer is applied which handles sending logs, traces, and custom metrics. The parameter `extensionLayerVersion` must be set. While using `extensionLayerVersion` do not set `forwarderArn`. To learn more, read the [Datadog Lambda Extension][8] documentation. |

Comment on lines 125 to 126
| `addExtension` | Whether to add the Lambda extension layer to the Lambda function. Defaults to true. When true, the extension layer is applied which handles sending logs, traces and custom metrics. The Parameter `extensionLayerVersion` must be set. While using `extensionLayerVersion` do not set `forwarderArn`. Learn more about the Lambda extension [here][8]. |
| `exclude` | When set, the macro will ignore lambda specified functions. Use this if there are any functions that do not require datadog instrumentation. Defaults to []. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Mmm where does addExtension come from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was added in a previous commit. At the time I thought documentation was in a separate place. 😬 I am addressing it in this PR.

Updating the readme based on suggestion.
Fixing a typo.
@@ -63,6 +63,9 @@ export interface Configuration {
// Optionally set by customer using `sam deploy --parameter-overrides DDGitData="$(git rev-parse HEAD),$(git config --get remote.origin.url)"`
// The customer template takes in the DDGitData override param and passes that to this macro's gitData param
gitData?: string;
// When set, the list of strings will be evalutated when processing each lambda function. if the string matches that function will not be instrumented by the macro.
exclude?: string[];
Copy link
Contributor

@duncanista duncanista Sep 19, 2024

Choose a reason for hiding this comment

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

Set the default to empty in the defaultConfiguration

serverless/README.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants