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

AwsLogDriverProps interface property streamPrefix should be required #1422

Closed
nathanpeck opened this issue Dec 21, 2018 · 3 comments
Closed
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container feature-request A feature should be added or improved.

Comments

@nathanpeck
Copy link
Member

nathanpeck commented Dec 21, 2018

The AwsLogDriverProps interface has a property streamPrefix. This is currently not required by CDK, so you can cdk synth and attempt to cdk deploy the stack without specifying this property. However CloudFormation deployment will then fail with an error:

Fargate requires log configuration options to include awslogs-stream-prefix
to support log driver awslogs. (Service: AmazonECS; Status Code: 400;
Error Code: ClientException

Example code that reproduces the issue:

    const definition = new ecs.FargateTaskDefinition(this, 'definition', {});
    definition.addContainer('my-app', {
      image: ecs.ContainerImage.fromAsset(this, 'my-app-image', { directory: './app' }),
      logging: new ecs.AwsLogDriver(this, 'my-app', {})
    });

    // Launch the image as a service in Fargate
    new ecs.FargateService(this, 'app', {
      cluster: cluster, 
      cpu: '256',
      memoryMiB: '512',
      desiredCount: 1,
      taskDefinition: definition
    });

Ideally this streamPrefix property should be marked as required by CDK for Fargate task definitions such that cdk synth and cdk deploy fail locally with an error message prior to sending the synthesized CF template off to CloudFormation

Alternatively perhaps CDK could choose to automatically fill the streamPrefix with the container name, as this is a fairly logical default.

@eladb eladb added enhancement @aws-cdk/aws-ecs Related to Amazon Elastic Container labels Dec 23, 2018
@eladb
Copy link
Contributor

eladb commented Dec 23, 2018

If we have a good smart default, that's always the preferred option in the CDK.

@srchase srchase added feature-request A feature should be added or improved. and removed enhancement labels Jan 3, 2019
@piradeepk
Copy link
Contributor

The streamPrefix is already a required property.

@SoManyHs
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

5 participants