DataDog / serverless-plugin-datadog

Serverless plugin to automagically instrument your Lambda functions with Datadog
Apache License 2.0
96 stars 49 forks source link

Plugin configuration with only log forwarding (via forwarder) and monitors should not require wrapper #480

Closed max-lobur closed 7 months ago

max-lobur commented 8 months ago

Expected Behavior

Extensions/Layers/Library should not be not required when using plugin only to link log forwarder and setup monitors.

Actual Behavior

Lambda error:

/opt/datadog_wrapper: does not exist

Steps to Reproduce the Problem

  datadog:
    enabled: true
    site: us5.datadoghq.com
    apiKey: <>
    appKey: <>
    enableDDLogs: false
    enableXrayTracing: false
    enableDDTracing: false
    enableASM: false
    enableProfiling: false
    enableSourceCodeIntegration: false
    enableColdStartTracing: false
    uploadGitMetadata: false
    addLayers: false
    addExtension: false
    encodeAuthorizerContext: false
    decodeAuthorizerContext: false
    subscribeToAccessLogs: true
    subscribeToExecutionLogs: true
    forwarderArn: <arn>
    failOnError: true
    logLevel: DEBUG
    service: api
    env: dev
    monitors:
      - high_error_rate: {}
      - timeout: {}
      - high_throttles: {}
sls deploy

Specifications

Stacktrace

/opt/datadog_wrapper: does not exist
astuyve commented 8 months ago

Hi @max-lobur, thanks for reaching out!

Can you remove enableASM: false and try again?

I'll reach out to that group and ask them review a change to fix this logic. I'm sorry about this.

Thanks! AJ

max-lobur commented 8 months ago

@astuyve with enableASM: false removed I'm getting

No module named 'datadog_lambda'

Guess it's not over yet

With addExtension: true it prints out

Warning: Datadog Lambda Extension and forwarder are both enabled. Only APIGateway log groups will be subscribed to the forwarder.

So not desirable either.

It works with addLayers: true , but that's still adding an execution layer which is not used by us

max-lobur commented 8 months ago

let me know if you need the new issue

astuyve commented 8 months ago

Hi Max, sorry about that, I forgot GitHub closes issues when the corresponding PR is merged.

For now, one more thing to try is setting customHandler: your-handler.handler

This will prevent this library from attempting to wrap your handler, as in your case addLayers is not set so there is no datadog_lambda module. Unfortunately that's not what this setting was designed for, so this path only works for templates with one handler.

The reason for this friction is that this library was not designed to simply subscribe Datadog forwarders to log groups or create monitors. The purpose of this plugin is to add tracing libraries and the extension, but with all of these disabled the library is effectively moot. It always redirects the handler. We offer auto subscription to log groups, and have other automations for creating monitors.

That said, I'm still committed to resolving this issue for you and certainly getting to a place where the cloudformation template is correct, it just may require another change.

max-lobur commented 8 months ago

I see, thanks! Maybe worth splitting the plugin to two parts

astuyve commented 8 months ago

Maybe eventually? I think redirectHandlers covers your use case and a bit more. #482 should do it. You can clone this repo, pull this branch, and test your project by:

  1. In the clone of branch aj/skip-redirect-handlers-optionally, run npm link
  2. In your own project, run npm link serverless-plugin-datadog

Let me know!

max-lobur commented 7 months ago

customHandler: your-handler.handler won't work as I plan to deploy multiple functions , and these settings are global (am I right?) redirectHandlers - yeah good tradeoff for now , but don't have time to test it, I'll wait for the release. Thanks!

astuyve commented 7 months ago

Correct, custom handlers are a global setting. Okay, will do a release later today.

max-lobur commented 7 months ago

Just tested the latest release, it works, thanks!