awslabs / aws-embedded-metrics-dotnet Goto Github PK

Amazon CloudWatch Embedded Metric Format Client Library

License: Apache License 2.0

Dockerfile 0.54% Shell 2.92% C# 96.54%

aws-embedded-metrics-dotnet's Issues

Add validation for metrics, dimensions and namespace

Related issues:

Instead of silently dropping non-compliant characters, the client will now throw an error if an invalid dimension is found according to specification here.

Improve logging

Several classes do not have a logger passed to them

Memory leak with specific configuration

When using the example configuration as stated in the README, the library fails to send all its logs to the CloudWatch agent.

This has caused the canary's heap usage to increase until crashing and is therefore blocking the release of v2.0.0.

Support resetting custom dimensions and related enhancements

Related issue:

This is the equivalent enhancements related to resetting custom dimensions in the Dotnet library. Similar APIs and features should be implemented in all libraries for consistency. Specifially, the enhancements include:

Provide a ResetDimensions(bool useDefault) API for resetting dimensions
Provide a feature flag to control whether Flush() should preserve custom dimensions after each execution
Provide a setDimensions(bool useDefault, params DimensionSet[] dimensionSets) API to control the behavior of default dimensions while setting new dimensions.

ASP.NET Helpers

Is there any way to ignore logging specific controller actions or status codes? If you try to use this on a server behind an NLB, the TCP health checks just create a bunch of useless 404 responses that all get logged.

Setting dimensions as null breaks metrics logger

I had this code:

class MyMetricsLogger
{
    private readonly MetricsLogger _metricsLogger = new MetricsLogger();
    public void LogMetric(string key, double value, DimensionSet dimensionSet = null)
    {
        _metricsLogger.SetDimensions(dimensionSet);
        _metricsLogger.PutMetric(key, value);
        _metricsLogger.Flush();
    }
}

Invoking MetricsLogger.SetDimensions with null will break the MetricsLogger. When it next tries to log any metrics there is a NullReferenceException thrown from

   at Amazon.CloudWatch.EMF.Model.MetricDirective.<>c.<get_AllDimensionKeys>b__20_0(DimensionSet s)

There is a simple workaround, avoid using null when calling SetDimensions.

if (dimensionSet != null)
{
    _metricsLogger.SetDimensions(dimensionSet);
}
else
{
    _metricsLogger.SetDimensions();
}

It is a weakness of the package design for me to have to do this, the null check should be handled gracefully within the package itself.

EC2/ECS Metadata Endpoint checks block Lambda function

We are not short-circuiting environment detection on env var checks. The impact of this is that the flush time on Lambda functions will be extended by the network I/O which will block function response and drive up latency.

Add CI/CD

Transitive dependency on Json.NET makes library incompatible for AOT

Hi, as far as I can tell there is a transitive dependency on Newtonsoft.JSON in the Amazon.Cloudwatch.EMF lib.

Here is the output from dotnet-depends:

This would make any lambda that uses this library incompatible with AOT compilation if I understand correctly. Are there any plans to address this?

The EMF Web Extension Methods Are Not in Nuget Package

The advertised extension methods in the EMF.Web namespace are not part of the Amazon.CloudWatch.EMF nuget package (since it's a different project and I don't see it published). This means the example with .UseEmfMiddleware(); and services.AddEmf(); don't work. It's simple enough to copy the code into your project, but wasn't expecting to need to do that (and didn't specify in the readme). In the example code, it all works because you're still in the EMF namespace, but consumers of the library can't access it.

PutDimensions Doesn't Work

In an ASP.NET web application, I'm using the EMF middleware. I want to add dimensions besides Action, Status, and controller, so I'm creating a DimensionSet and calling PutDimensions.

DimensionSet dimensionSet = new DimensionSet();
dimensionSet.AddDimension("AZ-ID", Utils.GetAZId());
dimensionSet.AddDimension("Region", Utils.GetRegion());
dimensionSet.AddDimension("InstanceId", Utils.GetInstanceId());

metrics.PutDimensions(dimensionSet);

This doesn't work. In fact, any attempt to use PutDimensions or PutProperty inside a controller action results in nothing being logged. If I don't call these methods on the IMetricsLogger object (the one that's injected into the constructor), then logs are produced when a controller action is called. Finally, on the IMetricsLogger object that's injected into the controller, using SetNamespace in the constructor isn't honored for the log object created by the middleware, but calling a method like PutProperty is.

I believe you should be able to set the namespace (not always use the default of the ServiceName) and add dimensions and properties inside a controller action while using the EMF middleware.

Metric Logger behaving very inconsistently with custom MetricsLoggerWrapper class

So after looking at the docs, we decided to write our own metrics logger since we want to avoid metric logger disposal on each flush to Cloudwatch and buffer as much of the metric data as possible for better performance. Also since we can have different types and members per metric we decided to create multiple metric loggers each tracking metrics uniquely, buffering and sending them to cloud watch after scheduled flushes while avoiding overrites at the same time. Attaching some of the screen shots of the implementation...

The _loggers holds and identifies each logger uniquely per member and type.

The flush gets invoked by our own implementation. Now the custom implementation is as follows...

private class MetricsLoggerWrapper : IMetricsLogger
    {
        private readonly MetricsLogger _logger;
        private int _metricsCount;

        public MetricsLoggerWrapper(IEnvironment environment, string @namespace, string type, string? member)
        {
            _logger = new MetricsLogger(environment, NullLoggerFactory.Instance);
            _logger.SetNamespace(@namespace);

            var dimensionSet = new DimensionSet();
            if (!string.IsNullOrEmpty(type))
            {
                dimensionSet.AddDimension("Type", type);
            }

            if (!string.IsNullOrEmpty(member))
            {
                dimensionSet.AddDimension("Member", member);
            }

            _logger.SetDimensions(dimensionSet);
        }

        public void Flush()
        {
            if (_metricsCount > 0)
            {
                SetTimestamp(DateTime.UtcNow);
                _logger.Flush();
                _metricsCount = 0;
            }
        }

        public Task ShutdownAsync() => _logger.ShutdownAsync();

        public MetricsLogger PutProperty(string key, object value) => _logger.PutProperty(key, value);

        public MetricsLogger PutDimensions(DimensionSet dimensions) => _logger.PutDimensions(dimensions);

        public MetricsLogger SetDimensions(params DimensionSet[] dimensionSets) => _logger.SetDimensions(dimensionSets);

        public MetricsLogger PutMetric(string key, double value, Unit unit, StorageResolution storageResolution = StorageResolution.STANDARD)
        {
            _metricsCount++;
            return _logger.PutMetric(key, value, unit, storageResolution);
        }

        public MetricsLogger PutMetric(string key, double value, StorageResolution storageResolution = StorageResolution.STANDARD)
        {
            _metricsCount++;
            return _logger.PutMetric(key, value, storageResolution);
        }

        public MetricsLogger PutMetadata(string key, object value) => _logger.PutMetadata(key, value);

        public MetricsLogger SetNamespace(string logNamespace) => _logger.SetNamespace(logNamespace);

        public MetricsLogger SetTimestamp(DateTime dateTime) => _logger.SetTimestamp(dateTime);
    }

The results we are getting with this implementation are very inconsistent. I double checked logs groups some metrics are being logged but some are never reported. Also I expect our old metric dashboard work without any changes in the query since none of the metric names were changed only the medium for sending metrics was updated. Are we missing something or doing something wrong? Will appreciate help. Thanks

MetaData may not be serialized correctly when JsonConvert.DefaultSettings has been changed.

The serialization of the MetaData class is impacted by changing the default JsonSerializerSettings. This may lead to the json no longer being in valid EMF. This was found using Amazon.CloudWatch.EMF 2.1.0.

var jsonSettings = new JsonSerializerSettings
{
	ContractResolver = new DefaultContractResolver
	{
		NamingStrategy = new CamelCaseNamingStrategy()
	}
};

JsonConvert.DefaultSettings = () => jsonSettings;

using var metrics = new MetricsLogger();
var dimensions = new DimensionSet("Dim1", "abc");
metrics.PutMetric("Count", 1, Unit.COUNT);

Actual output:

{"_aws":{"timestamp":1699986206875,"cloudWatchMetrics":[{"Namespace":"aws-embedded-metrics","Metrics":[{"Name":"Count","Unit":"Count"}],"Dimensions":[["ServiceName","ServiceType"]]}]},"ServiceName":"Unknown","ServiceType":"Unknown","Count":1.0}

Expected output:

{"_aws":{"Timestamp":1699986206875,"CloudWatchMetrics":[{"Namespace":"aws-embedded-metrics","Metrics":[{"Name":"Count","Unit":"Count"}],"Dimensions":[["ServiceName","ServiceType"]]}]},"ServiceName":"Unknown","ServiceType":"Unknown","Count":1.0}

Method not found: 'Amazon.CloudWatch.EMF.Logger.MetricsLogger Amazon.CloudWatch.EMF.Logger.IMetricsLogger.PutMetric(System.String, Double, Amazon.CloudWatch.EMF.Model.Unit)'

We are using :

Amazon.CloudWatch.EMF 2.1.0
Amazon.CloudWatch.EMF.Web 1.0.0
ASP.NET Core 6.0 / .NET 6.0

I then add UseEmf and UseEmfMiddleware we are getting the following error when making any call to our API:

An unhandled exception has occurred while executing the request.
System.MissingMethodException: Method not found: 'Amazon.CloudWatch.EMF.Logger.MetricsLogger Amazon.CloudWatch.EMF.Logger.IMetricsLogger.PutMetric(System.String, Double, Amazon.CloudWatch.EMF.Model.Unit)'.
at Amazon.CloudWatch.EMF.Web.ApplicationBuilderExtensions.<>c__DisplayClass1_0.<b__0>d.MoveNext()
at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
at Amazon.CloudWatch.EMF.Web.ApplicationBuilderExtensions.<>c__DisplayClass1_0.b__0(HttpContext context, Func`1 next)
at Microsoft.AspNetCore.Builder.UseExtensions.<>c__DisplayClass0_1.b__1(HttpContext context)
at Microsoft.AspNetCore.StaticFiles.StaticFileMiddleware.Invoke(HttpContext context)
at Swashbuckle.AspNetCore.SwaggerUI.SwaggerUIMiddleware.Invoke(HttpContext httpContext)
at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider)
at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)

This looks to be caused by #48

It should be resolved just by rebuilding and publishing the .Web against 2.1.0.

I've worked around the issue by just copying the source for the .Web package into my codebase.

Undocumented MetricsContext

I see a MetricsContext class in the package, which appears to be undocumented. Can you please explain how this should be used?

MetricsLogger cannot send metrics in parallel

We found that if multiple tasks/threads use the same MetricsLogger and run in parallel, the metrics sent by this MetricsLogger could be messed up by each other.

I have a sample code below

...
public static void SendMetricLog(this IMetricsLogger metricsLogger, string metricsNamespace, 
params (string metricName, double value, Unit unit)[] metricValues)
{
     metricsLogger.SetNamespace(metricsNamespace);
     metricValues.ToList().ForEach(metricValue =>
      {
            metricsLogger.PutMetric(metricValue.metricName, metricValue.value, metricValue.unit);
      });
      metricsLogger.Flush();
}

public static async Task TaskA(IMetricsLogger metricsLogger)
{
      var failedCount = await makeRequestsToServiceAAsync();
      metricsLogger.SendMetricLog("namespaceA", ("RequestAErrors", failedCount, Unit.COUNT));
}

public static async Task TaskB(IMetricsLogger metricsLogger)
{
      var failedCount = await makeRequestsToServiceBAsync();
      metricsLogger.SendMetricLog("namespaceB", ("RequestBErrors", failedCount, Unit.COUNT));
}

...
using var logger = new MetricsLogger();
await Task.WhenAll(TaskA(logger), TaskB(logger));
...

It turns out that the metrics which should be sent to namespaceA are sporadically sent to namespaceB.

Am I missing the doc about the usage of MetricsLogger in the above case or is it a bug in this SDK?

Disallow duplicate dimension sets

Related issues:

Currently duplicate dimension set are being allowed:
Example:

var ds1 = new DimensionSet();
ds1.AddDimension("Region", "us-west-1");
var ds2 = new DimensionSet();
ds2.AddDimension("Region", "us-west-2");

metrics.SetDimensions(ds1);
metrics.PutDimensions(ds2);

metrics.PutMetric("Metric1", 1.0);
metrics.Flush();

This creates

{
  "_aws": {
    "Timestamp": 1660330702000,
    "CloudWatchMetrics": [
      {
        "Namespace": "aws-embedded-metrics",
        "Metrics": [
          {
            "Name": "Metric1",
            "Unit": "None"
          }
        ],
        "Dimensions": [
          [
            "Region"
          ],
          [
            "Region"
          ]
        ]
      }
    ],
    "LogGroupName": "DemoApp"
  },
  "Region": "us-west-1",
  "LogGroupName": "DemoApp",
  "Metric1": 1
}

Instead of:

{
  "_aws": {
    "Timestamp": 1660330702000,
    "CloudWatchMetrics": [
      {
        "Namespace": "aws-embedded-metrics",
        "Metrics": [
          {
            "Name": "Metric1",
            "Unit": "None"
          }
        ],
        "Dimensions": [
          [
            "Region"
          ]
        ]
      }
    ],
    "LogGroupName": "DemoApp"
  },
  "Region": "us-west-2",
  "LogGroupName": "DemoApp",
  "Metric1": 1
}

Investigate Memory Leak

Release canary is showing signs of a memory leak.

ASP.Net Example Project

Target netstandard2.1

Are there plans to target netstandard? Trying to integrate this package into my service and my team has expressed concern about having to change our target to netcoreapp3.1.

asp.net Use EMF for both Controllers and IHostedService

I'm trying to use EMF for both controllers in my web app as well as an IHostedService that does a background refresh of a local cache. The asp.net helpers add the IMetricsLogger dependency as a scoped service. IHostedServices are singletons. When trying to set up a new MetricsLogger() in the hosted service, it looks like it's thrashing configs provided by the singleton elements like IEnvironmentProvider. Is there a way to setup a completely separate MetricsLogger that does not use any of the configs or other resources used in the controller actions?

Question: How is this different than AWS Distro for OpenTelemetry.NET?

I recently came across this nuget as well as AWS Distro for OpenTelemetry.NET. How is this different than AWS Distro for OpenTelemetry.NET and when should I use one over the other?

Can `Timestamp` in metadata be overwritten?

By calling this SDK to send EMF logs, it seems we cannot define the timestamp of the metrics.
The timestamp is generated at

aws-embedded-metrics-dotnet/src/Amazon.CloudWatch.EMF/Model/MetaData.cs

Lines 27 to 29 in fec85c5

    
           [JsonProperty] 
        
           [JsonConverter(typeof(UnixMillisecondDateTimeConverter))] 
        
           internal DateTime Timestamp { get; set; }

, and I don't find the way to overwrite it.

However, we could assign Timestamp as wanted, if we construct the EMF log by our own, see: https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch_Embedded_Metric_Format_Generation_PutLogEvents.html.

Audit Environment Detection

When deploying the canary to ECS, environment detection did not work out of the box as expected. The default sink was stdout.

Implement `PutDimensions` API

The other embedded metrics libraries (python, node, java) have an API to add individual metrics, to stay consistent with them, the .Net library should also have said API.

@gordonpn The dotnet library doesn't have this issue since it doesn't have a putDimension method. It only has a setDimensions method.

Originally posted by @markkuhn in awslabs/aws-embedded-metrics-python#83 (comment)

Readme documentation

Minimal Apis Integration

The current aspnetcore integration relies on Controller to be defined

	[JsonProperty]
	[JsonConverter(typeof(UnixMillisecondDateTimeConverter))]
	internal DateTime Timestamp { get; set; }

awslabs / aws-embedded-metrics-dotnet Goto Github PK

aws-embedded-metrics-dotnet's Issues

Recommend Projects

Recommend Topics

Recommend Org