Skip to content

Conversation

@Gekko0114
Copy link

What does this PR do?

This PR enhances the get_api_key() function to support both plain text and JSON format values from AWS Secrets Manager. Previously, the function only supported plain text values. Now it can handle:

  • Plain text API keys stored directly in Secrets Manager
  • JSON format secrets with keys like DD_API_KEY or DATADOG_API_KEY

The function first treats the secret value as plain text. If the value looks like JSON (starts with { and ends with }), it attempts to parse it and extract the API key from common key names. If JSON parsing fails or no common key is found, it falls back to treating the value as plain text.

Motivation

Some users store their Datadog API keys in Secrets Manager as JSON objects (key-value pairs) rather than plain text. This change makes the library more flexible and compatible with different secret storage formats, improving the developer experience.

Testing Guidelines

Added comprehensive test cases covering:

  • Plain text secrets (existing behavior)
  • JSON format secrets with api_key key
  • JSON format secrets with DD_API_KEY key
  • JSON format secrets with custom keys (fallback to plain text)

All tests pass and maintain backward compatibility with existing plain text secrets.

Additional Notes

  • The implementation prioritizes plain text handling first, ensuring backward compatibility
  • JSON parsing only occurs when the value appears to be JSON (starts/ends with braces)
  • Common key names checked: DD_API_KEY, DATADOG_API_KEY
  • If JSON parsing fails or no common key is found, the original value is used as-is

Types of Changes

  • New feature

Check all that apply

  • This PR's description is comprehensive
  • This PR's changes are covered by the automated tests

- Modify get_api_key() to first treat Secrets Manager value as plain text
- If the value looks like JSON (starts with '{' and ends with '}'), parse it and look for common keys (DD_API_KEY, DATADOG_API_KEY)
- If JSON parsing fails or no common key is found, fall back to plain text
- Add test cases for both plain text and JSON formats
@Gekko0114 Gekko0114 requested review from a team as code owners December 9, 2025 14:43
@astuyve
Copy link
Contributor

astuyve commented Dec 9, 2025

Thanks for the PR, this is great! However this code path is only taken when the user is not using the datadog lambda extension (which is the default for the last ~3 years), and only when the user is sending custom metrics with the send_distribution_metric method.

Wanna write some rust? I think we should add this to the extension and release it at the same time as this change.

@Gekko0114
Copy link
Author

@astuyve

Thanks!
I installed datadog-lambda via a container image by following the documentation below. Regarding the Secrets Manager, I have it registered as a Key-Value store rather than plain text. I encountered this error when I tried using lambda_metric.

https://docs.datadoghq.com/ja/serverless/aws_lambda/instrumentation/python/?tab=containerimage

This documentation appears to be up-to-date, but is this method no longer recommended?
Could you please let me know the best practice for monitoring Python running as a container image on AWS Lambda?

@astuyve
Copy link
Contributor

astuyve commented Dec 10, 2025

@astuyve

Thanks! I installed datadog-lambda via a container image by following the documentation below. Regarding the Secrets Manager, I have it registered as a Key-Value store rather than plain text. I encountered this error when I tried using lambda_metric.

https://docs.datadoghq.com/ja/serverless/aws_lambda/instrumentation/python/?tab=containerimage

This documentation appears to be up-to-date, but is this method no longer recommended? Could you please let me know the best practice for monitoring Python running as a container image on AWS Lambda?

The intention is that this library, when configured with Datadog Lambda Extension, will use the extension to send metrics asynchronously to the backend.

I think there are two possible explanations for the issue you encountered. The first is that the extension also failed to decrypt the key (because it lacks this implementation), so it entered a no-op loop and then this library could not write to it and fell back to talking directly to the API.

The second is that this library is not correctly detecting the extension and is pushing data directly to the backend without using the extension at all (which is not ideal).

I'm going to recommend that we close this for now as the proper place for this change is in https://github.com/datadog/datadog-lambda-extension/ where the API key is ultimately designed to be used (this library should be API-key free in nearly all cases). The second issue is that this would require changes across all of our runtime libraries and careful coordination with our documentation to ensure that this change is consistent everywhere.

Thank you very much for your contribution though!

@Gekko0114
Copy link
Author

Gekko0114 commented Dec 11, 2025

@astuyve

Thanks for your explanation! I understand.

However, on the other hand, since the function is named get_api_key,
it seems weird to return a JSON object instead of the actual API key string.
I believe it should be the responsibility of this function to parse and retrieve the API key itself.

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.

2 participants