Revert Enhanced metrics changes

[RaphaelAllier]

We've been asked to revert the changes made to DD_ENHANCED_METRICS for a few reasons.

Automatically generated enhanced metrics may be confusing but they are free of charge (since metrics starts with aws., then don't count towards metric billing)
People only get billed for Datadog Serverless by aws.lambda.enhanced.invocations metric, which isn't generated by the forwarder (it's generated by our datadog-lambda library when a function gets instrumented. This behavior isn't ideal, but it has no real customer impact (all free), and it's inevitable, because the forwarder has no way to know if the logs' origin lambda function is instrumented or not (aka, whether ppl expect the forwarder to generate enhanced metrics or not). That's why we landed in that slightly confusing, yet no harm behavior.

The forwarder reuses datadog-lambda library (which would generate aws.lambda.enhanced.invocations) under the hood, therefore DD_ENHANCED_METRICS was originally default to false to prevent the forwarder from generating aws.lambda.enhanced.invocations for the forwarder itself.

To avoid potential regressions we decided along w/ our Serverless team to revert to the original behavior

[magnetikonline]

What's the reason for this revert @RaphaelAllier ?

[RaphaelAllier]

@magnetikonline Updated PR description to explain the change

[magnetikonline]

Thanks @RaphaelAllier for that.

That does clear things up - but the fact still remains, until 3.76 the default was to push these metrics - as I mentioned in #679. So I guess this means it's been wrong all along?

People only get billed for Datadog Serverless by aws.lambda.enhanced.invocations metric, which isn't generated by the forwarder (it's generated by our datadog-lambda library when a function gets instrumented.

Does this apply only to Python Lambda functions?

All in all, a little confused now - I might have to deploy one of these new releases and see how our metrics look.

[magnetikonline]

Infact - you've gone backwards now @RaphaelAllier 😄

aws-dd-forwarder-3.77.0...aws-dd-forwarder-3.83.0

So now:

• You once again provide DD_ENHANCED_METRICS in the CloudFormation stack.
• But once again it's never used! by the Python code.

So you've introduced a bug again?

[RaphaelAllier]

Yes, I've gone backward to try and address the issue.
To clarify, the env var DD_ENHANCED_METRICS isn't used in the Forwarder code, but it is used in the Datadog Lambda Library code the Forwarder reuses.
If set to True, it would publish a metric aws.lambda.enhanced.invocations when invoked, which we use for Serverless billing. That's why it's set to False by default (and that led to me misunderstanding the first time I had a look at this)
The Forwarder does generate some enhanced metrics from other lambdas when forwarding their logs, but they are free of charge.

[tianchu]

@magnetikonline I'm the person calling out for the revert and the original contributor to the forwarder. The missed historical context here is that DD_ENHANCED_METRICS was meant to control whether to generate enhanced metrics for the individual Lambda function where this env var was defined on, that's why it was set to false -- avoid generating the enhanced metrics for the forwarder lambda itself.

The recent changes (to be reverted) was overloading this env var's behavior to also control whether to generate enhanced metrics from logs for other lambda functions.

As @RaphaelAllier already mentioned, the original behavior was a bit confusing, but it had no real customer impact (the generated metrics are free of charge).

One alternative solution I can think of is to define a new env var say DD_ENHANCED_METRICS_FROM_LAMBDA_LOGS to specifically control wether to generate enhanced metrics from the forwarded lambda logs and that can default to true to avoid breaking changes. However, that may introduce a friction for customers who want to generate enhanced metrics for some lambda functions, but not all, they would need two forwarders -- that's actually why we didn't add a control for enhanced metrics generation in the first place.

That said, I don't see a perfect solution here, and reverting to the original behavior (no real issue for anyone since day 1) is probably the safest action.

@RaphaelAllier if you don't mind, perhaps you could help add a comment to explain this next to

datadog-serverless-functions/aws/logs_monitoring/template.yaml

Line 489 in f5ecc1e

DD_ENHANCED_METRICS: "false"

[magnetikonline]

Hey @RaphaelAllier / @tianchu - thanks so much for the detailed responses. That now all finally clicks as to they why and where 👍

that's why it was set to false -- avoid generating the enhanced metrics for the forwarder lambda itself.

Makes perfect sense - and 100% agree - so DD_ENHANCED_METRICS=false within the CloudFormation was for the benefit of the Datadog Lambda Library code the Forwarder uses internally 👍.

The function parse_and_submit_enhanced_metrics() relates to the parsing of logs and pushing out the enhanced metrics - for Lambdas that are not the log forwarder and emit enhanced metrics in their log output. That still remains and is active.

One alternative solution I can think of is to define a new env var say DD_ENHANCED_METRICS_FROM_LAMBDA_LOGS to specifically control wether to generate enhanced metrics from the forwarded lambda logs and that can default to true to avoid breaking changes.

yeah being purely selfish - having the log forwarder generating enhanced metrics isn't something I would need/use. agreed.

All understood - thanks again for the clarification! 👍