aws-powertools / powertools-lambda-dotnet

Powertools is a developer toolkit to implement Serverless best practices and increase developer velocity.
https://docs.powertools.aws.dev/lambda/dotnet/
MIT No Attribution
158 stars 24 forks source link

Bug: Unable to push metrics with multiple dimensions #662

Open Euclidite opened 1 week ago

Euclidite commented 1 week ago

Expected Behaviour

I expect to be able to push metrics with multiple dimensions either via PushSingleMetric or multiple calls to AddDimension.

Current Behaviour

When using any of the following:

Metrics.SetDefaultDimensions(new Dictionary<string, string> {
    { "SessionId", input.SessionId ?? "Unset" }
});

Metrics.PushSingleMetric("Lambda Execute", 1, MetricUnit.Count, metricResolution: MetricResolution.High,
    defaultDimensions: new Dictionary<string, string> {
        { "Type", "Start" }
});
Metrics.PushSingleMetric("Lambda Execute", 1, MetricUnit.Count, metricResolution: MetricResolution.High,
    defaultDimensions: new Dictionary<string, string> {
        { "Type", "Start" },
        { "SessionId", input.SessionId ?? "Unset" }
});
Metrics.AddMetric("Lambda Execute", 1, MetricUnit.Count, MetricResolution.High);
Metrics.AddDimension("SessionId", input.SessionId ?? "Unset");
Metrics.AddDimension("Type", "Start");

The following EMF is logged:

{
    "_aws": {
        "Timestamp": 1728338129260,
        "CloudWatchMetrics": [
            {
                "Namespace": "Services",
                "Metrics": [
                    {
                        "Name": "Lambda Execute",
                        "Unit": "Count",
                        "StorageResolution": 1
                    }
                ],
                "Dimensions": [
                    [
                        "SessionId"
                    ],
                    [
                        "Type"
                    ],
                    [
                        "Service"
                    ]
                ]
            }
        ]
    },
    "SessionId": "3da49fdb-aa91-4523-a6ea-a030817d2410",
    "Type": "Start",
    "Service": "coordinator",
    "Lambda Execute": 1
}

And the following appears in the CloudWatch console: Image

Essentially, it creates 3 different metrics for each dimension.

Code snippet

Metrics.AddMetric("Lambda Execute", 1, MetricUnit.Count, MetricResolution.High);
Metrics.AddDimension("SessionId", input.SessionId ?? "Unset");
Metrics.AddDimension("Type", "Start");

Possible Solution

No response

Steps to Reproduce

See code snippet

Powertools for AWS Lambda (.NET) version

latest

AWS Lambda function runtime

dotnet8

Debugging logs

No response

boring-cyborg[bot] commented 1 week ago

Thanks for opening your first issue here! We'll come back to you as soon as we can. In the meantime, check out the #dotnet channel on our Powertools for AWS Lambda Discord: Invite link

hjgraca commented 1 week ago

@Euclidite thanks for reporting the issue, we are actively working on a fix.

leandrodamascena commented 1 week ago

Hello @hjgraca and @Euclidite! Yes, I agree the way the metric is being serialized today needs to be changed. Dimensions has to be a string array and not a list array, because in this case it will have the side effect of not aggregating the dimensions into a metric and creating 3 instead.

The official documentations says:

A DimensionSet is an array of strings containing the dimension keys that will be applied to all metrics in the document. The values within this array MUST also be members on the root-node—referred to as the Target members

The EMF BLOB should be:

"_aws": {
        "Timestamp": 1728338129260,
        "CloudWatchMetrics": [
            {
                "Namespace": "Services",
                "Metrics": [
                    {
                        "Name": "Lambda Execute",
                        "Unit": "Count",
                        "StorageResolution": 1
                    }
                ],
                "Dimensions": [
                    [
                        "SessionId",
                        "Type",
                        "Service"
                    ]
                ]
            }
        ]
    },
    "SessionId": "3da49fdb-aa91-4523-a6ea-a030817d2410",
    "Type": "Start",
    "Service": "coordinator",
    "Lambda Execute": 1
    }
Euclidite commented 1 week ago

Thank you for looking at this so quickly! I'll keep an eye on this in case you need me to test the fix or need any additional info.

hjgraca commented 1 week ago

@Euclidite thanks again for reporting the issue we have a fix ready to merge, if everything goes as planned we will be releasing tomorrow. I will keep you updated. #664

hjgraca commented 1 week ago

@Euclidite Metrics version 1.8.0 release. This new version will fix the issue reported. Please let us know how your tests go. Thanks again for reporting the issue.

Euclidite commented 1 week ago

@hjgraca I just tested it out, the EMF is now:

{
    "_aws": {
        "Timestamp": 1728489768978,
        "CloudWatchMetrics": [
            {
                "Namespace": "Services",
                "Metrics": [
                    {
                        "Name": "Lambda Execute",
                        "Unit": "None",
                        "StorageResolution": 1
                    }
                ],
                "Dimensions": [
                    "ServiceName",
                    "SessionId",
                    "Type"
                ]
            }
        ]
    },
    "ServiceName": "coordinator",
    "SessionId": "cb236318-0817-41af-beac-02ef2c36880a",
    "Type": "Start",
    "Lambda Execute": 1
}

I've waited about an hour, but this time nothing appears in CloudWatch metrics - we only have ~170 instances of this metric logged (from a Python service which uses Powertools too), so I don't think I'm just overlooking it.

This is my code:

        Metrics.ClearDefaultDimensions();
        Metrics.SetDefaultDimensions(new Dictionary<string, string> {
            { "ServiceName", context.FunctionName },
            { "SessionId", input.SessionId ?? "Unset" }
        });

        Metrics.AddMetric("Lambda Execute", 1, MetricUnit.None, MetricResolution.High);
        Metrics.AddDimension("Type", "Start");

FYI (Separate issue if you're curious) - I would have used the default Service dimension, but the Python Powertools library logs it as service, the C# library logs it as Service - so we ended up using ServiceName to avoid differing dimensions

hjgraca commented 1 week ago

@Euclidite you are correct, there is a mistake on the Dimensions, missing a [], it should output:

"Dimensions": [
                    [
                        "ServiceName",
                        "SessionId",
                        "Type"
                    ]
                ]

Working to fix that.

By default .NET serialization is PascalCase that is why you are seeing Service instead of service. The Powertools Logging utility supports multiple cases (snake_case, camelCase and PascalCase), we should probably consider adding that to Metrics. Thanks for raising that.

hjgraca commented 1 week ago

@Euclidite can you please update to the preview version 1.8.1-alpha and test. https://www.nuget.org/packages/AWS.Lambda.Powertools.Metrics/1.8.1-alpha Thank you

Euclidite commented 1 week ago

@hjgraca Looks like that did it! When I run the following, I see a single metric in the console with all my dimensions:

Metrics.ClearDefaultDimensions();
Metrics.SetDefaultDimensions(new Dictionary<string, string> {
    { "ServiceName", context.FunctionName },
    { "SessionId", input.SessionId ?? "Unset" }
});

Metrics.AddMetric("Lambda Execute", 1, MetricUnit.None, MetricResolution.High);
Metrics.AddDimension("Type", "Start");

However, if I do the following:

Metrics.ClearDefaultDimensions();
Metrics.SetDefaultDimensions(new Dictionary<string, string> {
    { "ServiceName", context.FunctionName },
    { "SessionId", input.SessionId ?? "Unset" }
});

Metrics.AddMetric("Lambda Execute", 1, MetricUnit.None, MetricResolution.High);
Metrics.AddDimension("Type", "Start");

Metrics.AddMetric("Lambda Execute", 1, MetricUnit.Count, MetricResolution.High);
Metrics.AddDimension("Type", "End");

I only get the first metric - not the second one. And I see ##WARNING##: Failed to Add dimension 'Type'. Dimension already exists. in the logs.

My intention here is to change the Type dimension value for the second metric, but I don't see any Set method - is that not possible? (Please let me know if this is a different issue / you want a new ticket)

hjgraca commented 1 week ago

@Euclidite

That behaviour should be expected, to overcome it you could push a single metric with PushSingleMetric method

We have this section in the docs https://docs.powertools.aws.dev/lambda/dotnet/core/metrics/#single-metric-with-a-different-dimension but I believe python docs have more info https://docs.powertools.aws.dev/lambda/python/latest/core/metrics/#working-with-different-dimensions

one example approach could be:

Metrics.ClearDefaultDimensions();
Metrics.SetDefaultDimensions(new Dictionary<string, string> {
  { "ServiceName", context.FunctionName },
  { "SessionId",  input.SessionId ?? "Unset"  }
});

var nameSpace = "ns";
var service = "myservice";

Metrics.PushSingleMetric("Lambda Execute", 1, MetricUnit.Count, nameSpace: nameSpace,
    service: service, metricResolution: MetricResolution.High, defaultDimensions: new Dictionary<string, string> {
     { "ServiceName", context.FunctionName },
    { "SessionId",   input.SessionId ?? "Unset"  },
    { "Type", "Start" }});

Metrics.PushSingleMetric("Lambda Execute", 1, MetricUnit.Count, nameSpace: nameSpace,
            service: service, metricResolution: MetricResolution.High, defaultDimensions: new Dictionary<string, string> {
     { "ServiceName", context.FunctionName },
    { "SessionId",   input.SessionId ?? "Unset"  },
    { "Type", "End" }});

Lots of code repetition but that could be improved.

it will output two metrics with the dimensions with no warnings

{
  "_aws": {
    "Timestamp": 1728510818704,
    "CloudWatchMetrics": [
      {
        "Namespace": "ns",
        "Metrics": [
          {
            "Name": "Lambda Execute",
            "Unit": "Count",
            "StorageResolution": 1
          }
        ],
        "Dimensions": [
          [
            "ServiceName",
            "SessionId",
            "Type",
            "Service"
          ]
        ]
      }
    ]
  },
  "ServiceName": "dotnet8-lambda",
  "SessionId": "Unset",
  "Type": "Start",
  "Service": "myservice",
  "Lambda Execute": 1
}
{
  "_aws": {
    "Timestamp": 1728510818705,
    "CloudWatchMetrics": [
      {
        "Namespace": "ns",
        "Metrics": [
          {
            "Name": "Lambda Execute",
            "Unit": "Count",
            "StorageResolution": 1
          }
        ],
        "Dimensions": [
          [
            "ServiceName",
            "SessionId",
            "Type",
            "Service"
          ]
        ]
      }
    ]
  },
  "ServiceName": "dotnet8-lambda",
  "SessionId": "Unset",
  "Type": "End",
  "Service": "myservice",
  "Lambda Execute": 1
}
Euclidite commented 1 week ago

Alright that's fine I can use PushSingleMetric instead - I just tested it out and it works as expected.

It does feel a bit odd however that I can't change a dimension value - it almost feels like AddMetric should return some sort of Metric object - a nice to have improvement.

As for this ticket - thank you for the quick turn-around!

Euclidite commented 1 week ago

Ack spoke a little too soon - I just noticed PushSingleMetric always adds the Service dimension (with a value of service_undefined) even if I call ClearDefaultDimensions beforehand. Is there any way to avoid that?

I'm trying to coordinate metric creation with a few other teams, so having this extra dimension would require others to add it as well (and it would go unused).

For reference, this is what's being done using the Python powertools library:

metrics.add_metric(name='Lambda Execute', unit='Count', value=1, resolution=1)
metrics.add_dimension(name='SessionId', value=session_id)
metrics.add_dimension(name='Type', value='Start')
metrics.add_dimension(name='ServiceName', value=context.function_name)
metrics.flush_metrics()

And it's not adding the service dimension - I'm assuming the library checks to see if a value was specified for service first.

hjgraca commented 1 week ago

@Euclidite thanks once again, you are correct, Service should not be added as a default dimension. Working on another fix.

Euclidite commented 3 days ago

@hjgraca Any word on when it would be possible to get a version where Service is not added as a default? Even a, alpha release would be appreciated 🙂

hjgraca commented 3 days ago

@Euclidite I will probably have an alpha version for you to test tomorrow. The main reason for the delay is that these issues you reported will probably lead to breaking changes and release a v2 of Metrics, and if that is the case we will take the opportunity to make the api similar to python on the examples you shared. So to sum, I will probably release a 2.0-rc version tomorrow so you can use it, but the final version might take a week or two, but at least you will have the new changes, will that work for you?

Euclidite commented 3 days ago

Completely understand and that works for me - I'll be able to test out & use the 2.0-rc version tomorrow then. Thank you!