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

Bug: Unable to push metrics with multiple dimensions #662

Open
Euclidite opened this issue Oct 8, 2024 · 19 comments
Open

Bug: Unable to push metrics with multiple dimensions #662

Euclidite opened this issue Oct 8, 2024 · 19 comments
Assignees
Labels
area/metrics Core metrics utility bug Unexpected, reproducible and unintended software behaviour pending-release Fix or implementation already in dev waiting to be released v2 v2 release

Comments

@Euclidite
Copy link

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

@Euclidite Euclidite added bug Unexpected, reproducible and unintended software behaviour triage Pending triage from maintainers labels Oct 8, 2024
Copy link

boring-cyborg bot commented Oct 8, 2024

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 hjgraca self-assigned this Oct 8, 2024
@hjgraca hjgraca added area/metrics Core metrics utility and removed triage Pending triage from maintainers labels Oct 8, 2024
@hjgraca hjgraca moved this to 📋 Backlog in Powertools for AWS Lambda (.NET) Oct 8, 2024
@hjgraca
Copy link
Contributor

hjgraca commented Oct 8, 2024

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

@dreamorosi dreamorosi moved this from 📋 Backlog to 🏗 In progress in Powertools for AWS Lambda (.NET) Oct 8, 2024
@leandrodamascena
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

hjgraca commented Oct 8, 2024

@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 hjgraca moved this from 🏗 In progress to ✅ Done in Powertools for AWS Lambda (.NET) Oct 9, 2024
@github-actions github-actions bot added the pending-release Fix or implementation already in dev waiting to be released label Oct 9, 2024
@hjgraca
Copy link
Contributor

hjgraca commented Oct 9, 2024

@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
Copy link
Author

Euclidite commented Oct 9, 2024

@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
Copy link
Contributor

hjgraca commented Oct 9, 2024

@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
Copy link
Contributor

hjgraca commented Oct 9, 2024

@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
Copy link
Author

@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
Copy link
Contributor

hjgraca commented Oct 9, 2024

@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
Copy link
Author

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
Copy link
Author

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
Copy link
Contributor

hjgraca commented Oct 10, 2024

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

@Euclidite
Copy link
Author

@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
Copy link
Contributor

hjgraca commented Oct 15, 2024

@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
Copy link
Author

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

@hjgraca
Copy link
Contributor

hjgraca commented Oct 21, 2024

@Euclidite sorry for the delay, version 2.0.0-alpha1 now available.
Please let us know how your tests go.

@hjgraca hjgraca moved this from ✅ Done to 🏗 In progress in Powertools for AWS Lambda (.NET) Oct 21, 2024
@hjgraca
Copy link
Contributor

hjgraca commented Oct 24, 2024

@Euclidite where you able to test this new version?

@hjgraca hjgraca added the v2 v2 release label Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics Core metrics utility bug Unexpected, reproducible and unintended software behaviour pending-release Fix or implementation already in dev waiting to be released v2 v2 release
Projects
Status: 🏗 In progress
Development

No branches or pull requests

3 participants