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

chore: FIx Metrics - Add missing array to wrap metrics array #666

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 50 additions & 28 deletions libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
*
* http://aws.amazon.com/apache2.0
*
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
Expand All @@ -31,7 +31,7 @@
/// The instance
/// </summary>
private static IMetrics _instance;

/// <summary>
/// The context
/// </summary>
Expand All @@ -46,7 +46,7 @@
/// If true, Powertools for AWS Lambda (.NET) will throw an exception on empty metrics when trying to flush
/// </summary>
private readonly bool _raiseOnEmptyMetrics;

/// <summary>
/// The capture cold start enabled
/// </summary>
Expand Down Expand Up @@ -76,9 +76,8 @@
_raiseOnEmptyMetrics = raiseOnEmptyMetrics;
_captureColdStartEnabled = captureColdStartEnabled;
_context = InitializeContext(nameSpace, service, null);

_powertoolsConfigurations.SetExecutionEnvironment(this);

}

/// <summary>
Expand All @@ -96,18 +95,20 @@
{
if (string.IsNullOrWhiteSpace(key))
throw new ArgumentNullException(
nameof(key), "'AddMetric' method requires a valid metrics key. 'Null' or empty values are not allowed.");

if (value < 0) {
nameof(key),
"'AddMetric' method requires a valid metrics key. 'Null' or empty values are not allowed.");

Check warning on line 99 in libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs

View check run for this annotation

Codecov / codecov/patch

libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs#L98-L99

Added lines #L98 - L99 were not covered by tests

if (value < 0)
{
throw new ArgumentException(
"'AddMetric' method requires a valid metrics value. Value must be >= 0.", nameof(value));
}

lock (_lockObj)
{
var metrics = _context.GetMetrics();
if (metrics.Count > 0 &&

if (metrics.Count > 0 &&
(metrics.Count == PowertoolsConfigurations.MaxMetrics ||
metrics.FirstOrDefault(x => x.Name == key)
?.Values.Count == PowertoolsConfigurations.MaxMetrics))
Expand All @@ -134,7 +135,14 @@
/// <returns>Namespace identifier</returns>
string IMetrics.GetNamespace()
{
return _context.GetNamespace();
try
{
return _context.GetNamespace();
}
catch
{
return null;
}
}

/// <summary>
Expand All @@ -143,7 +151,14 @@
/// <returns>System.String.</returns>
string IMetrics.GetService()
{
return _context.GetService();
try
{
return _context.GetService();
}
catch
{
return null;

Check warning on line 160 in libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs

View check run for this annotation

Codecov / codecov/patch

libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs#L158-L160

Added lines #L158 - L160 were not covered by tests
}
}

/// <summary>
Expand Down Expand Up @@ -226,7 +241,7 @@
{
if (!_captureColdStartEnabled)
Console.WriteLine(
"##WARNING## Metrics and Metadata have not been specified. No data will be sent to Cloudwatch Metrics.");
"##User-WARNING## No application metrics to publish. The cold-start metric may be published if enabled. If application metrics should never be empty, consider using 'RaiseOnEmptyMetrics = true'");
}
}

Expand Down Expand Up @@ -283,7 +298,7 @@
Dispose(true);
GC.SuppressFinalize(this);
}

/// <summary>
///
/// </summary>
Expand Down Expand Up @@ -356,7 +371,7 @@
{
_instance.SetDefaultDimensions(defaultDimensions);
}

/// <summary>
/// Clears both default dimensions and dimensions lists
/// </summary>
Expand Down Expand Up @@ -389,14 +404,14 @@
/// <param name="defaultDimensions">Default dimensions list</param>
/// <param name="metricResolution">Metrics resolution</param>
public static void PushSingleMetric(string metricName, double value, MetricUnit unit, string nameSpace = null,
string service = null, Dictionary<string, string> defaultDimensions = null, MetricResolution metricResolution = MetricResolution.Default)
string service = null, Dictionary<string, string> defaultDimensions = null,
MetricResolution metricResolution = MetricResolution.Default)
{
_instance.PushSingleMetric(metricName, value, unit, nameSpace, service, defaultDimensions, metricResolution);
}

/// <summary>
/// Sets global namespace, service name and default dimensions list. Service name is automatically added as a default
/// dimension
/// Sets global namespace, service name and default dimensions list.
/// </summary>
/// <param name="nameSpace">Metrics namespace</param>
/// <param name="service">Service Name</param>
Expand All @@ -406,19 +421,26 @@
Dictionary<string, string> defaultDimensions)
{
var context = new MetricsContext();
var defaultDimensionsList = DictionaryToList(defaultDimensions);

context.SetNamespace(!string.IsNullOrWhiteSpace(nameSpace)
? nameSpace
: _powertoolsConfigurations.MetricsNamespace);
: _instance.GetNamespace() ?? _powertoolsConfigurations.MetricsNamespace);

context.SetService(!string.IsNullOrWhiteSpace(service)
// this needs to check if service is set through code or env variables
// the default value service_undefined has to be ignored and return null so it is not added as default
// TODO: Check if there is a way to get the default dimensions and if it makes sense
var parsedService = !string.IsNullOrWhiteSpace(service)
? service
: _powertoolsConfigurations.Service);

var defaultDimensionsList = DictionaryToList(defaultDimensions);
: _powertoolsConfigurations.Service == "service_undefined"
? null
: _powertoolsConfigurations.Service;

// Add service as a default dimension
defaultDimensionsList.Add(new DimensionSet("Service", context.GetService()));
if (parsedService != null)
{
context.SetService(parsedService);
defaultDimensionsList.Add(new DimensionSet("Service", context.GetService()));
}

context.SetDefaultDimensions(defaultDimensionsList);

Expand Down Expand Up @@ -447,4 +469,4 @@
{
_instance = null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ namespace AWS.Lambda.Powertools.Metrics;
/// </listheader>
/// <item>
/// <term>Service</term>
/// <description>string, service name is used for metric dimension across all metrics, by default service_undefined</description>
/// <description>string, service name is used for metric dimension</description>
/// </item>
/// <item>
/// <term>Namespace</term>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ private MetricDirective(string nameSpace, List<MetricDefinition> metrics, List<D
/// </summary>
/// <value>All dimension keys.</value>
[JsonPropertyName("Dimensions")]
public List<string> AllDimensionKeys
public List<List<string>> AllDimensionKeys
{
get
{
Expand All @@ -123,7 +123,8 @@ public List<string> AllDimensionKeys

if (defaultKeys.Count == 0) defaultKeys = new List<string>();

return defaultKeys;
// Wrap the list of strings in another list
return new List<List<string>> { defaultKeys };
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public void WhenClearAllDimensions_NoDimensionsInOutput()
var metricsOutput = consoleOut.ToString();

// Assert
Assert.Contains("{\"Namespace\":\"dotnet-powertools-test\",\"Metrics\":[{\"Name\":\"Metric Name\",\"Unit\":\"Count\"}],\"Dimensions\":[]", metricsOutput);
Assert.Contains("{\"Namespace\":\"dotnet-powertools-test\",\"Metrics\":[{\"Name\":\"Metric Name\",\"Unit\":\"Count\"}],\"Dimensions\":[[]]", metricsOutput);

// Reset
MetricsAspect.ResetForTest();
Expand Down
Loading
Loading