Fix metric namespace (#108)

* Updated metric namespace to match other plugins

* Add accountID to constructor

* Update start logic to avoid a non lambda start

* Convert SetResourceTypeName method to a ResourceTypeName function

* Update testing for thr function

* remove code duplication

* remove code duplication

* Don't send metric data when no tag is provided

* log metric errors and move on

* Remove metric build tag because it is not needed

* Regenerate Makefile when cfn generate is ran

* Run pre-commit

* Fix name typo

* Update test resource name

*  simplify Start function logic

Author: brianterry

cfn/cfn.go

@@ -76,14 +76,14 @@ func Start(h Handler) {
 
 	// MODE is an environment variable that is set ONLY
 	// when contract test are performed.
-	if mode, ok := os.LookupEnv("MODE"); ok == true {
-		if mode == "Test" {
-			lambda.Start(makeTestEventFunc(h))
+	mode, _ := os.LookupEnv("MODE")
 
-		} else {
-			lambda.Start(makeEventFunc(h))
-		}
+	if mode == "Test" {
+		lambda.Start(makeTestEventFunc(h))
+	} else {
+		lambda.Start(makeEventFunc(h))
 	}
+
 }
 
 // Tags are stored as key/value paired strings
@@ -135,37 +135,21 @@ func invoke(handlerFn handlerFunc, request handler.Request, metricsPublisher *me
 		// Create a channel to received a signal that work is done.
 		ch := make(chan handler.ProgressEvent, 1)
 
-		// Create a channel to received error.
-		cherror := make(chan error, 1)
-
 		// Ask the goroutine to do some work for us.
 		go func() {
 			//start the timer
 			start := time.Now()
-			if err := metricsPublisher.PublishInvocationMetric(time.Now(), string(action)); err != nil {
-				cherror <- err
-			}
+			metricsPublisher.PublishInvocationMetric(time.Now(), string(action))
 
 			// Report the work is done.
 			progEvt := handlerFn(request)
-
 			elapsed := time.Since(start)
-
-			if err := metricsPublisher.PublishDurationMetric(time.Now(), string(action), elapsed.Seconds()*1e3); err != nil {
-				cherror <- err
-			}
-
+			metricsPublisher.PublishDurationMetric(time.Now(), string(action), elapsed.Seconds()*1e3)
 			ch <- progEvt
 		}()
 
 		// Wait for the work to finish. If it takes too long move on. If the function returns an error, signal the error channel.
 		select {
-		case e := <-cherror:
-			cfnErr := cfnerr.New(timeoutError, "Handler error", e)
-			metricsPublisher.PublishExceptionMetric(time.Now(), string(action), cfnErr)
-			//The handler returned an error.
-			return handler.ProgressEvent{}, e
-
 		case d := <-ch:
 			//Return the response from the handler.
 			return d, nil
@@ -263,8 +247,7 @@ func makeEventFunc(h Handler) eventFunc {
 		// Set default logger to output to CWL in the provider account
 		logging.SetProviderLogOutput(logsProvider)
 
-		metricsPublisher := metrics.New(cloudwatch.New(platformSession))
-		metricsPublisher.SetResourceTypeName(event.ResourceType)
+		metricsPublisher := metrics.New(cloudwatch.New(platformSession), event.AWSAccountID, event.ResourceType)
 		callbackAdapter := callback.New(cloudformation.New(platformSession), event.BearerToken)
 		invokeScheduler := scheduler.New(cloudwatchevents.New(platformSession))
 		re := newReportErr(callbackAdapter, metricsPublisher)

cfn/entry_test.go

@@ -128,8 +128,7 @@ func TestHandler(t *testing.T) {
 
 func TestInvoke(t *testing.T) {
 	mockClient := NewMockedMetrics()
-	mockPub := metrics.New(mockClient)
-	mockPub.SetResourceTypeName("dsf::fs::sfa")
+	mockPub := metrics.New(mockClient, "12345678", "foo::bar::test")
 
 	// For test purposes, set the timeout low
 	Timeout = time.Second

cfn/metrics/publisher.go

@@ -1,16 +1,12 @@
-// +build metrics
-
 package metrics
 
 import (
-	"errors"
 	"fmt"
 	"log"
 	"os"
 	"strings"
 	"time"
 
-	"github.com/aws-cloudformation/cloudformation-cli-go-plugin/cfn/cfnerr"
 	"github.com/aws-cloudformation/cloudformation-cli-go-plugin/cfn/logging"
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/cloudwatch"
@@ -30,108 +26,76 @@ const (
 	DimensionKeyAcionType = "Action"
 	//DimensionKeyExceptionType  is the ExceptionType in the dimension.
 	DimensionKeyExceptionType = "ExceptionType"
-	//DimensionKeyResouceType  is the ResourceType in the dimension.
-	DimensionKeyResouceType = "ResourceType"
+	//DimensionKeyResourceType  is the ResourceType in the dimension.
+	DimensionKeyResourceType = "ResourceType"
 	//ServiceInternalError ...
 	ServiceInternalError string = "ServiceInternal"
 )
 
 // A Publisher represents an object that publishes metrics to AWS Cloudwatch.
 type Publisher struct {
-	client    cloudwatchiface.CloudWatchAPI // AWS CloudWatch Service Client
-	namespace string                        // custom resouces's namespace
-	logger    *log.Logger
+	client       cloudwatchiface.CloudWatchAPI // AWS CloudWatch Service Client
+	namespace    string                        // custom resouces's namespace
+	logger       *log.Logger
+	resourceType string // type of resource
 }
 
 // New creates a new Publisher.
-func New(client cloudwatchiface.CloudWatchAPI) *Publisher {
+func New(client cloudwatchiface.CloudWatchAPI, account string, resType string) *Publisher {
 	if len(os.Getenv("AWS_SAM_LOCAL")) > 0 {
 		client = newNoopClient()
 	}
 
+	rn := ResourceTypeName(resType)
 	return &Publisher{
-		client: client,
-		logger: logging.New("metrics"),
+		client:       client,
+		logger:       logging.New("metrics"),
+		namespace:    fmt.Sprintf("%s/%s/%s", MetricNameSpaceRoot, account, rn),
+		resourceType: rn,
 	}
 }
 
 // PublishExceptionMetric publishes an exception metric.
-func (p *Publisher) PublishExceptionMetric(date time.Time, action string, e error) error {
-
-	if len(p.namespace) == 0 {
-		message := fmt.Sprintf("Name Space was not set")
-		err := errors.New(message)
-		return cfnerr.New(ServiceInternalError, "Publisher error", err)
-	}
-
+func (p *Publisher) PublishExceptionMetric(date time.Time, action string, e error) {
 	dimensions := map[string]string{
 		DimensionKeyAcionType:     string(action),
 		DimensionKeyExceptionType: e.Error(),
-		DimensionKeyResouceType:   p.namespace,
-	}
-
-	_, err := p.publishMetric(MetricNameHanderException, dimensions, cloudwatch.StandardUnitCount, 1.0, date)
-
-	if err != nil {
-		return cfnerr.New(ServiceInternalError, "Publisher error", err)
+		DimensionKeyResourceType:  p.resourceType,
 	}
 
-	return nil
+	p.publishMetric(MetricNameHanderException, dimensions, cloudwatch.StandardUnitCount, 1.0, date)
 }
 
 // PublishInvocationMetric publishes an invocation metric.
-func (p *Publisher) PublishInvocationMetric(date time.Time, action string) error {
-
-	if len(p.namespace) == 0 {
-		message := fmt.Sprintf("Name Space was not set")
-		err := errors.New(message)
-		return cfnerr.New(ServiceInternalError, "Publisher error", err)
-	}
-
+func (p *Publisher) PublishInvocationMetric(date time.Time, action string) {
 	dimensions := map[string]string{
-		DimensionKeyAcionType:   string(action),
-		DimensionKeyResouceType: p.namespace,
+		DimensionKeyAcionType:    string(action),
+		DimensionKeyResourceType: p.resourceType,
 	}
 
-	_, err := p.publishMetric(MetricNameHanderInvocationCount, dimensions, cloudwatch.StandardUnitCount, 1.0, date)
+	p.publishMetric(MetricNameHanderInvocationCount, dimensions, cloudwatch.StandardUnitCount, 1.0, date)
 
-	if err != nil {
-		return cfnerr.New(ServiceInternalError, "Publisher error", err)
-	}
-
-	return nil
 }
 
 // PublishDurationMetric publishes an duration metric.
 //
 // A duration metric is the timing of something.
-func (p *Publisher) PublishDurationMetric(date time.Time, action string, secs float64) error {
-	if len(p.namespace) == 0 {
-		message := fmt.Sprintf("Name Space was not set")
-		err := errors.New(message)
-		return cfnerr.New(ServiceInternalError, "Publisher error", err)
-	}
+func (p *Publisher) PublishDurationMetric(date time.Time, action string, secs float64) {
 	dimensions := map[string]string{
-		DimensionKeyAcionType:   string(action),
-		DimensionKeyResouceType: p.namespace,
+		DimensionKeyAcionType:    string(action),
+		DimensionKeyResourceType: p.resourceType,
 	}
 
-	_, err := p.publishMetric(MetricNameHanderDuration, dimensions, cloudwatch.StandardUnitMilliseconds, secs, date)
+	p.publishMetric(MetricNameHanderDuration, dimensions, cloudwatch.StandardUnitMilliseconds, secs, date)
 
-	if err != nil {
-		return cfnerr.New(ServiceInternalError, "Publisher error", err)
-	}
-
-	return nil
 }
 
-func (p *Publisher) publishMetric(metricName string, data map[string]string, unit string, value float64, date time.Time) (*cloudwatch.PutMetricDataOutput, error) {
+func (p *Publisher) publishMetric(metricName string, data map[string]string, unit string, value float64, date time.Time) {
 
 	var d []*cloudwatch.Dimension
 
 	for k, v := range data {
 		dim := &cloudwatch.Dimension{
-
 			Name:  aws.String(k),
 			Value: aws.String(v),
 		}
@@ -152,25 +116,22 @@ func (p *Publisher) publishMetric(metricName string, data map[string]string, uni
 		MetricData: md,
 	}
 
-	out, err := p.client.PutMetricData(&pi)
+	_, err := p.client.PutMetricData(&pi)
 
 	if err != nil {
+		p.logger.Printf("An error occurred while publishing metrics: %s", err)
 
-		return nil, cfnerr.New(ServiceInternalError, "Publisher error", err)
 	}
-
-	return out, nil
 }
 
-// SetResourceTypeName returns a type name by removing (::) and replaing with (/)
+// ResourceTypeName returns a type name by removing (::) and replaing with (/)
 //
 // Example
 //
-// 	pub := metrics.New(cw)
+// 	r := metrics.ResourceTypeName("AWS::Service::Resource")
 //
 // 	// Will return "AWS/Service/Resource"
-// 	pub.SetResourceTypeName("AWS::Service::Resource")
-func (p *Publisher) SetResourceTypeName(t string) {
-	p.namespace = strings.ReplaceAll(t, "::", "/")
+func ResourceTypeName(t string) string {
+	return strings.ReplaceAll(t, "::", "/")
 
 }