Skip to content

Commit dcede7f

Browse files
hjgracahjgraca
committed
Revert "Prevent default service to be added to PushSingleMetric"
This reverts commit 31dad3b.
1 parent 5da432c commit dcede7f

File tree

4 files changed

+45
-110
lines changed

4 files changed

+45
-110
lines changed

‎libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs

Lines changed: 28 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
/*
22
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3-
*
3+
*
44
* Licensed under the Apache License, Version 2.0 (the "License").
55
* You may not use this file except in compliance with the License.
66
* A copy of the License is located at
7-
*
7+
*
88
* http://aws.amazon.com/apache2.0
9-
*
9+
*
1010
* or in the "license" file accompanying this file. This file is distributed
1111
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
1212
* express or implied. See the License for the specific language governing
@@ -31,7 +31,7 @@ public class Metrics : IMetrics, IDisposable
3131
/// The instance
3232
/// </summary>
3333
private static IMetrics _instance;
34-
34+
3535
/// <summary>
3636
/// The context
3737
/// </summary>
@@ -46,7 +46,7 @@ public class Metrics : IMetrics, IDisposable
4646
/// If true, Powertools for AWS Lambda (.NET) will throw an exception on empty metrics when trying to flush
4747
/// </summary>
4848
private readonly bool _raiseOnEmptyMetrics;
49-
49+
5050
/// <summary>
5151
/// The capture cold start enabled
5252
/// </summary>
@@ -76,8 +76,9 @@ internal Metrics(IPowertoolsConfigurations powertoolsConfigurations, string name
7676
_raiseOnEmptyMetrics = raiseOnEmptyMetrics;
7777
_captureColdStartEnabled = captureColdStartEnabled;
7878
_context = InitializeContext(nameSpace, service, null);
79-
79+
8080
_powertoolsConfigurations.SetExecutionEnvironment(this);
81+
8182
}
8283

8384
/// <summary>
@@ -95,20 +96,18 @@ void IMetrics.AddMetric(string key, double value, MetricUnit unit, MetricResolut
9596
{
9697
if (string.IsNullOrWhiteSpace(key))
9798
throw new ArgumentNullException(
98-
nameof(key),
99-
"'AddMetric' method requires a valid metrics key. 'Null' or empty values are not allowed.");
100-
101-
if (value < 0)
102-
{
99+
nameof(key), "'AddMetric' method requires a valid metrics key. 'Null' or empty values are not allowed.");
100+
101+
if (value < 0) {
103102
throw new ArgumentException(
104103
"'AddMetric' method requires a valid metrics value. Value must be >= 0.", nameof(value));
105104
}
106105

107106
lock (_lockObj)
108107
{
109108
var metrics = _context.GetMetrics();
110-
111-
if (metrics.Count > 0 &&
109+
110+
if (metrics.Count > 0 &&
112111
(metrics.Count == PowertoolsConfigurations.MaxMetrics ||
113112
metrics.FirstOrDefault(x => x.Name == key)
114113
?.Values.Count == PowertoolsConfigurations.MaxMetrics))
@@ -135,14 +134,7 @@ void IMetrics.SetNamespace(string nameSpace)
135134
/// <returns>Namespace identifier</returns>
136135
string IMetrics.GetNamespace()
137136
{
138-
try
139-
{
140-
return _context.GetNamespace();
141-
}
142-
catch
143-
{
144-
return null;
145-
}
137+
return _context.GetNamespace();
146138
}
147139

148140
/// <summary>
@@ -151,14 +143,7 @@ string IMetrics.GetNamespace()
151143
/// <returns>System.String.</returns>
152144
string IMetrics.GetService()
153145
{
154-
try
155-
{
156-
return _context.GetService();
157-
}
158-
catch
159-
{
160-
return null;
161-
}
146+
return _context.GetService();
162147
}
163148

164149
/// <summary>
@@ -241,7 +226,7 @@ void IMetrics.Flush(bool metricsOverflow)
241226
{
242227
if (!_captureColdStartEnabled)
243228
Console.WriteLine(
244-
"##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'");
229+
"##WARNING## Metrics and Metadata have not been specified. No data will be sent to Cloudwatch Metrics.");
245230
}
246231
}
247232

@@ -298,7 +283,7 @@ public void Dispose()
298283
Dispose(true);
299284
GC.SuppressFinalize(this);
300285
}
301-
286+
302287
/// <summary>
303288
///
304289
/// </summary>
@@ -371,7 +356,7 @@ public static void SetDefaultDimensions(Dictionary<string, string> defaultDimens
371356
{
372357
_instance.SetDefaultDimensions(defaultDimensions);
373358
}
374-
359+
375360
/// <summary>
376361
/// Clears both default dimensions and dimensions lists
377362
/// </summary>
@@ -404,14 +389,14 @@ private void Flush(MetricsContext context)
404389
/// <param name="defaultDimensions">Default dimensions list</param>
405390
/// <param name="metricResolution">Metrics resolution</param>
406391
public static void PushSingleMetric(string metricName, double value, MetricUnit unit, string nameSpace = null,
407-
string service = null, Dictionary<string, string> defaultDimensions = null,
408-
MetricResolution metricResolution = MetricResolution.Default)
392+
string service = null, Dictionary<string, string> defaultDimensions = null, MetricResolution metricResolution = MetricResolution.Default)
409393
{
410394
_instance.PushSingleMetric(metricName, value, unit, nameSpace, service, defaultDimensions, metricResolution);
411395
}
412396

413397
/// <summary>
414-
/// Sets global namespace, service name and default dimensions list.
398+
/// Sets global namespace, service name and default dimensions list. Service name is automatically added as a default
399+
/// dimension
415400
/// </summary>
416401
/// <param name="nameSpace">Metrics namespace</param>
417402
/// <param name="service">Service Name</param>
@@ -421,26 +406,19 @@ private MetricsContext InitializeContext(string nameSpace, string service,
421406
Dictionary<string, string> defaultDimensions)
422407
{
423408
var context = new MetricsContext();
424-
var defaultDimensionsList = DictionaryToList(defaultDimensions);
425409

426410
context.SetNamespace(!string.IsNullOrWhiteSpace(nameSpace)
427411
? nameSpace
428-
: _instance.GetNamespace() ?? _powertoolsConfigurations.MetricsNamespace);
412+
: _powertoolsConfigurations.MetricsNamespace);
429413

430-
// this needs to check if service is set through code or env variables
431-
// the default value service_undefined has to be ignored and return null so it is not added as default
432-
// TODO: Check if there is a way to get the default dimensions and if it makes sense
433-
var parsedService = !string.IsNullOrWhiteSpace(service)
414+
context.SetService(!string.IsNullOrWhiteSpace(service)
434415
? service
435-
: _powertoolsConfigurations.Service == "service_undefined"
436-
? null
437-
: _powertoolsConfigurations.Service;
416+
: _powertoolsConfigurations.Service);
438417

439-
if (parsedService != null)
440-
{
441-
context.SetService(parsedService);
442-
defaultDimensionsList.Add(new DimensionSet("Service", context.GetService()));
443-
}
418+
var defaultDimensionsList = DictionaryToList(defaultDimensions);
419+
420+
// Add service as a default dimension
421+
defaultDimensionsList.Add(new DimensionSet("Service", context.GetService()));
444422

445423
context.SetDefaultDimensions(defaultDimensionsList);
446424

@@ -469,4 +447,4 @@ internal static void ResetForTest()
469447
{
470448
_instance = null;
471449
}
472-
}
450+
}

‎libraries/src/AWS.Lambda.Powertools.Metrics/MetricsAttribute.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ namespace AWS.Lambda.Powertools.Metrics;
6666
/// </listheader>
6767
/// <item>
6868
/// <term>Service</term>
69-
/// <description>string, service name is used for metric dimension</description>
69+
/// <description>string, service name is used for metric dimension across all metrics, by default service_undefined</description>
7070
/// </item>
7171
/// <item>
7272
/// <term>Namespace</term>

‎libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/EMFValidationTests.cs

Lines changed: 4 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -157,39 +157,11 @@ public void When_Multiple_DimensionsAreAdded_MustExistAsMembers()
157157
var metricsOutput = _consoleOut.ToString();
158158

159159
// Assert
160-
Assert.Contains("\"CloudWatchMetrics\":[{\"Namespace\":\"dotnet-powertools-test\",\"Metrics\":[{\"Name\":\"ColdStart\",\"Unit\":\"Count\"}],\"Dimensions\":[[\"Service\"]]}]},\"Service\":\"ServiceName\",\"ColdStart\":1}", metricsOutput);
161-
Assert.Contains("\"CloudWatchMetrics\":[{\"Namespace\":\"dotnet-powertools-test\",\"Metrics\":[{\"Name\":\"SingleMetric1\",\"Unit\":\"Count\",\"StorageResolution\":1}],\"Dimensions\":[[\"Default1\"]]}]},\"Default1\":\"SingleMetric1\",\"SingleMetric1\":1}", metricsOutput);
162-
Assert.Contains("\"CloudWatchMetrics\":[{\"Namespace\":\"ns2\",\"Metrics\":[{\"Name\":\"SingleMetric2\",\"Unit\":\"Count\",\"StorageResolution\":1}],\"Dimensions\":[[\"Default1\",\"Default2\"]]}]},\"Default1\":\"SingleMetric2\",\"Default2\":\"SingleMetric2\",\"SingleMetric2\":1}", metricsOutput);
163-
Assert.Contains("\"CloudWatchMetrics\":[{\"Namespace\":\"dotnet-powertools-test\",\"Metrics\":[{\"Name\":\"AddMetric\",\"Unit\":\"Count\",\"StorageResolution\":1},{\"Name\":\"AddMetric2\",\"Unit\":\"Count\",\"StorageResolution\":1}],\"Dimensions\":[[\"Service\"]]}]},\"Service\":\"ServiceName\",\"AddMetric\":1,\"AddMetric2\":1}", metricsOutput);
164-
}
165-
166-
[Trait("Category", "SchemaValidation")]
167-
[Fact]
168-
public void When_PushSingleMetric_With_Namespace()
169-
{
170-
// Act
171-
_handler.PushSingleMetricWithNamespace();
172-
173-
var metricsOutput = _consoleOut.ToString();
174-
175-
// Assert
176-
Assert.Contains("\"CloudWatchMetrics\":[{\"Namespace\":\"ExampleApplication\",\"Metrics\":[{\"Name\":\"SingleMetric\",\"Unit\":\"Count\",\"StorageResolution\":1}],\"Dimensions\":[[\"Default\"]]}]},\"Default\":\"SingleMetric\",\"SingleMetric\":1}", metricsOutput);
177-
}
178-
179-
[Trait("Category", "SchemaValidation")]
180-
[Fact]
181-
public void When_PushSingleMetric_With_Env_Namespace()
182-
{
183-
// Arrange
184-
Environment.SetEnvironmentVariable("POWERTOOLS_METRICS_NAMESPACE", "EnvNamespace");
160+
Assert.Contains("\"CloudWatchMetrics\":[{\"Namespace\":\"ns1\",\"Metrics\":[{\"Name\":\"Lambda Execute\",\"Unit\":\"Count\",\"StorageResolution\":1}],\"Dimensions\":[[\"Type\",\"Service\"]]}]},\"Type\":\"Start\",\"Service\":\"service_undefined\",\"Lambda Execute\":1}", metricsOutput);
185161

186-
// Act
187-
_handler.PushSingleMetricWithEnvNamespace();
188-
189-
var metricsOutput = _consoleOut.ToString();
190-
191-
// Assert
192-
Assert.Contains("\"CloudWatchMetrics\":[{\"Namespace\":\"EnvNamespace\",\"Metrics\":[{\"Name\":\"SingleMetric\",\"Unit\":\"Count\",\"StorageResolution\":1}],\"Dimensions\":[[\"Default\"]]}]},\"Default\":\"SingleMetric\",\"SingleMetric\":1}", metricsOutput);
162+
Assert.Contains("\"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}", metricsOutput);
163+
164+
Assert.Contains("\"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}", metricsOutput);
193165
}
194166

195167
[Trait("Category", "MetricsImplementation")]
@@ -394,7 +366,6 @@ public void Dispose()
394366
{
395367
// need to reset instance after each test
396368
MetricsAspect.ResetForTest();
397-
Environment.SetEnvironmentVariable("POWERTOOLS_METRICS_NAMESPACE", null);
398369
}
399370
}
400371
}

‎libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/FunctionHandler.cs

Lines changed: 12 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -39,39 +39,25 @@ public void AddDimensions()
3939
Metrics.AddMetric("TestMetric", 1, MetricUnit.Count);
4040
}
4141

42-
[Metrics(Namespace = "dotnet-powertools-test", Service = "ServiceName", CaptureColdStart = true)]
42+
[Metrics(Namespace = "dotnet-powertools-test", Service = "testService")]
4343
public void AddMultipleDimensions()
4444
{
45-
Metrics.PushSingleMetric("SingleMetric1", 1, MetricUnit.Count, metricResolution: MetricResolution.High,
45+
Metrics.SetDefaultDimensions(new Dictionary<string, string> {
46+
{ "Default", "Initial" }
47+
});
48+
Metrics.PushSingleMetric("Lambda Execute", 1, MetricUnit.Count, metricResolution: MetricResolution.High, nameSpace: "ns1",
4649
defaultDimensions: new Dictionary<string, string> {
47-
{ "Default1", "SingleMetric1" }
50+
{ "Type", "Start" }
4851
});
4952

50-
Metrics.PushSingleMetric("SingleMetric2", 1, MetricUnit.Count, metricResolution: MetricResolution.High, nameSpace: "ns2",
51-
defaultDimensions: new Dictionary<string, string> {
52-
{ "Default1", "SingleMetric2" },
53-
{ "Default2", "SingleMetric2" }
54-
});
55-
Metrics.AddMetric("AddMetric", 1, MetricUnit.Count, MetricResolution.High);
56-
Metrics.AddMetric("AddMetric2", 1, MetricUnit.Count, MetricResolution.High);
57-
}
58-
59-
[Metrics(Namespace = "ExampleApplication")]
60-
public void PushSingleMetricWithNamespace()
61-
{
62-
Metrics.PushSingleMetric("SingleMetric", 1, MetricUnit.Count, metricResolution: MetricResolution.High,
63-
defaultDimensions: new Dictionary<string, string> {
64-
{ "Default", "SingleMetric" }
65-
});
66-
}
67-
68-
[Metrics]
69-
public void PushSingleMetricWithEnvNamespace()
70-
{
71-
Metrics.PushSingleMetric("SingleMetric", 1, MetricUnit.Count, metricResolution: MetricResolution.High,
53+
Metrics.PushSingleMetric("Lambda Execute", 1, MetricUnit.Count, metricResolution: MetricResolution.High, nameSpace: "ns2",
7254
defaultDimensions: new Dictionary<string, string> {
73-
{ "Default", "SingleMetric" }
55+
{ "Type", "Start" },
56+
{ "SessionId", "Unset" }
7457
});
58+
Metrics.AddMetric("Lambda Execute", 1, MetricUnit.Count, MetricResolution.High);
59+
Metrics.AddDimension("SessionId", "MySessionId");
60+
Metrics.AddDimension("Type", "Start");
7561
}
7662

7763
[Metrics(Namespace = "dotnet-powertools-test", Service = "testService")]

0 commit comments

Comments
 (0)