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

(ecs): ContainerDefinition.addDockerLabel #29728

Closed
1 of 2 tasks
blimmer opened this issue Apr 4, 2024 · 9 comments · Fixed by #29734, rwlxxvii/containers#124 or rwlxxvii/containers#140 · May be fixed by NOUIY/aws-solutions-constructs#98 or NOUIY/aws-solutions-constructs#99
Closed
1 of 2 tasks
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@blimmer
Copy link
Contributor

blimmer commented Apr 4, 2024

Describe the feature

It would be nice to be able to add docker labels after the container has been constructed.

For example, I can call addEnvironment to add an environment variable after the fact: https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ecs.ContainerDefinition.html#addwbrenvironmentname-value

However, I can't do the same with docker labels.

Use Case

I have a construct that automatically applies Datadog monitoring information to a TaskDefinition with an application container. Datadog requires that I set some information as Docker Labels for observability:

Screenshot 2024-04-04 at 12 23 53

However, I want to add these labels after the user already created their default container.

Proposed Solution

Add a .addDockerLabel method similar to .addEnvironment to the ContainerDefiniton construct. It'd be similar to #2559 / #17889

Other Information

You can hack this like so:

const defaultContainerAsAny = defaultContainer as any;
defaultContainerAsAny.props.dockerLabels = Object.assign(defaultContainerAsAny.props.dockerLabels || {}, {
  "com.datadoghq.tags.env": this.env,
  "com.datadoghq.tags.service": serviceName,

  // TODO: The ECS CircleCI orb doesn't expose the ability to set `dockerLabels` when deploying
  // a task definition update. See https://github.com/CircleCI-Public/aws-ecs-orb/issues/119
  // "com.datadoghq.tags.version": "",

  "com.datadoghq.ad.instances": JSON.stringify([
    { host: "%%host%%", port: defaultContainer.portMappings.length ? defaultContainer.ingressPort : null },
  ]),
  "com.datadoghq.ad.check_names": JSON.stringify([serviceName]),
  "com.datadoghq.ad.init_configs": JSON.stringify([{}]),
});

But it's not ideal

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.135.0

Environment details (OS name and version, etc.)

MacOS Sonoma

@blimmer blimmer added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Apr 4, 2024
@github-actions github-actions bot added the @aws-cdk/aws-ecs Related to Amazon Elastic Container label Apr 4, 2024
@khushail khushail added investigating This issue is being investigated and/or work is in progress to resolve the issue. p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Apr 4, 2024
@khushail
Copy link
Contributor

khushail commented Apr 4, 2024

@blimmer ,thanks for reaching out with this request and volunteering for PR contribution.

@khushail khushail removed the investigating This issue is being investigated and/or work is in progress to resolve the issue. label Apr 4, 2024
@blimmer
Copy link
Contributor Author

blimmer commented Apr 4, 2024

@khushail - I'm not sure when I'll be able to get to this. Would you mind also throwing a "good first issue" tag on it?

@khushail khushail added the good first issue Related to contributions. See CONTRIBUTING.md label Apr 4, 2024
@khushail
Copy link
Contributor

khushail commented Apr 4, 2024

@khushail - I'm not sure when I'll be able to get to this. Would you mind also throwing a "good first issue" tag on it?

done!

@alpha951
Copy link

Hey @khushail , I'm looking for my first contribution here. Could you please assign this issue to me?

@khushail
Copy link
Contributor

khushail commented Apr 15, 2024

Thanks @alpha951 for volunteering to contribute. I just checked there is already a PR submitted.

@alpha951
Copy link

Let me know if there are any other beginner friendly issues are still open.

@khushail
Copy link
Contributor

Let me know if there are any other beginner friendly issues are still open.

You can filter out the issues with label "good first issue" and would get a list like this and can pick to work on any issue, you want

@mergify mergify bot closed this as completed in #29734 Apr 18, 2024
mergify bot pushed a commit that referenced this issue Apr 18, 2024
…29734)

Closes #29728. 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@aws-cdk-automation
Copy link
Collaborator

Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.

@aws aws locked as resolved and limited conversation to collaborators Jul 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.