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

Cannot apply addLambdaFunctions via aspect: "We detected an Aspect was added via another Aspect, and will not be applied" #114

Open
mhenniges opened this issue Jun 28, 2022 · 2 comments
Labels
enhancement New feature or request

Comments

@mhenniges
Copy link

Expected Behavior

I attempted to write an aspect that would automatically set up tracing for my nodeJS lambdas. I expected this would result in full instrumentation of these functions without multiple source file edits.

Actual Behavior

Aspect fails to apply with an error "We detected an Aspect was added via another Aspect, and will not be applied".

Steps to Reproduce the Problem

Example aspect for datadog:

abstract class TraceAgentBase implements IAspect {

    protected readonly props: DatadogProps;
    protected readonly ddtrace: Datadog;
    // protected readonly scope: IConstruct;

    protected constructor(ddtrace: Datadog, props: DatadogProps) {
        this.props = props;
        this.ddtrace = ddtrace
    }

    public visit(node: IConstruct): void {
        if (node instanceof NodejsFunction) {
            this.applyNodeTracingToLambda(node)
        }
    }
    protected abstract applyNodeTracingToLambda(resource: NodejsFunction): void;
}
`
export class TraceAgent extends TraceAgentBase  {
    constructor(ddtrace: Datadog, props: DatadogProps) {
        super(ddtrace, props);
    }
    protected applyNodeTracingToLambda(resource: NodejsFunction) {
        this.ddtrace.addLambdaFunctions([resource])
        resource.addEnvironment('DD_MERGE_XRAY_TRACES', 'true');
        const cfnFunc = resource.node.defaultChild as CfnFunction;
        cfnFunc.tracingConfig = {mode: Tracing.ACTIVE};
    }
}

apply to stack with something like this:

        Aspects.of(runner).add(new TraceAgent(ddtrace, {
            apiKey: DatadogAPISecret.secretValue.toString(),
            nodeLayerVersion: 78,
            extensionLayerVersion: 23,
            env: 'dev',
            service: serviceName,
            version: '0.1.2-datadog',
            captureLambdaPayload: true,
        }));

Specifications

  • Datadog Lambda Layer version: 23
  • Node version: 78

Stacktrace

None available

After much debugging, I've tracked this down to the use of Tags.of() in addCdkConstructVersionTag and setTags() calls starting here - Tags.of() is an aspect as well.

Has anybody else hit this, and did you work around it successfully? Has there been any thought or exploration into how to make the datadog construct aspect-compatible?

Although not ideal, I believe this can be solved by using something like construct.tags.setTag(key, value, 100, true) to set tags directly, rather than calling the aspect to do the same. There won't be much (any?) cascading of tags to child objects in this case, so really not much is lost.

Happy to make a PR if there is no objection.

@astuyve astuyve added the enhancement New feature or request label Jun 28, 2022
@astuyve
Copy link
Contributor

astuyve commented Jun 28, 2022

Hey @mhenniges - this library wasn't specifically written to be implemented to support an Aspect, but we'd happily look at any PR you'd submit for this. Thanks!

@felixmagnus
Copy link

Hey @mhenniges - we just ran into the same problem 😄 Did you come up with any workaround for this yet?
Would be great if you could share any findings - thanks!

@astuyve astuyve mentioned this issue Feb 16, 2024
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants