-
Notifications
You must be signed in to change notification settings - Fork 397
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
Feature request: Add support for default dimensions on ColdStart metric captured with log_metrics decorator #5237
Comments
Thanks for opening your first issue here! We'll come back to you as soon as we can. |
Hi, thank you for opening this issue. Just for added context, I have gone ahead and created a sample project to help with contextualizing the request and better see the differences between the two versions. Given these two functions: TypeScript: import { Metrics } from '@aws-lambda-powertools/metrics';
import { logMetrics } from '@aws-lambda-powertools/metrics/middleware';
import middy from '@middy/core';
const metrics = new Metrics({
namespace: 'MyApp',
});
export const handler = middy(async () => {
return {
statusCode: 200,
body: JSON.stringify('Hello, World!'),
};
}).use(
logMetrics(metrics, {
throwOnEmptyMetrics: false,
captureColdStartMetric: true,
defaultDimensions: { Environment: 'prod' },
})
); Python: from aws_lambda_powertools import Metrics
from aws_lambda_powertools.metrics import MetricUnit
from aws_lambda_powertools.utilities.typing import LambdaContext
metrics = Metrics(namespace="MyApp")
@metrics.log_metrics(raise_on_empty_metrics=False, capture_cold_start_metric=True, default_dimensions={"Environment": "prod"})
def lambda_handler(event: dict, context: LambdaContext):
return {"statusCode": 200, "body": "Hello World"} The EMF blog emitted by the two is similar but different in three aspects:
Below the full metric logs: TypeScript: {
"_aws": {
"Timestamp": 1727339583244,
"CloudWatchMetrics": [
{
"Namespace": "MyApp",
"Dimensions": [
[
"service",
"Environment",
"function_name"
]
],
"Metrics": [
{
"Name": "ColdStart",
"Unit": "Count"
}
]
}
]
},
"service": "service_undefined",
"Environment": "prod",
"function_name": "DimensionsFn",
"ColdStart": 1
} Python: {
"_aws": {
"Timestamp": 1727339552523,
"CloudWatchMetrics": [
{
"Namespace": "MyApp",
"Dimensions": [
[
"function_name"
]
],
"Metrics": [
{
"Name": "ColdStart",
"Unit": "Count"
}
]
}
]
},
"function_name": "DimensionsFnPython",
"ColdStart": [
1.0
]
} For the purpose of this issue, I think we are focusing on the first bullet point, the default dimensions being included in the cold start metric. I see value in aligning the behavior between the two versions, although I don't have a strong opinion on which one should be the preferred one. What I can say for sure is that making this change in either of the two libraries would be a breaking change, so we'll need to address this in a backwards compatible way if we decide to do so. |
Hi @dreamorosi, thank you very much for this investigation using both runtimes! This will be very useful in helping us make a decision. I have a few considerations below.
Yes, we don't include the default dimensions in Python, which I think is wrong behavior because we should consider the default dimension in all metrics, including
We also include the service dimension in Python too. In this case, you're not seeing it because you didn't define the service using the parameter constructor or the env variable.
In this case, there is no difference in using an int/float or a list with int/float. We do this to try to optimize as much as possible the blob that we emit to EMF, so in cases where you have two metrics, we define both in the
I also see value in aligning behavior, and I think including default dimensions in the
This is the most problematic point in this discussion. If we decide to include default dimensions in the I was thinking about creating a new parameter in @metrics.log_metrics(capture_cold_start_metric=True, include_default_dimensions_cold_start=True)
def lambda_handler(event: dict, context: LambdaContext): I don't like the name of this parameter, but it's just for illustration. Please let me know what do you think @angelcabo and @dreamorosi. |
Thanks for clarifying the int/float difference, for the service name I think the Python behavior is better for cost savings, but I can see why it was implemented this way in TS - anyway for now onwards I'll focus the discussion only on the default dimensions being included in the cold start metric. Seeing how we all agree that default dimensions should be included in that metric, I think we have two options forward. One is the one you suggested, making the behavior opt-in in the current major version and eventually default in the next one. The other option would be to consider this a bug and fix it right away. I acknowledge that the line here is quite blurry, however while the With this in mind, I think there's room for interpret them being excluded as an unintended behavior and thus treat this as a bug fix. There are also precedents for this type of change being treated as bugs and having been fixed within minor releases. Either way, I think there's a potential for data loss. Customers might be trying to set up dashboards that query the cold start metric with certain dimensions expecting to find values because they set If we go to the route you suggested, I'd like us to consider using an environment variable instead of adding another parameter. I think in the long term this would make the v3 to v4 switch easier, and the API less verbose in the code. For example, having an env variable like |
Use case
In my environment, I have two serverless applications: one built using Python and the other using TypeScript/Node.js. Both utilize AWS Powertools. However, I have noticed a discrepancy between the two in how they handle CloudWatch cold start metrics by default.
In the Node.js Powertools, the middleware automatically includes default dimensions such as service name, function name, and additional custom dimensions.
On the other hand, in the Python Powertools, the cold start metric only includes the function and service name by default, with no apparent way to extend or override this behavior using custom dimensions without completely managing the cold start metric manually.
This creates an inconsistency between the two libraries and forces us to write additional boilerplate code in Python if we want additional dimensions to be recorded along with the ColdStart metric, while the Node.js implementation provides a more seamless experience.
FWIW:
The docs are accurate in both cases:
Python's page says:
and TypeScript's page says:
Solution/User Experience
Ideally, the
default_dimensions
provided to the log_metrics decorator would be used when recording the cold start metric in Python.Alternative solutions
As mentioned, an alternative is to continue to have users manually handle the cold start metrics themselves to support adding custom dimensions to the ColdStart metric in Python.
Acknowledgment
The text was updated successfully, but these errors were encountered: