From 966ce58525c96b4909735bf53575832ea0db7221 Mon Sep 17 00:00:00 2001
From: Gerardo Viedma <gviedma@users.noreply.github.com>
Date: Mon, 15 Jan 2018 10:09:03 +0000
Subject: [PATCH] Use new metrics API for s3 log metrics (#680)

* use new metrics API for histogram metrics

* Avoid creating an extra tracing span

* use new metrics api for histograms

* fix minor formatting issue
---
 api/common/metrics.go | 29 +++++++++++++++++++++++++++++
 api/logs/s3/s3.go     |  6 +++---
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/api/common/metrics.go b/api/common/metrics.go
index d939ac251..712675ccc 100644
--- a/api/common/metrics.go
+++ b/api/common/metrics.go
@@ -61,6 +61,7 @@ func IncrementCounter(ctx context.Context, metric string) {
 
 // PublishHistograms publishes the specifed histogram metrics
 // It does this by logging appropriate field values to a tracing span
+// Use this when the current tracing span is long-lived and you want the metric to be visible before it ends
 func PublishHistograms(ctx context.Context, metrics map[string]float64) {
 
 	// Spans are not processed by the collector until the span ends, so to prevent any delay
@@ -77,6 +78,34 @@ func PublishHistograms(ctx context.Context, metrics map[string]float64) {
 	}
 }
 
+// PublishHistogram publishes the specifed histogram metric
+// It does this by logging an appropriate field value to a tracing span
+// Use this when the current tracing span is long-lived and you want the metric to be visible before it ends
+func PublishHistogram(ctx context.Context, key string, value float64) {
+
+	// Spans are not processed by the collector until the span ends, so to prevent any delay
+	// in processing the stats when the current span is long-lived we create a new span for every call.
+	// suffix the span name with SpannameSuffixDummy to denote that it is used only to hold a metric and isn't itself of any interest
+	span, ctx := opentracing.StartSpanFromContext(ctx, "histogram_metrics"+SpannameSuffixDummy)
+	defer span.Finish()
+
+	// The field name we use is the metric name prepended with FieldnamePrefixHistogram to designate that it is a Prometheus histogram metric
+	// The collector will replace that prefix with "fn_" and use the result as the Prometheus metric name.
+	fieldname := FieldnamePrefixHistogram + key
+	span.LogFields(log.Float64(fieldname, value))
+}
+
+// PublishHistogramToSpan publishes the specifed histogram metric
+// It does this by logging an appropriate field value to the specified tracing span
+// Use this when you don't need to create a new tracing span
+func PublishHistogramToSpan(span opentracing.Span, key string, value float64) {
+
+	// The field name we use is the metric name prepended with FieldnamePrefixHistogram to designate that it is a Prometheus histogram metric
+	// The collector will replace that prefix with "fn_" and use the result as the Prometheus metric name.
+	fieldname := FieldnamePrefixHistogram + key
+	span.LogFields(log.Float64(fieldname, value))
+}
+
 const (
 
 	// FnPrefix is a constant for "fn_", used as a prefix for span names, field names, Prometheus metric names and Prometheus label names
diff --git a/api/logs/s3/s3.go b/api/logs/s3/s3.go
index 20c214f73..ccaa816b7 100644
--- a/api/logs/s3/s3.go
+++ b/api/logs/s3/s3.go
@@ -17,9 +17,9 @@ import (
 	"github.com/aws/aws-sdk-go/aws/session"
 	"github.com/aws/aws-sdk-go/service/s3"
 	"github.com/aws/aws-sdk-go/service/s3/s3manager"
+	"github.com/fnproject/fn/api/common"
 	"github.com/fnproject/fn/api/models"
 	"github.com/opentracing/opentracing-go"
-	"github.com/opentracing/opentracing-go/log"
 	"github.com/sirupsen/logrus"
 )
 
@@ -144,7 +144,7 @@ func (s *store) InsertLog(ctx context.Context, appName, callID string, callLog i
 		return fmt.Errorf("failed to write log, %v", err)
 	}
 
-	span.LogFields(log.Int("fn_s3_log_upload_size", cr.count))
+	common.PublishHistogramToSpan(span, "s3_log_upload_size", float64(cr.count))
 	return nil
 }
 
@@ -169,6 +169,6 @@ func (s *store) GetLog(ctx context.Context, appName, callID string) (io.Reader,
 		return nil, fmt.Errorf("failed to read log, %v", err)
 	}
 
-	span.LogFields(log.Int64("fn_s3_log_download_size", size))
+	common.PublishHistogramToSpan(span, "s3_log_download_size", float64(size))
 	return bytes.NewReader(target.Bytes()), nil
 }