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

Multiple values for a single metric not working #165

Closed
Dunedan opened this issue Sep 16, 2020 · 7 comments
Closed

Multiple values for a single metric not working #165

Dunedan opened this issue Sep 16, 2020 · 7 comments
Assignees
Labels
feature-request feature request

Comments

@Dunedan
Copy link
Contributor

Dunedan commented Sep 16, 2020

I'm trying to log multiple values for a single metric using powertool's Metrics class.
However when doing so I only get the last added value flushed.

Here is a minimal example which triggers the problem:

#!/usr/bin/env python3

import json

from aws_lambda_powertools import Metrics
from aws_lambda_powertools.metrics import MetricUnit

metrics = Metrics(namespace="Test", service="Test")

@metrics.log_metrics
def handler(event, context):
    metrics.add_metric(name="TestMetric", unit=MetricUnit.Count, value=1)
    metrics.add_metric(name="TestMetric", unit=MetricUnit.Count, value=5)

handler(None, None)

The code above produces the following output (formatted for better readability):

{
    "_aws": {
        "Timestamp": 1600250139457,
        "CloudWatchMetrics": [
            {
                "Namespace": "Test",
                "Dimensions": [
                    [
                        "service"
                    ]
                ],
                "Metrics": [
                    {
                        "Name": "TestMetric",
                        "Unit": "Count"
                    }
                ]
            }
        ]
    },
    "service": "Test",
    "TestMetric": 5.0
}

What I'd expect instead would be the following:

{
    "_aws": {
        "Timestamp": 1600250139457,
        "CloudWatchMetrics": [
            {
                "Namespace": "Test",
                "Dimensions": [
                    [
                        "service"
                    ]
                ],
                "Metrics": [
                    {
                        "Name": "TestMetric",
                        "Unit": "Count"
                    }
                ]
            }
        ]
    },
    "service": "Test",
    "TestMetric": [1.0, 5.0]
}

I'm not sure if this is a bug or if I'm missing something from the documentation, but to me it currently looks that logging multiple values for a single metric isn't possible without flushing manually between adding them.

@Dunedan Dunedan added bug Something isn't working triage Pending triage from maintainers labels Sep 16, 2020
@heitorlessa
Copy link
Contributor

hey @Dunedan - This is not a bug but a known behaviour of Embedded Metric Format (EMF) feature we use for creating metrics async. It only supports one value per unique key name - If you add multiple values it'll override with the last one.

For that and other purposes, we created the with single_metric context manager so it'll create one metric and flush it automatically.

I thought of having another API for that would keep multiple values for the same metric, and then flush them at the end as a workaround to EMF specification - But not entirely sure on the value and UX yet. If you think this is a good idea, please feel free to write up a RFC to discuss more about it :)

@heitorlessa heitorlessa self-assigned this Sep 16, 2020
@heitorlessa heitorlessa added area/metrics feature-request feature request and removed bug Something isn't working triage Pending triage from maintainers labels Sep 16, 2020
@Dunedan
Copy link
Contributor Author

Dunedan commented Sep 16, 2020

Are you sure that this is a limitation of EMF?

The EMF specification suggests otherwise:

Metric targets MUST be either a numeric value or an array of numeric values.

Also aws-embedded-metrics-python implements it as I would expect it and I believe such records are properly handled by CloudWatch Metrics.

#!/usr/bin/env python3

import os

os.environ["AWS_LAMBDA_FUNCTION_NAME"] = "dummy"

from aws_embedded_metrics import metric_scope

@metric_scope
def handler(metrics):
    metrics.put_dimensions({"Foo": "Bar"})
    metrics.put_metric("TestMetric", 1, "Count")
    metrics.put_metric("TestMetric", 1, "Count")

handler()
{
    "LogGroup": "dummy",
    "ServiceName": "dummy",
    "ServiceType": "AWS::Lambda::Function",
    "Foo": "Bar",
    "executionEnvironment": "",
    "memorySize": "",
    "functionVersion": "",
    "logStreamId": "",
    "_aws": {
        "Timestamp": 1600271848937,
        "CloudWatchMetrics": [
            {
                "Dimensions": [
                    [
                        "LogGroup",
                        "ServiceName",
                        "ServiceType",
                        "Foo"
                    ]
                ],
                "Metrics": [
                    {
                        "Name": "TestMetric",
                        "Unit": "Count"
                    }
                ],
                "Namespace": "aws-embedded-metrics"
            }
        ]
    },
    "TestMetric": [
        1,
        1
    ]
}

@heitorlessa
Copy link
Contributor

heitorlessa commented Sep 16, 2020 via email

@heitorlessa heitorlessa added bug Something isn't working feature-request feature request and removed feature-request feature request bug Something isn't working labels Sep 16, 2020
@heitorlessa
Copy link
Contributor

@Dunedan just confirmed with the CloudWatch team - This was added recently, it wasn't in the original implementation.

Added the feature request label, and we'll implement this to be available in the next release 1.6.0 this month.

Thanks for flagging it, much much appreciated

@heitorlessa heitorlessa added the pending-release Fix or implementation already in dev waiting to be released label Sep 18, 2020
@heitorlessa
Copy link
Contributor

hey @Dunedan - @cakepietoast just finished implementing it, and as I mentioned before, it'll be available in the next release.

Thanks for flagging this and letting us know about the EMF update, much appreciated!

@mwarkentin
Copy link

This can be closed now :)

@to-mc
Copy link
Contributor

to-mc commented Sep 22, 2020

Fixed in 1.6.0

@to-mc to-mc closed this as completed Sep 22, 2020
@heitorlessa heitorlessa removed the pending-release Fix or implementation already in dev waiting to be released label Oct 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request feature request
Projects
None yet
Development

No branches or pull requests

4 participants