(aws-synthetics): Canary inline code doesn’t allow other values for handler expect index.handler

rgoltz commented 3 years ago

We like to deploy and manage an AWS CloudWatch Canary via CDK. We like to define our Canaries-Definition (Canary using Lambda in the background) in Python – Our CDK is using TypeScript. Canary inline code doesn’t allow other values for handler expect index.handler

Reproduction Steps

"good case": using index.handler (please see chapter "Notes / Reference to Canary Bug" = CDK is fine (Canary isn't)

`

export class DemoStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

        const short_script =
        'def handler(event, context): \n' +
        '   return \"Successfully completed basicCanary checks.\" \n'

        const canary = new synthetics.Canary(this, "HealthyAtHome", {
          canaryName: 'healthyathome',
          runtime: new synthetics.Runtime('syn-python-selenium-1.0'),
          test: synthetics.Test.custom({
            code: synthetics.Code.fromInline(short_script),
            handler: 'index.handler'
          })
        });
  }
}`

"bad case": using an other value for handler = CDK reject this [same code as above, just value for handler is changed from 'index.handler' to 'alivecheck.handler'].

`

export class DemoStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

        const short_script =
        'def handler(event, context): \n' +
        '   return \"Successfully completed basicCanary checks.\" \n'

        const canary = new synthetics.Canary(this, "HealthyAtHome", {
          canaryName: 'healthyathome',
          runtime: new synthetics.Runtime('syn-python-selenium-1.0'),
          test: synthetics.Test.custom({
            code: synthetics.Code.fromInline(short_script),
            handler: 'alivecheck.handler'
          })
        });
  }
}`

What did you expect to happen?

I’m able to define a Canary in CDK as Python inline-code and define a handler value based on my needs, which fitting the general pattern .handler. The value-check for inline code follow the same logic / verifications, like aws-synthetics/lib/canary.ts (see line 33 - 40)

What actually happened?

During cdk synth, I’m getting the error:

Error: The handler for inline code must be "index.handler" (got "alivecheck.handler")

I’ve tried to identify the CDK-part and I found that for inline-code following check is additional in place: aws-synthetics/lib/code.ts (see line 157 - There is a “hard” check for index.handler value).

Environment

CDK CLI Version : 1.111.0
OS : Windows 10
Language (Version): TypeScript

Notes / Reference to Canary Bug (Other)

Currently there is an Issue (Bug) in AWS Canary-Service in case the “Canary script” is defined in Python AND it’s using handler = index.handler. It seems that Canary is “wrapping” the defined Canary-Code into another (within the Lambda in the background). Once you deployed a (simple) Python-Code for your Canary with index.handler, you getting in Canary-Log:

INFO: Customer canary entry file name: index INFO: Customer canary entry function name: handler INFO: Calling customer canary: index.handler() ERROR: Canary execution exception. Traceback (most recent call last): File "/var/task/index.py", line 76, in handle_canary response = customer_canary.handler(event, context) File "/var/task/index.py", line 13, in handler return asyncio.run(handle_canary(event, context))
File "/var/lang/lib/python3.8/asyncio/runners.py", line 33, in run raise RuntimeError(RuntimeError: asyncio.run() cannot be called from a running event loop`

In case you use an other value for handler and deploy this via CFN, it's working fine.

By keeping both issues in mind: CDK doesn't allow other value for handler + Canary working only with a handler value except index.handler we are currently in a "deadlock". May aws-synthetics/lib/code.ts could be a little bit more 'flexible' for handler?

This is :bug: Bug Report

BenChaimberg commented 3 years ago

From the CFN docs:

If you include your function source inline with this parameter, AWS CloudFormation places it in a file named index and zips it to create a deployment package.

This means that the value provided for "handler" must begin with "index" since that is the file where your handler function is located.

BenChaimberg commented 3 years ago

Ah, I see the part about the canary not running your code correctly; are you sure the name of the handler is what is causing that issue? It does not seem entirely related

rgoltz commented 3 years ago

Hi @BenChaimberg - Thank you very much for taking over and checking on this issue. Thanks for the reference to CFN documentation. I'd like to share some more findings with you here. In a nutshell, we see the root cause by an issue in CloudWatch Canary Ressource once using (any) custom Python-Code. I'm aware that's an issue of an other AWS-team.

You can reconstruct the issue by doing the following two steps:

1) Using the (plain) High-Level-Construct for Canary

export class DemoStack extends cdk.Stack {
    constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
      super(scope, id, props);

          const short_script =
          'def handler(event, context): \n' +
          '   return \"Successfully completed basicCanary checks.\" \n'

          const canary = new synthetics.Canary(this, "HealthyAtHome", {
            canaryName: 'healthyathome',
            runtime: new synthetics.Runtime('syn-python-selenium-1.0'),
            test: synthetics.Test.custom({
              code: synthetics.Code.fromInline(short_script),
              handler: 'index.handler', 
            })
          });
    }
  }

expect result: Canary would be executed without an error. actual state: Canary reporting an error: asyncio.run() cannot be called from a running event loop - We are running into this Canary bug, since we have to use index.handler. You can see this error in Canary AWS Console UI.

2) Using the High-Level-Construct, but overwriting the code prop to define an other value for handler by using Low-Level Cfn-Construct

  export class DemoStack extends cdk.Stack {
    constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
      super(scope, id, props);

          const short_script =
          'def handler(event, context): \n' +
          '   return \"Successfully completed basicCanary checks.\" \n'

          const canary = new synthetics.Canary(this, "HealthyAtHome", {
            canaryName: 'healthyathome',
            runtime: new synthetics.Runtime('syn-python-selenium-1.0'),
            test: synthetics.Test.custom({
              code: synthetics.Code.fromInline(`print("OverwriteMeLater")`),
              handler: 'index.handler'
            })
          });

          const cfnCanary = canary.node.defaultChild as synthetics.CfnCanary
          cfnCanary.code = {
            script: short_script,
            handler: 'alivecheck.handler',
          }
    }
  }

expect result: Canary would be executed without an error. Canary configuration using value alivecheck.handler (since it's getting overwritten from low-level cfnCanary code) actual state: Canary + Code is working fine, since we avoid to use index.handler as handler.

In both cases we are using the same supersimple example-code (here short_script).
CFN itself (and Canary Ressource) supporting a other value for handler, as long as it's ending with .handler
To deploy an other value expect index.handler, it's "blocked" by CDK (following the CFN-docs). I assume the docs currently not representing the current possibilities for handler-settings of Canary.

It would be really helpful and great if you can cross-check our findings (in terms of CDK and Canary/CFN).

In case you support or simplify your testing, please let me know.

Cheers, Robert :)

BenChaimberg commented 3 years ago

Confirmed this behaviour – weird! Synthetics seems to overwrite Lambda's default behaviour of placing inline code into a file named "index.py" and instead places it in a file named after the first part of the handler string. On top of that, the module-loading behaviour for Python modules breaks when the customer handler file name is "index.py" because the canary handler that wraps the customer handler is also in a file called "index.py". I notified the Synthetics team about this (internal reference: P50456886).

The workaround is to either use the escape hatch method which is outlined above or to use some other method of inserting code (fromAsset or fromBucket), as long as the entrypoint file name isn't "index.py".

This is still a bug in our library, since the placement behaviour is the same for the node.js runtimes. We should remove the validation that the handler name for inline code is strictly "index.handler". Also since the second part must be "handler" and the first part apparently doesn't matter, maybe we should generate the whole thing...

BenChaimberg commented 3 years ago

I am unassigning myself and marking this issue as p2, which means that we are unable to work on this immediately.

We use +1s to help prioritize our work, and are happy to reevaluate this issue based on community feedback. You can reach out to the cdk.dev community on Slack to solicit support for reprioritization.

Please feel free to work on this issue yourself and request my review if you submit a PR or would like design help! See CONTRIBUTING.md for guidelines.

github-actions[bot] commented 2 years ago

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.

rgoltz commented 2 years ago

Hey @greg5123334 - Let's re-visit this issue here and sync on status regarding this. I've pushed an note to your queue ;-)

kaizencc commented 1 year ago

"Bug is fixed in syn-python-selenium-1.3 runtime."

Tested this to confirm that is the case with later runtimes. Since this is fixed on the synthetics side we're not going to need to do anything here

github-actions[bot] commented 1 year ago

⚠️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 / aws-cdk