Skip to content

Conversation

@hjgraca
Copy link
Contributor

@hjgraca hjgraca commented Oct 8, 2024

Please provide the issue number

Issue number: #662

Summary

Changes

Dimensions is now a List and not a List<List>
Added tests

User experience

The following example:

[Metrics(Namespace = "dotnet-powertools-test", Service = "testService")]
  public void AddMultipleDimensions()
  {
      Metrics.SetDefaultDimensions(new Dictionary<string, string> {
          { "Default", "Initial" }
      });
      Metrics.PushSingleMetric("Lambda Execute", 1, MetricUnit.Count, metricResolution: MetricResolution.High, nameSpace: "ns1",
          defaultDimensions: new Dictionary<string, string> {
              { "Type", "Start" }
          });
      
      Metrics.PushSingleMetric("Lambda Execute", 1, MetricUnit.Count, metricResolution: MetricResolution.High,  nameSpace: "ns2",
          defaultDimensions: new Dictionary<string, string> {
              { "Type", "Start" },
              { "SessionId", "Unset" }
          });
      Metrics.AddMetric("Lambda Execute", 1, MetricUnit.Count, MetricResolution.High);
      Metrics.AddDimension("SessionId", "MySessionId");
      Metrics.AddDimension("Type", "Start");
  }

Now outputs the correct value for Dimensions

{
    "_aws": {
        "Timestamp": 1728403078569,
        "CloudWatchMetrics": [
            {
                "Namespace": "ns1",
                "Metrics": [
                    {
                        "Name": "Lambda Execute",
                        "Unit": "Count",
                        "StorageResolution": 1
                    }
                ],
                "Dimensions": [
                    "Type",
                    "Service"
                ]
            }
        ]
    },
    "Type": "Start",
    "Service": "service_undefined",
    "Lambda Execute": 1
}
          
{
    "_aws": {
        "Timestamp": 1728403078575,
        "CloudWatchMetrics": [
            {
                "Namespace": "ns2",
                "Metrics": [
                    {
                        "Name": "Lambda Execute",
                        "Unit": "Count",
                        "StorageResolution": 1
                    }
                ],
                "Dimensions": [
                    "Type",
                    "SessionId",
                    "Service"
                ]
            }
        ]
    },
    "Type": "Start",
    "SessionId": "Unset",
    "Service": "service_undefined",
    "Lambda Execute": 1
}

{
    "_aws": {
        "Timestamp": 1728403078575,
        "CloudWatchMetrics": [
            {
                "Namespace": "dotnet-powertools-test",
                "Metrics": [
                    {
                        "Name": "Lambda Execute",
                        "Unit": "Count",
                        "StorageResolution": 1
                    }
                ],
                "Dimensions": [
                    "Service",
                    "Default",
                    "SessionId",
                    "Type"
                ]
            }
        ]
    },
    "Service": "testService",
    "Default": "Initial",
    "SessionId": "MySessionId",
    "Type": "Start",
    "Lambda Execute": 1
}

Note

Clearing Dimensions will now result in a empty array [] instead of [[]]
For example:

{
  "_aws": {
    "Timestamp": 1728408583773,
    "CloudWatchMetrics": [
      {
        "Namespace": "dotnet-powertools-test",
        "Metrics": [
          {
            "Name": "Metric Name",
            "Unit": "Count"
          }
        ],
        "Dimensions": []
      }
    ]
  },
  "Metric Name": 1
}

Checklist

Please leave checklist items unchecked if they do not apply to your change.

Is this a breaking change?

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@auto-assign auto-assign bot requested review from amirkaws and sliedig October 8, 2024 16:03
@boring-cyborg boring-cyborg bot added area/metrics Core metrics utility tests labels Oct 8, 2024
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 8, 2024
@github-actions github-actions bot added the internal Maintenance changes label Oct 8, 2024
@codecov
Copy link

codecov bot commented Oct 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.43%. Comparing base (a979415) to head (f5dad6c).
Report is 4 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #664      +/-   ##
===========================================
+ Coverage    73.40%   73.43%   +0.03%     
===========================================
  Files          196      196              
  Lines         8027     8027              
  Branches       857      857              
===========================================
+ Hits          5892     5895       +3     
+ Misses        1849     1846       -3     
  Partials       286      286              
Flag Coverage Δ
unittests 73.43% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 9, 2024

@hjgraca hjgraca merged commit 5eb8338 into aws-powertools:develop Oct 9, 2024
8 checks passed
@hjgraca hjgraca deleted the fix(metrics)-multiple-dimension-array branch October 9, 2024 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/metrics Core metrics utility internal Maintenance changes size/M Denotes a PR that changes 30-99 lines, ignoring generated files. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants