Merge pull request #547 from bookingcom/dzhdanov/aliassub-fix

Backporting aliasSub fix from go-graphite/carbonapi/issues/290

Author: deniszh

.github/workflows/ci.yml

@@ -9,7 +9,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        go-version: [ 1.21.x, 1.22.x, 1.23.x, tip ]
+        go-version: [ 1.23.x, 1.24.x, tip ]
     steps:
       - name: Set up Go stable
         if: matrix.go-version != 'tip'
@@ -31,11 +31,18 @@ jobs:
         uses: actions/checkout@v4
       - name: test
         run: make test
+      - name: govulncheck
+        if: matrix.go-version != 'tip'
+        uses: golang/govulncheck-action@v1
+        with:
+          go-version-input: ${{ matrix.go-version }}
+          check-latest: true
+          go-package: ./...
       - name: golangci-lint
-        if: matrix.go-version == '1.23.x'
-        uses: golangci/golangci-lint-action@v6
+        if: matrix.go-version == '1.24.x'
+        uses: golangci/golangci-lint-action@v8
         with:
-          version: v1.60.1
+          version: v2.1.6
       - name: integration test
-        if: matrix.go-version == '1.23.x'
+        if: matrix.go-version == '1.24.x'
         run: tests/system_test.sh

.golangci.yml

@@ -1,49 +1,65 @@
+version: "2"
 run:
-  timeout: 2m
+  build-tags:
+    - cairo
   tests: true
-  build-tags: [ cairo ]
-  skip-files:
-    - ".*\\.pb\\.go$"
-    - "function_test.go"
-    - "expr_test.go"
-    - "cairo_test.go"
-
+  timeout: 5m
 linters:
   enable:
     - asciicheck
     - bodyclose
-    - gochecknoinits
-    - gofmt
-    - goimports
-    - gosec
-    - misspell
     - unparam
-
-    # The linters we need to enable ASAP.
-    # Enabling would require significant changes.
-    # - gochecknoglobals
-    # - unconvert
-    # - wrapcheck
-
-    # The linters that would be nice to have in order of decreasing priority.
-    # They are disabled now because they cause a lot of warnings that would require multiple changes.
-    # - revive
-    # - errorlint
-    # - promlinter
-    # - cyclop or gocyclo
-    # - whitespace
-    # - gomnd
-    # - lll
-    # - gocritic
-
+    - staticcheck
+  settings:
+    staticcheck:
+      # SAxxxx checks in https://staticcheck.dev/docs/configuration/options/#checks
+      # Example (to disable some checks): [ "all", "-SA1000", "-SA1001"]
+      # Default: ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022"]
+      checks:
+        - all
+        # Incorrect or missing package comment.
+        # https://staticcheck.dev/docs/checks/#ST1000
+        - -ST1000
+        # Use consistent method receiver names.
+        # https://staticcheck.dev/docs/checks/#ST1016
+        - -ST1016
+        # Omit embedded fields from selector expression.
+        # https://staticcheck.dev/docs/checks/#QF1008
+        - -QF1008
+        # Some other staticcheck checks
+        - -ST1003
+        - -ST1005
+        - -ST1008
+        - -QF1004
+        - -QF1001
+  exclusions:
+    generated: lax
+    presets:
+      - comments
+      - common-false-positives
+      - legacy
+      - std-error-handling
+    rules:
+      - linters:
+          - errcheck
+          - govet
+          - gofmt
+        path: _test\.go
+    paths:
+      - third_party$
+      - builtin$
+      - examples$
+      - \.pb\.go$
 issues:
-  # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
   max-issues-per-linter: 0
-  # Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
   max-same-issues: 0
-  # Excluding configuration per-path, per-linter, per-text and per-source
-  exclude-rules:
-    # Exclude some linters from running on tests files.
-    - path: _test\.go
-      linters:
-        - errcheck
+formatters:
+  enable:
+    - gofmt
+    - goimports
+  exclusions:
+    generated: lax
+    paths:
+      - third_party$
+      - builtin$
+      - examples$

docker/carbonapi/Dockerfile

@@ -1,4 +1,4 @@
-FROM golang:1.21
+FROM golang:1.24
 
 RUN apt-get update
 RUN apt-get install -y libcairo2-dev

docker/go-carbon/Dockerfile

@@ -1,4 +1,4 @@
-FROM golang:1.21
+FROM golang:1.24
 
 RUN mkdir -p /data/graphite/whisper/
 COPY ./docker/go-carbon/*.conf /etc/

pkg/expr/functions/aliasSub/function.go

@@ -55,10 +55,8 @@ func (f *aliasSub) Do(ctx context.Context, e parser.Expr, from, until int32, val
 	var results []*types.MetricData
 
 	for _, a := range args {
-		metric := helper.ExtractMetric(a.Name)
-
 		r := *a
-		r.Name = re.ReplaceAllString(metric, replace)
+		r.Name = re.ReplaceAllString(r.Name, replace)
 		results = append(results, &r)
 	}
 

pkg/expr/functions/aliasSub/function_test.go

@@ -51,6 +51,24 @@ func TestAliasByNode(t *testing.T) {
 			[]*types.MetricData{types.MakeMetricData("100",
 				[]float64{1, 2, 3, 4, 5}, 1, now32)},
 		},
+		{
+			"aliasSub(metric1.foo.bar.baz, \"foo\", \"replaced\")",
+			map[parser.MetricRequest][]*types.MetricData{
+				{"metric1.foo.bar.baz", 0, 1}: {types.MakeMetricData("metric1.foo.bar.baz", []float64{1, 2, 3, 4, 5}, 1, now32)},
+			},
+			[]*types.MetricData{types.MakeMetricData("metric1.replaced.bar.baz",
+				[]float64{1, 2, 3, 4, 5}, 1, now32)},
+		},
+		// https://github.com/go-graphite/carbonapi/issues/290
+		{
+			//"aliasSub(*, '.dns.([^.]+).zone.', '\\1 diff to sql')",
+			"aliasSub(*, 'dns.([^.]*).zone.', '\\1 diff to sql ')",
+			map[parser.MetricRequest][]*types.MetricData{
+				{"*", 0, 1}: {types.MakeMetricData("diffSeries(dns.snake.sql_updated, dns.snake.zone_updated)", []float64{1, 2, 3, 4, 5}, 1, now32)},
+			},
+			[]*types.MetricData{types.MakeMetricData("diffSeries(dns.snake.sql_updated, snake diff to sql updated)",
+				[]float64{1, 2, 3, 4, 5}, 1, now32)},
+		},
 	}
 
 	for _, tt := range tests {

pkg/expr/functions/cactiStyle/function.go

@@ -76,7 +76,8 @@ func (f *cactiStyle) Do(ctx context.Context, e parser.Expr, from, until int32, v
 		min := ""
 		max := ""
 		current := ""
-		if system == "si" {
+		switch system {
+		case "si":
 			mv, mf := humanize.ComputeSI(minVal)
 			xv, xf := humanize.ComputeSI(maxVal)
 			cv, cf := humanize.ComputeSI(currentVal)
@@ -85,12 +86,12 @@ func (f *cactiStyle) Do(ctx context.Context, e parser.Expr, from, until int32, v
 			max = fmt.Sprintf("%.0f%s", xv, xf)
 			current = fmt.Sprintf("%.0f%s", cv, cf)
 
-		} else if system == "" {
+		case "":
 			min = fmt.Sprintf("%.0f", minVal)
 			max = fmt.Sprintf("%.0f", maxVal)
 			current = fmt.Sprintf("%.0f", currentVal)
 
-		} else {
+		default:
 			return nil, fmt.Errorf("%s is not supported for system", system)
 		}
 

pkg/expr/functions/cairo/cairo_test.go

@@ -4,10 +4,11 @@
 package cairo
 
 import (
-	"go.uber.org/zap"
 	"testing"
 	"time"
 
+	"go.uber.org/zap"
+
 	"github.com/bookingcom/carbonapi/pkg/expr/metadata"
 	"github.com/bookingcom/carbonapi/pkg/expr/types"
 	"github.com/bookingcom/carbonapi/pkg/parser"

pkg/expr/functions/cairo/png/cairo.go

@@ -1200,15 +1200,16 @@ func drawGraph(cr *cairoSurfaceContext, params *Params,
 	}
 
 	// check if we need to stack all the things
-	if params.areaMode == AreaModeStacked {
+	switch params.areaMode {
+	case AreaModeStacked:
 		params.hasStack = true
 		for _, r := range results {
 			r.Stacked = true
 			r.StackName = "stack"
 		}
-	} else if params.areaMode == AreaModeFirst {
+	case AreaModeFirst:
 		results[0].Stacked = true
-	} else if params.areaMode == AreaModeAll {
+	case AreaModeAll:
 		for _, r := range results {
 			r.Stacked = true
 		}

pkg/expr/functions/legendValue/function.go

@@ -63,14 +63,15 @@ func (f *legendValue) Do(ctx context.Context, e parser.Expr, from, until int32,
 			}
 
 			summary := ""
-			if system == "si" {
+			switch system {
+			case "si":
 				sv, sf := humanize.ComputeSI(summaryVal)
 				summary = fmt.Sprintf("%.1f %s", sv, sf)
-			} else if system == "binary" {
+			case "binary":
 				summary = humanize.IBytes(uint64(summaryVal))
-			} else if system == "" {
+			case "":
 				summary = fmt.Sprintf("%f", summaryVal)
-			} else {
+			default:
 				return nil, fmt.Errorf("%s is not supported for system", system)
 			}
 			values = append(values, fmt.Sprintf("%s: %s", method, summary))