fix: stop reading private aws-cdk-lib Function.environment field - #621
Conversation
This comment has been minimized.
This comment has been minimized.
31ab410 to
5b06be5
Compare
janine-c
left a comment
There was a problem hiding this comment.
Just some optional writing suggestions 🙂
5b06be5 to
062d08f
Compare
|
Can we please get this merged? |
We'll try to prioritize it, my main concern is that this version of the fix has a behavior change which may break some usage patterns. I have another potential option up as a separate PR, and I'll weight the options with the team before merging either. Sorry for the delay, and thanks for your patience on this! I just want to make sure we're being careful before potentially releasing a breaking change. |
|
Appreciate the caution, and I get that the behaviour change needs weighing. But ISTM this is being triaged as a minor cleanup. From the consumer side it is major. The un-patched library reads a private aws-cdk-lib field, so every consumer of sourceCodeIntegration breaks whenever CDK reshapes that internal. #596 was triggered by a semver-minor (2.252 -> 2.253), and the v4.0.0 fix kept the private coupling, so the next break is a when, not an if. We're an engineering org with compliance and audit obligations: dependency upkeep is mandatory, automated, and continuous. A library that can break This PR has been open since June 23, and I want to be direct about the consequences. Next week I'll remove our usage of this library and with it Datadog observability support for lambdas. Future observability work will land in our in-house observability platform. I'd rather not deal with this right now, but Datadog's failure to address our ongoing exposure from this issue in a timely way is forcing my hand. |
|
@ahammond I've chatted with the team, and we'll go through with the current implementation, which includes a breaking change around setting environment variables. I'll work on getting this merged and released in the next couple days so you can migrate to the safer version. |
f15ce72 to
82cb41d
Compare
|
@ahammond we're working on some additional changes before we release the next version (with a major version bump). it should be out sometime next week. |

What does this PR do?
Removes all reads of
aws-cdk-lib's privateFunction.environmentfield from the library. Every read is replaced with aWeakMap-backed tracker that records the library's ownaddEnvironmentcalls. Adds a publicDatadogLambda.setEnvironment()method so users can seed env vars the construct will respect.Motivation
Closes #620
aws-cdk-lib changed
Function.environment's internal shape in a semver-minor (2.252 → 2.253), breakingcdk synthfor all users ofsourceCodeIntegration(#596). The v4.0.0 fix re-pointed the access at the new internal shape and raised the peer floor -- it kept the private coupling, so the library remained one CDK refactor away from breaking again.Three call sites read across the private boundary:
env.ts--setGitEnvironmentVariablesreadsDD_TAGSto append git metadataenv.ts--applyEnvVariableschecks each key before writing a defaultdatadog-lambda.ts--overrideGitMetadatareadsDD_TAGSto rewrite git componentsChanges
src/env-tracker.ts(new)WeakMap<LambdaFunction, Map<string, string>>(ddEnvTracker) and three helpers --setTrackedEnv,getTrackedEnv,hasTrackedEnv-- that record every env write the library makes. Internal, not re-exported fromindex.ts.src/env.tsaddEnvironmentcalls insetGitEnvironmentVariables,applyEnvVariables, andsetDDEnvVariablesgo throughsetTrackedEnv.setGitEnvironmentVariablesparameter type tightened fromany[]toLambdaFunction[].src/datadog-lambda.tsoverrideGitMetadatareadsDD_TAGSfromgetTrackedEnvinstead oflambdaFunction.environment.map.get(DD_TAGS), removing theanycast. Tag rewriting extracted to anupsertTaghelper.setEnvironment(lambdaFunction, key, value), which writes throughsetTrackedEnvso the construct treats the value as one it manages.README.mdDD_*vars viaDatadogLambdaProps; callfunc.addEnvironment()afteraddLambdaFunctions()to override; or callsetEnvironment()before to seed a value the construct will respect.Behavior change
Previously the library read the private field to detect whether the user had already set a
DD_*var on a function beforeaddLambdaFunctions(), skipping its default if so -- and to append git metadata onto a user-setDD_TAGS. Without a public CDK read API (which does not exist), arbitrary pre-set env vars are no longer visible to the construct. The new contract:DatadogLambdaPropsfields (enableDatadogTracing,logLevel,tags, etc.)datadogLambda.setEnvironment(func, key, value)beforeaddLambdaFunctions(). The construct will not override it, and when source code integration is enabled it appends git metadata (git.commit.sha,git.repository_url) onto a seededDD_TAGSrather than overriding it. This preserves the prior per-functionDD_TAGS+ git-metadata workflow.func.addEnvironment("DD_*", value)afteraddLambdaFunctions()-- CDK's last-write-wins semantics handle this naturally.addLambdaFunctions()(not viasetEnvironment): will be overridden by the library's defaults.Testing Guidelines
The existing suite covers the tracker migration; two tests were updated to reflect the behavior change (comments explain the new contract), and the
overrideGitMetadatatests now assert viaTemplate.fromStack()instead of reading the CDK private field. New tests coversetEnvironment: git metadata appending onto a seededDD_TAGS, and the construct not overriding a seeded value.Run locally with
yarn test.Types of Changes
Check all that apply