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

Automatically refer to the appropriate, regional FireLens image with addFirelensLogRouter() #7366

Open
1 of 2 tasks
blimmer opened this issue Apr 15, 2020 · 8 comments
Open
1 of 2 tasks
Assignees
Labels
@aws-cdk/aws-ecr Related to Amazon Elastic Container Registry @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 15, 2020

The documentation suggests using Amazon's own ECR images for the FireLens container. However, image is a required property.

It would be a nice convenience if CDK could determine the destination stack's region and automatically use the proper ecr.Repository.fromRepositoryArn() behavior.

Use Case

Today, I can get this behavior manually, but I had to generate the reference myself:

// Amazon recommends pulling this image from their regional ECR repo
// https://docs.aws.amazon.com/AmazonECS/latest/developerguide/using_firelens.html#firelens-using-fluentbit
const awsEcrRepo = ecr.Repository.fromRepositoryArn(
  parent,
  "AWSFireLensRepo",
  "arn:aws:ecr:us-west-2:906394416424:repository/aws-for-fluent-bit",
);
taskDefinition.addFirelensLogRouter("DatadogFirelensLogRouter", {
  image: ecs.ContainerImage.fromEcrRepository(awsEcrRepo, "latest"),
  memoryReservationMiB: 50,
  essential: true,
  firelensConfig: {
    type: ecs.FirelensLogRouterType.FLUENTBIT,
    options: {
      enableECSLogMetadata: true,
      configFileType: ecs.FirelensConfigFileType.FILE,
      configFileValue: "/fluent-bit/configs/parse-json.conf",
    },
  },
});

Proposed Solution

Ideally, image on FirelensLogRouterDefinitionOptions is an optional parameter and defaults to the proper ECR image from this table in the docs.

Other

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

This is a 🚀 Feature Request

@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 15, 2020
@SomayaB SomayaB added the @aws-cdk/aws-ecr Related to Amazon Elastic Container Registry label Apr 16, 2020
@SomayaB SomayaB added the @aws-cdk/aws-ecs Related to Amazon Elastic Container label Apr 16, 2020
@bvtujo bvtujo added effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2 and removed needs-triage This issue or PR still needs to be triaged. labels Apr 17, 2020
@bvtujo
Copy link
Contributor

bvtujo commented Apr 17, 2020

Thanks for bringing this to our attention! Definitely let us know if you're willing to work on the PR; otherwise someone from our team will pick it up.

@rcollette
Copy link
Contributor

The package @aws-cdk/aws-ecs exports the function obtainDefaultFluentBitECRImage. This will reference the appropriate regional image for fluent bit.

You can view usage here:
https://stackoverflow.com/questions/64299664/how-to-configure-aws-cdk-applicationloadbalancedfargateservice-to-log-parsed-jso/

I will say that it takes a lot of discovery to figure out how to override the default logDriver to enable json line parsing. Definitely either more documentation or a simpler API would be helpful.

@MrArnoldPalmer MrArnoldPalmer removed their assignment Jun 21, 2021
@github-actions
Copy link

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Jun 21, 2022
@rcollette
Copy link
Contributor

keep

@github-actions github-actions bot removed the closing-soon This issue will automatically close in 4 days unless further comments are made. label Jun 21, 2022
@PettitWesley
Copy link
Contributor

PettitWesley commented Dec 7, 2022

Q: I can't find any docs for obtainDefaultFluentBitECRImage.

So I think the AWS ECS FireLens constructs are supposed to live here now: https://github.com/cdklabs/cdk-ecs-service-extensions/

But then the function still seems to only be here: obtainDefaultFluentBitECRImage in this repo.

Are there no docs for this function? Or do I not know the right place to look? Is it not intended for end-user direct use?

cdklabs/cdk-ecs-service-extensions#249

@SomayaB @uttarasridhar

@madeline-k
Copy link
Contributor

Hello @PettitWesley, obtainDefaultFluentBitECRImage is just a free-floating function, so it's not included in the JSII assembly (i.e. it's omitted from the API in non -TS/JS langs), and it's excluded from the docs. I am not sure if that was the original intent for this function or not.

If you want to use it in TS/JS, you can, see the documentation in our source code: https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-ecs/lib/firelens-log-router.ts#L149-L155

I think this feature request is still valid, and this function is only a workaround for TS/JS users.

@PettitWesley
Copy link
Contributor

@madeline-k How would I add it to the JSII assembly so that it will become a real official function?

@madeline-k
Copy link
Contributor

madeline-k commented Jan 26, 2023

Only Classes, Interfaces, and Enums are included. So it would need to be added to a relevant class. Edit: Or possibly a new class made for this? I don't have a strong opinion yet, would need to spend some time diving into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ecr Related to Amazon Elastic Container Registry @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
Projects
None yet
Development

No branches or pull requests

9 participants