Merge branch 'chore/clean-logging-in-metrics-go' of github.com:wonko/opencost into chore/clean-logging-in-metrics-go

CONTRIBUTING.md

@@ -32,6 +32,7 @@ Dependencies:
 1. Docker (with `buildx`)
 2. [just](https://github.com/casey/just) (if you don't want to install it , Just read the `justfile` and run the commands manually)
 3. Multi-arch `buildx` builders set up via https://github.com/tonistiigi/binfmt
+4. `manifest-tool` via https://github.com/estesp/manifest-tool
 4. `npm` (if you want to build the UI)
 
 ### Build the backend

justfile

@@ -1,6 +1,6 @@
 commonenv := "CGO_ENABLED=0"
 
-version := "dev"
+version := `./tools/image-tag`
 commit := `git rev-parse --short HEAD`
 
 default:

pkg/cloud/aws/provider_test.go

@@ -2,7 +2,7 @@ package aws
 
 import (
 	"bytes"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"net/url"
 	"reflect"
@@ -306,7 +306,7 @@ func Test_populate_pricing(t *testing.T) {
 	`
 
 	testResponse := http.Response{
-		Body: ioutil.NopCloser(bytes.NewBufferString(awsUSEastString)),
+		Body: io.NopCloser(bytes.NewBufferString(awsUSEastString)),
 		Request: &http.Request{
 			URL: &url.URL{
 				Scheme: "https",
@@ -445,7 +445,7 @@ func Test_populate_pricing(t *testing.T) {
 	}
 
 	testResponse = http.Response{
-		Body: ioutil.NopCloser(bytes.NewBufferString(awsCnString)),
+		Body: io.NopCloser(bytes.NewBufferString(awsCnString)),
 		Request: &http.Request{
 			URL: &url.URL{
 				Scheme: "https",

pkg/cloud/scaleway/provider.go

@@ -207,7 +207,13 @@ func (key *scalewayPVKey) Features() string {
 
 func (c *Scaleway) GetPVKey(pv *v1.PersistentVolume, parameters map[string]string, defaultRegion string) models.PVKey {
 	// the csi volume handle is the form <az>/<volume-id>
-	zone := strings.Split(pv.Spec.CSI.VolumeHandle, "/")[0]
+	zone := ""
+	if pv.Spec.CSI != nil {
+		zoneVolID := strings.Split(pv.Spec.CSI.VolumeHandle, "/")
+		if len(zoneVolID) > 0 {
+			zone = zoneVolID[0]
+		}
+	}
 	return &scalewayPVKey{
 		Labels:                 pv.Labels,
 		StorageClassName:       pv.Spec.StorageClassName,

pkg/cloudcost/memoryrepository_test.go

@@ -2,6 +2,7 @@ package cloudcost
 
 import (
 	"reflect"
+	"sort"
 	"testing"
 	"time"
 
@@ -172,6 +173,8 @@ func TestMemoryRepository_Keys(t *testing.T) {
 				t.Errorf("Keys() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
+			sort.Strings(got)
+			sort.Strings(tt.want)
 			if !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("Keys() got = %v, want %v", got, tt.want)
 			}

pkg/cmd/costmodel/costmodel.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"net/http/pprof"
 	"time"
 
 	"github.com/julienschmidt/httprouter"
@@ -62,6 +63,16 @@ func Execute(opts *CostModelOpts) error {
 	a.Router.GET("/cloudCost/rebuild", a.CloudCostPipelineService.GetCloudCostRebuildHandler())
 	a.Router.GET("/cloudCost/repair", a.CloudCostPipelineService.GetCloudCostRepairHandler())
 
+	if env.IsPProfEnabled() {
+		a.Router.HandlerFunc(http.MethodGet, "/debug/pprof/", pprof.Index)
+		a.Router.HandlerFunc(http.MethodGet, "/debug/pprof/cmdline", pprof.Cmdline)
+		a.Router.HandlerFunc(http.MethodGet, "/debug/pprof/profile", pprof.Profile)
+		a.Router.HandlerFunc(http.MethodGet, "/debug/pprof/symbol", pprof.Symbol)
+		a.Router.HandlerFunc(http.MethodGet, "/debug/pprof/trace", pprof.Trace)
+		a.Router.Handler(http.MethodGet, "/debug/pprof/goroutine", pprof.Handler("goroutine"))
+		a.Router.Handler(http.MethodGet, "/debug/pprof/heap", pprof.Handler("heap"))
+	}
+
 	rootMux.Handle("/", a.Router)
 	rootMux.Handle("/metrics", promhttp.Handler())
 	telemetryHandler := metrics.ResponseMetricMiddleware(rootMux)

pkg/costmodel/cluster_helpers.go

@@ -636,13 +636,11 @@ func buildLabelsMap(
 			Name:    node,
 		}
 
-		m[key] = make(map[string]string)
-
-		for name, value := range result.Metric {
-			if val, ok := value.(string); ok {
-				m[key][name] = val
-			}
-		}
+		// The QueryResult.GetLabels function needs to be called to sanitize the
+		// ingested label data. This removes the label_ prefix that prometheus
+		// adds to emitted labels. It also keeps from ingesting prometheus labels
+		// that aren't a part of the asset.
+		m[key] = result.GetLabels()
 	}
 	return m
 }

pkg/costmodel/cluster_helpers_test.go

@@ -2,6 +2,7 @@ package costmodel
 
 import (
 	"reflect"
+	"strings"
 	"testing"
 	"time"
 
@@ -1108,3 +1109,74 @@ func TestAssetCustompricing(t *testing.T) {
 	}
 
 }
+
+func TestBuildLabelsMap(t *testing.T) {
+	const (
+		labelKey1   = "testlabelkey1"
+		labelValue1 = "testlabel1-value"
+		labelKey2   = "test-label-key-2"
+		labelValue2 = "testlabel2.value"
+		nonLabelKey = "instance_type"
+		labelPrefix = "label_"
+	)
+
+	startTimestamp := float64(windowStart.Unix())
+
+	nodePromResult := []*prom.QueryResult{
+		{
+			Metric: map[string]interface{}{
+				"cluster_id":             "cluster1",
+				"node":                   "node1",
+				"instance_type":          "type1",
+				"provider_id":            "provider1",
+				"label_testlabelkey1":    "testlabel1-value",
+				"label_test-label-key-2": "testlabel2.value",
+			},
+			Values: []*util.Vector{
+				{
+					Timestamp: startTimestamp,
+					Value:     0.5,
+				},
+			},
+		},
+		{
+			Metric: map[string]interface{}{
+				"cluster_id":             "cluster1",
+				"node":                   "node2",
+				"instance_type":          "type1",
+				"provider_id":            "provider1",
+				"label_testlabelkey1":    "testlabel1-value",
+				"label_test-label-key-2": "testlabel2.value",
+			},
+			Values: []*util.Vector{
+				{
+					Timestamp: startTimestamp,
+					Value:     0.5,
+				},
+			},
+		},
+	}
+
+	nodeLabelMap := buildLabelsMap(nodePromResult)
+	// Test that for all nodes and all label keys in the map there isn't a key with the label_ prefix.
+	for _, labelMap := range nodeLabelMap {
+		for key, value := range labelMap {
+			if strings.HasPrefix(key, labelPrefix) {
+				t.Errorf("Asset label maps aren't sanitized. Expected no '%v' prefix in %v", labelPrefix, key)
+			}
+			// Test that the label value isn't touched
+			if key == labelKey1 && value != labelValue1 {
+				t.Errorf("Label Value didn't match. Got %v, but Expected: %v", value, labelValue1)
+			}
+			// Test that the label value isn't touched
+			if key == labelKey2 && value != labelValue2 {
+				t.Errorf("Label Value didn't match. Got %v, but Expected: %v", value, labelValue2)
+			}
+		}
+		// Test that keys that don't have the label_ prefix aren't in the resultant label map.
+		_, ok := labelMap[nonLabelKey]
+		if ok {
+			t.Errorf("Non-label keys are included in label mapping for asset labels. Expected '%v' to not exist'.", nonLabelKey)
+		}
+	}
+}

pkg/costmodel/metrics.go

@@ -665,7 +665,7 @@ func (cmme *CostModelMetricsEmitter) Start() bool {
 				GetPVCost(cacPv, pv, cmme.CloudProvider, region)
 				c, _ := strconv.ParseFloat(cacPv.Cost, 64)
 				cmme.PersistentVolumePriceRecorder.WithLabelValues(pv.Name, pv.Name, cacPv.ProviderID).Set(c)
-				labelKey := getKeyFromLabelStrings(pv.Name, pv.Name)
+				labelKey := getKeyFromLabelStrings(pv.Name, pv.Name, cacPv.ProviderID)
 				pvSeen[labelKey] = true
 			}
 

pkg/env/costmodelenv.go

@@ -51,6 +51,8 @@ const (
 	ThanosOffsetEnvVar       = "THANOS_QUERY_OFFSET"
 	ThanosMaxSourceResEnvVar = "THANOS_MAX_SOURCE_RESOLUTION"
 
+	PProfEnabledEnvVar = "PPROF_ENABLED"
+
 	LogCollectionEnabledEnvVar    = "LOG_COLLECTION_ENABLED"
 	ProductAnalyticsEnabledEnvVar = "PRODUCT_ANALYTICS_ENABLED"
 	ErrorReportingEnabledEnvVar   = "ERROR_REPORTING_ENABLED"
@@ -137,6 +139,10 @@ func GetExportCSVLabelsList() []string {
 	return GetList(ExportCSVLabelsList, ",")
 }
 
+func IsPProfEnabled() bool {
+	return GetBool(PProfEnabledEnvVar, false)
+}
+
 func GetExportCSVMaxDays() int {
 	return GetInt(ExportCSVMaxDays, 90)
 }

pkg/kubecost/asset.go

@@ -4,7 +4,6 @@ import (
 	"encoding"
 	"fmt"
 	"math"
-	"regexp"
 	"strings"
 	"time"
 
@@ -12,6 +11,7 @@ import (
 	"github.com/opencost/opencost/pkg/filter21/ast"
 	"github.com/opencost/opencost/pkg/filter21/matcher"
 	"github.com/opencost/opencost/pkg/log"
+	"github.com/opencost/opencost/pkg/prom"
 	"github.com/opencost/opencost/pkg/util/json"
 	"github.com/opencost/opencost/pkg/util/timeutil"
 )
@@ -4139,8 +4139,8 @@ func GetNodePoolName(provider string, labels map[string]string) string {
 }
 
 func getPoolNameHelper(label string, labels map[string]string) string {
-	sanitizedLabel := regexp.MustCompile(`[^a-zA-Z0-9 ]+`).ReplaceAllString(label, "_")
-	if poolName, found := labels[fmt.Sprintf("label_%s", sanitizedLabel)]; found {
+	sanitizedLabel := prom.SanitizeLabelName(label)
+	if poolName, found := labels[sanitizedLabel]; found {
 		return poolName
 	} else {
 		log.Warnf("unable to derive node pool name from node labels")

ui/package.json

@@ -21,7 +21,7 @@
     "@material-ui/icons": "^4.11.2",
     "@material-ui/pickers": "^3.3.10",
     "@material-ui/styles": "^4.11.5",
-    "axios": "^1.4.0",
+    "axios": "^1.6.0",
     "date-fns": "^2.30.0",
     "material-design-icons-iconfont": "^6.1.0",
     "prop-types": "^15.7.2",