Repository navigation
fix(ecs): [SVLS-9885] convert comma-separated globalTags for Agent DD_TAGS - #758
Conversation
- Convert comma-separated globalTags to whitespace-separated DD_TAGS for the Agent - Keep comma-separated tags for the Fluent Bit log router - Add synth warnings for whitespace/token in globalTags and commas in DD_TAGS - Update docs, sample, and tests Rationale: The Agent reads DD_TAGS as whitespace-separated, so comma-separated globalTags were parsed incorrectly. Ticket: SVLS-9885 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> This commit made by [/dd:git:commit:quick](https://github.com/DataDog/claude-marketplace/tree/main/dd/commands/git/commit/quick.md)
TalUsvyatsky
changed the title
…bal-tags-comma Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Remove the token warning. A token inside globalTags converts correctly when its value is one tag, so the warning fired on safe code. The construct now warns only about literal values it can check at synth time. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
stzou
left a comment
stzou
left a comment
There was a problem hiding this comment.
LGTM, this fixes the reported case: the Agent gets whitespace-separated DD_TAGS, Fluent Bit keeps the comma form, and DD_TAGS only reaches the Agent container, so tracers aren't affected. Two non-blocking comments inline.
| * Converts comma-separated global tags to the whitespace-separated format that the Agent reads from DD_TAGS. | ||
| */ | ||
| export function toAgentTags(globalTags: string): string { | ||
| return globalTags.replace(/,/g, " "); |
There was a problem hiding this comment.
If globalTags comes from a token whose resolved value has commas (e.g. a CfnParameter or SSM parameter set to env:prod,team:pay), the replace sees no commas at synth, so the Agent still gets the comma form at deploy and nothing warns. Could toAgentTags use Fn.join(" ", Fn.split(",", globalTags)) when Token.isUnresolved(globalTags), so CloudFormation does the conversion? The current token test only covers a token inside literal tags.
| /** | ||
| * Converts comma-separated global tags to the whitespace-separated format that the Agent reads from DD_TAGS. | ||
| */ | ||
| export function toAgentTags(globalTags: string): string { |
There was a problem hiding this comment.
nit: the Agent also accepts DD_TAGS as a JSON array, so globalTags: '["env:prod","team:pay"]' used to give correct Agent tags. After this change it becomes ["env:prod" "team:pay"] and splits into broken tags. Probably rare, but could values starting with [ pass through unchanged, or get a warning?
There was a problem hiding this comment.
Good catch, I didn't realize this was also an accepted format. Updated to pass through unchanged in bdae3bc
…rays - Convert globalTags that contain a token with Fn::Split and Fn::Join, so CloudFormation converts commas in the resolved value for DD_TAGS - Pass a JSON array of tags through unchanged, because the Agent also accepts DD_TAGS as a JSON array - Skip the tag warnings for JSON arrays Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What does this PR do?
Fixes #742.
globalTagsis now a comma-separated list. The construct converts it to the format that each destination expects:DD_TAGS(Fargate and Managed Instances): commas become spaces, because the Agent separates tags with whitespace.dd_tags(Fargate): the log driver readsglobalTagsdirectly, notDD_TAGS. The output is the same as before.The construct also adds synth warnings (
Annotations.addWarningV2) for literal values it can check:globalTagsitem with whitespace in it (for exampleenv:prod team:pay)environmentVariables.DD_TAGSwhenglobalTagsis not set (this value does not change)The
globalTagsJSDoc, both READMEs, and the Fargate sample now show the comma-separated format.Motivation
The construct copied
globalTagsunchanged toDD_TAGS. For"env:staging,version:abc", the Agent made one tag,env:staging_version:abc. The sample, tests, and code all used commas, so users who followed them got incorrect metric and trace tags. This has been the behavior sincev2-2.3.0.Testing Guidelines
toAgentTags(including tokens) and for each warning. Tokens get no warning.", ", and one tag, with and withoutclusterName. Other tests check the precedence overenvironmentVariables.DD_TAGSand that env-onlyDD_TAGSdoes not change.DD_TAGS.jest: 466 tests pass.eslintis clean.Additional Notes
globalTagshas more than one comma-separated tag, the Agent now sends separate tags. Old combined values (for exampleteam:a_owner:b) stop. Monitors, dashboards, or usage attribution that filter on these values need an update. Log tags do not change. Put this in the release notes.DD_TAGSset only throughenvironmentVariablesdoes not change (a warning only). Normalizing it would change user-set env vars, so it goes in a future major.Types of Changes
Check all that apply
🤖 Generated with Claude Code