From 9abd59f0ba5e45405e14c951d30b038cb97d8ddf Mon Sep 17 00:00:00 2001 From: Andrew Seigner Date: Sat, 23 Feb 2019 21:06:37 -0800 Subject: [PATCH] lint: Enable goconst goconst finds repeated strings that could be replaced by a constant: https://github.com/jgautheron/goconst Part of #217 Signed-off-by: Andrew Seigner --- .golangci.yml | 2 +- cli/cmd/endpoints.go | 16 ++++----- cli/cmd/endpoints_test.go | 4 +-- cli/cmd/inject_test.go | 33 ++++++++----------- cli/cmd/install_test.go | 8 ++--- cli/cmd/profile_test.go | 8 +++-- cli/cmd/root.go | 13 +++++--- cli/cmd/routes.go | 16 ++++----- cli/cmd/routes_test.go | 2 +- cli/cmd/stat.go | 6 ++-- cli/cmd/stat_test.go | 4 +-- cli/cmd/tap.go | 2 +- cli/cmd/tap_test.go | 5 ++- cni-plugin/test/install-cni_test.go | 5 ++- .../api/destination/endpoint_listener_test.go | 14 ++++---- controller/api/public/stat_summary.go | 7 ++-- controller/api/public/stat_summary_test.go | 28 ++++++++-------- controller/api/public/top_routes.go | 8 ++--- controller/api/public/top_routes_test.go | 4 +-- 19 files changed, 93 insertions(+), 92 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index b47dd0c6455fb..77bb34a134c68 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -6,6 +6,7 @@ linters: enable: - deadcode - depguard + - goconst - gofmt - golint - gosimple @@ -20,7 +21,6 @@ linters: # - dupl # - gochecknoglobals # - gochecknoinits - # - goconst # - gocyclo # - goimports # - gosec diff --git a/cli/cmd/endpoints.go b/cli/cmd/endpoints.go index 473fccf58dfde..fd400e0f78ebc 100644 --- a/cli/cmd/endpoints.go +++ b/cli/cmd/endpoints.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "encoding/json" - "errors" "fmt" "os" "sort" @@ -23,25 +22,24 @@ type endpointsOptions struct { outputFormat string } -var ( +const ( podHeader = "POD" ) // validate performs all validation on the command-line options. // It returns the first error encountered, or `nil` if the options are valid. func (o *endpointsOptions) validate() error { - switch o.outputFormat { - case "table", "json", "": + if o.outputFormat == tableOutput || o.outputFormat == jsonOutput { return nil } - return errors.New("--output currently only supports table and json") + return fmt.Errorf("--output currently only supports %s and %s", tableOutput, jsonOutput) } func newEndpointsOptions() *endpointsOptions { return &endpointsOptions{ namespace: "", - outputFormat: "", + outputFormat: tableOutput, } } @@ -89,7 +87,7 @@ requests.`, } cmd.PersistentFlags().StringVarP(&options.namespace, "namespace", "n", options.namespace, "Namespace of the specified endpoints (default: all namespaces)") - cmd.PersistentFlags().StringVarP(&options.outputFormat, "output", "o", options.outputFormat, "Output format; currently only \"table\" and \"json\" are supported (default \"table\")") + cmd.PersistentFlags().StringVarP(&options.outputFormat, "output", "o", options.outputFormat, fmt.Sprintf("Output format; one of: \"%s\" or \"%s\"", tableOutput, jsonOutput)) return cmd } @@ -166,13 +164,13 @@ func writeEndpointsToBuffer(endpoints *pb.EndpointsResponse, w *tabwriter.Writer } switch options.outputFormat { - case "table", "": + case tableOutput: if len(endpointsTables) == 0 { fmt.Fprintln(os.Stderr, "No endpoints found.") os.Exit(0) } printEndpointsTables(endpointsTables, w, options, maxPodLength, maxNamespaceLength) - case "json": + case jsonOutput: printEndpointsJSON(endpointsTables, w) } } diff --git a/cli/cmd/endpoints_test.go b/cli/cmd/endpoints_test.go index 02e6d16075ef1..58b2620d14b37 100644 --- a/cli/cmd/endpoints_test.go +++ b/cli/cmd/endpoints_test.go @@ -24,7 +24,7 @@ func TestEndpoints(t *testing.T) { }, t) }) - options.outputFormat = "json" + options.outputFormat = jsonOutput t.Run("Returns endpoints (json)", func(t *testing.T) { testEndpointsCall(endpointsExp{ options: options, @@ -42,7 +42,7 @@ func TestEndpoints(t *testing.T) { }, t) }) - options.outputFormat = "json" + options.outputFormat = jsonOutput t.Run("Returns all namespace endpoints (json)", func(t *testing.T) { testEndpointsCall(endpointsExp{ options: options, diff --git a/cli/cmd/inject_test.go b/cli/cmd/inject_test.go index 9270ce0411f3d..2620df0a10aa8 100644 --- a/cli/cmd/inject_test.go +++ b/cli/cmd/inject_test.go @@ -18,6 +18,13 @@ type injectYAML struct { testInjectOptions *injectOptions } +func mkFilename(filename string, verbose bool) string { + if verbose { + return fmt.Sprintf("%s.verbose", filename) + } + return filename +} + func testUninjectAndInject(t *testing.T, tc injectYAML) { file, err := os.Open("testdata/" + tc.inputFileName) if err != nil { @@ -34,10 +41,7 @@ func testUninjectAndInject(t *testing.T, tc injectYAML) { } diffTestdata(t, tc.goldenFileName, output.String()) - reportFileName := tc.reportFileName - if verbose { - reportFileName += ".verbose" - } + reportFileName := mkFilename(tc.reportFileName, verbose) diffTestdata(t, reportFileName, report.String()) } @@ -46,18 +50,18 @@ func TestUninjectAndInject(t *testing.T) { defaultOptions.linkerdVersion = "testinjectversion" tlsOptions := newInjectOptions() - tlsOptions.linkerdVersion = "testinjectversion" + tlsOptions.linkerdVersion = defaultOptions.linkerdVersion tlsOptions.tls = "optional" proxyResourceOptions := newInjectOptions() - proxyResourceOptions.linkerdVersion = "testinjectversion" + proxyResourceOptions.linkerdVersion = defaultOptions.linkerdVersion proxyResourceOptions.proxyCPURequest = "110m" proxyResourceOptions.proxyMemoryRequest = "100Mi" proxyResourceOptions.proxyCPULimit = "160m" proxyResourceOptions.proxyMemoryLimit = "150Mi" noInitContainerOptions := newInjectOptions() - noInitContainerOptions.linkerdVersion = "testinjectversion" + noInitContainerOptions.linkerdVersion = defaultOptions.linkerdVersion noInitContainerOptions.noInitContainer = true testCases := []injectYAML{ @@ -212,10 +216,7 @@ func testInjectCmd(t *testing.T, tc injectCmd) { t.Fatalf("Expected no standard output, but got: %s", outBuffer) } - stdErrGoldenFileName := tc.stdErrGoldenFileName - if verbose { - stdErrGoldenFileName += ".verbose" - } + stdErrGoldenFileName := mkFilename(tc.stdErrGoldenFileName, verbose) diffTestdata(t, stdErrGoldenFileName, errBuffer.String()) } @@ -266,10 +267,7 @@ func testInjectFilePath(t *testing.T, tc injectFilePath) { } diffTestdata(t, tc.expectedFile, actual.String()) - stdErrFile := tc.stdErrFile - if verbose { - stdErrFile += ".verbose" - } + stdErrFile := mkFilename(tc.stdErrFile, verbose) diffTestdata(t, stdErrFile, errBuf.String()) } @@ -288,10 +286,7 @@ func testReadFromFolder(t *testing.T, resourceFolder string, expectedFolder stri expectedFile := filepath.Join(expectedFolder, "injected_nginx_redis.yaml") diffTestdata(t, expectedFile, actual.String()) - stdErrFileName := filepath.Join(expectedFolder, "injected_nginx_redis.stderr") - if verbose { - stdErrFileName += ".verbose" - } + stdErrFileName := mkFilename(filepath.Join(expectedFolder, "injected_nginx_redis.stderr"), verbose) diffTestdata(t, stdErrFileName, errBuf.String()) } diff --git a/cli/cmd/install_test.go b/cli/cmd/install_test.go index 0ae3fec3ee4c7..9ab789ca9428e 100644 --- a/cli/cmd/install_test.go +++ b/cli/cmd/install_test.go @@ -112,7 +112,7 @@ func TestRender(t *testing.T) { haOptions := newInstallOptions() haOptions.highAvailability = true haConfig, _ := validateAndBuildConfig(haOptions) - haConfig.UUID = "deaab91a-f4ab-448a-b7d1-c832a2fa0a60" + haConfig.UUID = defaultConfig.UUID haWithOverridesOptions := newInstallOptions() haWithOverridesOptions.highAvailability = true @@ -120,19 +120,19 @@ func TestRender(t *testing.T) { haWithOverridesOptions.proxyCPURequest = "400m" haWithOverridesOptions.proxyMemoryRequest = "300Mi" haWithOverridesConfig, _ := validateAndBuildConfig(haWithOverridesOptions) - haWithOverridesConfig.UUID = "deaab91a-f4ab-448a-b7d1-c832a2fa0a60" + haWithOverridesConfig.UUID = defaultConfig.UUID noInitContainerOptions := newInstallOptions() noInitContainerOptions.noInitContainer = true noInitContainerConfig, _ := validateAndBuildConfig(noInitContainerOptions) - noInitContainerConfig.UUID = "deaab91a-f4ab-448a-b7d1-c832a2fa0a60" + noInitContainerConfig.UUID = defaultConfig.UUID noInitContainerWithProxyAutoInjectOptions := newInstallOptions() noInitContainerWithProxyAutoInjectOptions.noInitContainer = true noInitContainerWithProxyAutoInjectOptions.proxyAutoInject = true noInitContainerWithProxyAutoInjectOptions.tls = "optional" noInitContainerWithProxyAutoInjectConfig, _ := validateAndBuildConfig(noInitContainerWithProxyAutoInjectOptions) - noInitContainerWithProxyAutoInjectConfig.UUID = "deaab91a-f4ab-448a-b7d1-c832a2fa0a60" + noInitContainerWithProxyAutoInjectConfig.UUID = defaultConfig.UUID testCases := []struct { config installConfig diff --git a/cli/cmd/profile_test.go b/cli/cmd/profile_test.go index b570a554029cc..dd02b2d3416f7 100644 --- a/cli/cmd/profile_test.go +++ b/cli/cmd/profile_test.go @@ -101,9 +101,11 @@ func TestValidateOptions(t *testing.T) { t.Fatalf("validateOptions returned unexpected error: %s (expected: %s) for options: %+v", err, exp, options) } + serviceName := "service-name" + options = newProfileOptions() options.template = true - options.name = "service-name" + options.name = serviceName options.namespace = "" exp = fmt.Errorf("invalid namespace \"%s\": [a DNS-1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')]", options.namespace) err = options.validate() @@ -113,7 +115,7 @@ func TestValidateOptions(t *testing.T) { options = newProfileOptions() options.template = true - options.name = "service-name" + options.name = serviceName options.namespace = "invalid/namespace" exp = fmt.Errorf("invalid namespace \"%s\": [a DNS-1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')]", options.namespace) err = options.validate() @@ -123,7 +125,7 @@ func TestValidateOptions(t *testing.T) { options = newProfileOptions() options.template = true - options.name = "service-name" + options.name = serviceName options.namespace = "7eet-ns" err = options.validate() if err != nil { diff --git a/cli/cmd/root.go b/cli/cmd/root.go index 1b36e41062d19..67098aba0f7aa 100644 --- a/cli/cmd/root.go +++ b/cli/cmd/root.go @@ -2,7 +2,6 @@ package cmd import ( "bytes" - "errors" "fmt" "os" "regexp" @@ -21,6 +20,10 @@ import ( const ( defaultNamespace = "linkerd" + + jsonOutput = "json" + tableOutput = "table" + wideOutput = "wide" ) var ( @@ -182,23 +185,23 @@ func newStatOptionsBase() *statOptionsBase { return &statOptionsBase{ namespace: "default", timeWindow: "1m", - outputFormat: "", + outputFormat: tableOutput, } } func (o *statOptionsBase) validateOutputFormat() error { switch o.outputFormat { - case "table", "json", "": + case tableOutput, jsonOutput: return nil default: - return errors.New("--output currently only supports table and json") + return fmt.Errorf("--output currently only supports %s and %s", tableOutput, jsonOutput) } } func renderStats(buffer bytes.Buffer, options *statOptionsBase) string { var out string switch options.outputFormat { - case "json": + case jsonOutput: out = buffer.String() default: // strip left padding on the first column diff --git a/cli/cmd/routes.go b/cli/cmd/routes.go index 76e8d6e448bfa..2f90a6c89a4fd 100644 --- a/cli/cmd/routes.go +++ b/cli/cmd/routes.go @@ -75,7 +75,7 @@ This command will only display traffic which is sent to a service that has a Ser cmd.PersistentFlags().StringVarP(&options.timeWindow, "time-window", "t", options.timeWindow, "Stat window (for example: \"10s\", \"1m\", \"10m\", \"1h\")") cmd.PersistentFlags().StringVar(&options.toResource, "to", options.toResource, "If present, shows outbound stats to the specified resource") cmd.PersistentFlags().StringVar(&options.toNamespace, "to-namespace", options.toNamespace, "Sets the namespace used to lookup the \"--to\" resource; by default the current \"--namespace\" is used") - cmd.PersistentFlags().StringVarP(&options.outputFormat, "output", "o", options.outputFormat, "Output format; currently only \"table\" (default), \"wide\", and \"json\" are supported") + cmd.PersistentFlags().StringVarP(&options.outputFormat, "output", "o", options.outputFormat, fmt.Sprintf("Output format; one of: \"%s\", \"%s\", or \"%s\"", tableOutput, wideOutput, jsonOutput)) return cmd } @@ -142,7 +142,7 @@ func writeRouteStatsToBuffer(resp *pb.TopRoutesResponse, w *tabwriter.Writer, op sort.Strings(resources) switch options.outputFormat { - case "table", "wide", "": + case tableOutput, wideOutput: for _, resource := range resources { if len(tables) > 1 { fmt.Fprintf(w, "==> %s <==\t\f", resource) @@ -150,7 +150,7 @@ func writeRouteStatsToBuffer(resp *pb.TopRoutesResponse, w *tabwriter.Writer, op printRouteTable(tables[resource], w, options) fmt.Fprintln(w) } - case "json": + case jsonOutput: printRouteJSON(tables, w, options) } } @@ -168,7 +168,7 @@ func printRouteTable(stats []*routeRowStats, w *tabwriter.Writer, options *route fmt.Sprintf(routeTemplate, "ROUTE"), authorityColumn, } - outputActual := options.toResource != "" && options.outputFormat == "wide" + outputActual := options.toResource != "" && options.outputFormat == wideOutput if outputActual { headers = append(headers, []string{ "EFFECTIVE_SUCCESS", @@ -276,15 +276,15 @@ func printRouteJSON(tables map[string][]*routeRowStats, w *tabwriter.Writer, opt func (o *routesOptions) validateOutputFormat() error { switch o.outputFormat { - case "table", "json", "": + case tableOutput, jsonOutput: return nil - case "wide": + case wideOutput: if o.toResource == "" { - return errors.New("wide output is only available when --to is specified") + return fmt.Errorf("%s output is only available when --to is specified", wideOutput) } return nil default: - return fmt.Errorf("--output currently only supports table, wide, and json") + return fmt.Errorf("--output currently only supports %s, %s, and %s", tableOutput, wideOutput, jsonOutput) } } diff --git a/cli/cmd/routes_test.go b/cli/cmd/routes_test.go index 0d0aa07fb22f6..19e55ad25307e 100644 --- a/cli/cmd/routes_test.go +++ b/cli/cmd/routes_test.go @@ -24,7 +24,7 @@ func TestRoutes(t *testing.T) { }, t) }) - options.outputFormat = "json" + options.outputFormat = jsonOutput t.Run("Returns route stats (json)", func(t *testing.T) { testRoutesCall(routesParamsExp{ routes: []string{"/a", "/b", "/c"}, diff --git a/cli/cmd/stat.go b/cli/cmd/stat.go index 416cf547e44ca..a83febc4637a8 100644 --- a/cli/cmd/stat.go +++ b/cli/cmd/stat.go @@ -162,7 +162,7 @@ If no resource name is specified, displays stats about all resources of the spec cmd.PersistentFlags().StringVar(&options.fromResource, "from", options.fromResource, "If present, restricts outbound stats from the specified resource name") cmd.PersistentFlags().StringVar(&options.fromNamespace, "from-namespace", options.fromNamespace, "Sets the namespace used from lookup the \"--from\" resource; by default the current \"--namespace\" is used") cmd.PersistentFlags().BoolVar(&options.allNamespaces, "all-namespaces", options.allNamespaces, "If present, returns stats across all namespaces, ignoring the \"--namespace\" flag") - cmd.PersistentFlags().StringVarP(&options.outputFormat, "output", "o", options.outputFormat, "Output format; currently only \"table\" (default) and \"json\" are supported") + cmd.PersistentFlags().StringVarP(&options.outputFormat, "output", "o", options.outputFormat, "Output format; one of: \"table\" or \"json\"") return cmd } @@ -279,13 +279,13 @@ func writeStatsToBuffer(rows []*pb.StatTable_PodGroup_Row, w *tabwriter.Writer, } switch options.outputFormat { - case "table", "wide", "": + case tableOutput, wideOutput: if len(statTables) == 0 { fmt.Fprintln(os.Stderr, "No traffic found.") os.Exit(0) } printStatTables(statTables, w, maxNameLength, maxNamespaceLength, options) - case "json": + case jsonOutput: printStatJSON(statTables, w) } } diff --git a/cli/cmd/stat_test.go b/cli/cmd/stat_test.go index faf796566793d..7f4b7c6ee6f5b 100644 --- a/cli/cmd/stat_test.go +++ b/cli/cmd/stat_test.go @@ -29,7 +29,7 @@ func TestStat(t *testing.T) { }, t) }) - options.outputFormat = "json" + options.outputFormat = jsonOutput t.Run("Returns namespace stats (json)", func(t *testing.T) { testStatCall(paramsExp{ counts: &public.PodCounts{ @@ -58,7 +58,7 @@ func TestStat(t *testing.T) { }, t) }) - options.outputFormat = "json" + options.outputFormat = jsonOutput t.Run("Returns all namespace stats (json)", func(t *testing.T) { testStatCall(paramsExp{ counts: &public.PodCounts{ diff --git a/cli/cmd/tap.go b/cli/cmd/tap.go index 333277d592a92..0432a6a622eb6 100644 --- a/cli/cmd/tap.go +++ b/cli/cmd/tap.go @@ -105,7 +105,7 @@ func newCmdTap() *cobra.Command { // TODO: support more output formats? case "": // default output format. - case "wide": + case wideOutput: wide = true default: return fmt.Errorf("output format \"%s\" not recognized", options.output) diff --git a/cli/cmd/tap_test.go b/cli/cmd/tap_test.go index fe7023a21928b..a839dd02f604d 100644 --- a/cli/cmd/tap_test.go +++ b/cli/cmd/tap_test.go @@ -16,9 +16,10 @@ import ( "google.golang.org/grpc/codes" ) +const targetName = "pod-666" + func busyTest(t *testing.T, wide bool) { resourceType := k8s.Pod - targetName := "pod-666" params := util.TapRequestParams{ Resource: resourceType + "/" + targetName, Scheme: "https", @@ -113,7 +114,6 @@ func TestRequestTapByResourceFromAPI(t *testing.T) { t.Run("Should render empty response if no events returned", func(t *testing.T) { resourceType := k8s.Pod - targetName := "pod-666" params := util.TapRequestParams{ Resource: resourceType + "/" + targetName, Scheme: "https", @@ -152,7 +152,6 @@ func TestRequestTapByResourceFromAPI(t *testing.T) { t.Run("Should return error if stream returned error", func(t *testing.T) { t.SkipNow() resourceType := k8s.Pod - targetName := "pod-666" params := util.TapRequestParams{ Resource: resourceType + "/" + targetName, Scheme: "https", diff --git a/cni-plugin/test/install-cni_test.go b/cni-plugin/test/install-cni_test.go index 9791ba68e9e18..f0c5d5fddd56d 100644 --- a/cni-plugin/test/install-cni_test.go +++ b/cni-plugin/test/install-cni_test.go @@ -36,6 +36,8 @@ const ( cniConfName = "CNI_CONF_NAME" cniNetworkConfigName = "CNI_NETWORK_CONFIG" + + testWd = "/tmp" ) func env(key, fallback string) string { @@ -281,7 +283,6 @@ func TestInstallCNI_Scenario1(t *testing.T) { wd := pwd(t) t.Logf("..setting the working directory: %v", wd) - testWd := "/tmp" t.Logf("..setting the test working directory: %v", testWd) testCNINetDir := mktemp(testWd, "linkerd-cni-confXXXXX", t) t.Logf("..creating the test CNI_NET_DIR: %v", testCNINetDir) @@ -311,7 +312,6 @@ func TestInstallCNI_Scenario2(t *testing.T) { wd := pwd(t) t.Logf("..setting the working directory: %v", wd) - testWd := "/tmp" t.Logf("..setting the test working directory: %v", testWd) testCNINetDir := mktemp(testWd, "linkerd-cni-confXXXXX", t) t.Logf("..creating the test CNI_NET_DIR: %v", testCNINetDir) @@ -342,7 +342,6 @@ func TestInstallCNI_Scenario3(t *testing.T) { wd := pwd(t) t.Logf("..setting the working directory: %v", wd) - testWd := "/tmp" t.Logf("..setting the test working directory: %v", testWd) testCNINetDir := mktemp(testWd, "linkerd-cni-confXXXXX", t) t.Logf("..creating the test CNI_NET_DIR: %v", testCNINetDir) diff --git a/controller/api/destination/endpoint_listener_test.go b/controller/api/destination/endpoint_listener_test.go index 698111bfffef6..9b636cfc9735f 100644 --- a/controller/api/destination/endpoint_listener_test.go +++ b/controller/api/destination/endpoint_listener_test.go @@ -13,6 +13,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const thisNS = "this-namespace" + var ( addedAddress1 = &net.TcpAddress{ Ip: &net.IPAddress{Ip: &net.IPAddress_Ipv4{Ipv4: 1}}, @@ -143,8 +145,8 @@ func TestEndpointListener(t *testing.T) { t.Run("Sends metric labels with added addresses", func(t *testing.T) { expectedServiceName := "service-name" - expectedPodName := "pod1" - expectedNamespace := "this-namespace" + expectedPodName := pod1.Name + expectedNamespace := thisNS expectedReplicationControllerName := "rc-name" podForAddedAddress1 := &v1.Pod{ @@ -195,8 +197,8 @@ func TestEndpointListener(t *testing.T) { }) t.Run("Sends TlsIdentity when enabled", func(t *testing.T) { - expectedPodName := "pod1" - expectedPodNamespace := "this-namespace" + expectedPodName := pod1.Name + expectedPodNamespace := thisNS expectedControllerNamespace := "linkerd-namespace" expectedPodDeployment := "pod-deployment" expectedTLSIdentity := &pb.TlsIdentity_K8SPodIdentity{ @@ -247,8 +249,8 @@ func TestEndpointListener(t *testing.T) { }) t.Run("Does not send TlsIdentity when not enabled", func(t *testing.T) { - expectedPodName := "pod1" - expectedPodNamespace := "this-namespace" + expectedPodName := pod1.Name + expectedPodNamespace := thisNS expectedControllerNamespace := "linkerd-namespace" expectedPodDeployment := "pod-deployment" diff --git a/controller/api/public/stat_summary.go b/controller/api/public/stat_summary.go index bef78b141d842..efc8079194b9a 100644 --- a/controller/api/public/stat_summary.go +++ b/controller/api/public/stat_summary.go @@ -31,6 +31,9 @@ type rKey struct { } const ( + success = "success" + failure = "failure" + reqQuery = "sum(increase(response_total%s[%s])) by (%s, classification, tls)" latencyQuantileQuery = "histogram_quantile(%s, sum(irate(response_latency_ms_bucket%s[%s])) by (le, %s))" ) @@ -334,9 +337,9 @@ func processPrometheusMetrics(req *pb.StatSummaryRequest, results []promResult, switch result.prom { case promRequests: switch string(sample.Metric[model.LabelName("classification")]) { - case "success": + case success: basicStats[resource].SuccessCount += value - case "failure": + case failure: basicStats[resource].FailureCount += value } switch string(sample.Metric[model.LabelName("tls")]) { diff --git a/controller/api/public/stat_summary_test.go b/controller/api/public/stat_summary_test.go index 93c43993413eb..7ec769714bbbf 100644 --- a/controller/api/public/stat_summary_test.go +++ b/controller/api/public/stat_summary_test.go @@ -185,7 +185,7 @@ status: phase: Completed `, }, - mockPromResponse: prometheusMetric("emoji", "deployment", "emojivoto", "success", false), + mockPromResponse: prometheusMetric("emoji", "deployment", "emojivoto", success, false), }, req: pb.StatSummaryRequest{ Selector: &pb.ResourceSelection{ @@ -261,7 +261,7 @@ status: phase: Completed `, }, - mockPromResponse: prometheusMetric("emoji", "daemonset", "emojivoto", "success", false), + mockPromResponse: prometheusMetric("emoji", "daemonset", "emojivoto", success, false), }, req: pb.StatSummaryRequest{ Selector: &pb.ResourceSelection{ @@ -356,7 +356,7 @@ status: phase: Running `, }, - mockPromResponse: prometheusMetric("redis", "statefulset", "emojivoto", "success", false), + mockPromResponse: prometheusMetric("redis", "statefulset", "emojivoto", success, false), }, req: pb.StatSummaryRequest{ Selector: &pb.ResourceSelection{ @@ -396,7 +396,7 @@ status: phase: Running `, }, - mockPromResponse: prometheusMetric("emojivoto-1", "pod", "emojivoto", "success", false), + mockPromResponse: prometheusMetric("emojivoto-1", "pod", "emojivoto", success, false), expectedPrometheusQueries: []string{ `histogram_quantile(0.5, sum(irate(response_latency_ms_bucket{direction="inbound", namespace="emojivoto", pod="emojivoto-1"}[1m])) by (le, namespace, pod))`, `histogram_quantile(0.95, sum(irate(response_latency_ms_bucket{direction="inbound", namespace="emojivoto", pod="emojivoto-1"}[1m])) by (le, namespace, pod))`, @@ -443,7 +443,7 @@ status: phase: Running `, }, - mockPromResponse: prometheusMetric("emojivoto-2", "pod", "emojivoto", "success", false), + mockPromResponse: prometheusMetric("emojivoto-2", "pod", "emojivoto", success, false), expectedPrometheusQueries: []string{ `histogram_quantile(0.5, sum(irate(response_latency_ms_bucket{direction="outbound", dst_namespace="emojivoto", dst_pod="emojivoto-1", namespace="emojivoto", pod="emojivoto-2"}[1m])) by (le, dst_namespace, dst_pod))`, `histogram_quantile(0.95, sum(irate(response_latency_ms_bucket{direction="outbound", dst_namespace="emojivoto", dst_pod="emojivoto-1", namespace="emojivoto", pod="emojivoto-2"}[1m])) by (le, dst_namespace, dst_pod))`, @@ -494,7 +494,7 @@ status: `, }, mockPromResponse: model.Vector{ - genPromSample("emojivoto-1", "pod", "emojivoto", "success", false), + genPromSample("emojivoto-1", "pod", "emojivoto", success, false), }, expectedPrometheusQueries: []string{ `histogram_quantile(0.5, sum(irate(response_latency_ms_bucket{direction="outbound", dst_namespace="emojivoto", dst_pod="emojivoto-2", namespace="emojivoto", pod="emojivoto-1"}[1m])) by (le, namespace, pod))`, @@ -550,7 +550,7 @@ status: `, }, mockPromResponse: model.Vector{ - genPromSample("emojivoto-1", "pod", "emojivoto", "success", false), + genPromSample("emojivoto-1", "pod", "emojivoto", success, false), }, expectedPrometheusQueries: []string{ `histogram_quantile(0.5, sum(irate(response_latency_ms_bucket{direction="outbound", dst_namespace="totallydifferent", dst_pod="emojivoto-2", namespace="emojivoto", pod="emojivoto-1"}[1m])) by (le, namespace, pod))`, @@ -617,7 +617,7 @@ status: `, }, mockPromResponse: model.Vector{ - genPromSample("emojivoto-1", "pod", "emojivoto", "success", true), + genPromSample("emojivoto-1", "pod", "emojivoto", success, true), }, expectedPrometheusQueries: []string{ `histogram_quantile(0.5, sum(irate(response_latency_ms_bucket{direction="outbound", pod="emojivoto-2"}[1m])) by (le, dst_namespace, dst_pod))`, @@ -684,7 +684,7 @@ status: `, }, mockPromResponse: model.Vector{ - genPromSample("emojivoto-1", "pod", "emojivoto", "success", true), + genPromSample("emojivoto-1", "pod", "emojivoto", success, true), }, expectedPrometheusQueries: []string{ `histogram_quantile(0.5, sum(irate(response_latency_ms_bucket{direction="outbound", dst_namespace="emojivoto", dst_pod="emojivoto-1", namespace="totallydifferent", pod="emojivoto-2"}[1m])) by (le, dst_namespace, dst_pod))`, @@ -775,7 +775,7 @@ status: phase: Running `, }, - mockPromResponse: prometheusMetric("emoji-deploy", "deployment", "emojivoto", "success", false), + mockPromResponse: prometheusMetric("emoji-deploy", "deployment", "emojivoto", success, false), }, req: pb.StatSummaryRequest{ Selector: &pb.ResourceSelection{ @@ -1196,7 +1196,7 @@ metadata: status: phase: Succeeded `}, - mockPromResponse: prometheusMetric("emoji", "deployment", "emojivoto", "success", false), + mockPromResponse: prometheusMetric("emoji", "deployment", "emojivoto", success, false), }, req: pb.StatSummaryRequest{ Selector: &pb.ResourceSelection{ @@ -1238,7 +1238,7 @@ status: `, }, mockPromResponse: model.Vector{ - genPromSample("10.1.1.239:9995", "authority", "linkerd", "success", false), + genPromSample("10.1.1.239:9995", "authority", "linkerd", success, false), }, expectedPrometheusQueries: []string{ `histogram_quantile(0.5, sum(irate(response_latency_ms_bucket{direction="inbound", namespace="linkerd"}[1m])) by (le, namespace, authority))`, @@ -1282,7 +1282,7 @@ status: `, }, mockPromResponse: model.Vector{ - genPromSample("10.1.1.239:9995", "authority", "linkerd", "success", false), + genPromSample("10.1.1.239:9995", "authority", "linkerd", success, false), }, expectedPrometheusQueries: []string{ `histogram_quantile(0.5, sum(irate(response_latency_ms_bucket{deployment="emojivoto", direction="outbound"}[1m])) by (le, dst_namespace, authority))`, @@ -1333,7 +1333,7 @@ status: `, }, mockPromResponse: model.Vector{ - genPromSample("10.1.1.239:9995", "authority", "linkerd", "success", false), + genPromSample("10.1.1.239:9995", "authority", "linkerd", success, false), }, expectedPrometheusQueries: []string{ `histogram_quantile(0.5, sum(irate(response_latency_ms_bucket{authority="10.1.1.239:9995", direction="inbound", namespace="linkerd"}[1m])) by (le, namespace, authority))`, diff --git a/controller/api/public/top_routes.go b/controller/api/public/top_routes.go index 7dadc41f428b7..e10ebb6204420 100644 --- a/controller/api/public/top_routes.go +++ b/controller/api/public/top_routes.go @@ -337,16 +337,16 @@ func processRouteMetrics(results []promResult, timeWindow string, table indexedT switch result.prom { case promRequests: switch string(sample.Metric[model.LabelName("classification")]) { - case "success": + case success: table[key].Stats.SuccessCount += value - case "failure": + case failure: table[key].Stats.FailureCount += value } case promActualRequests: switch string(sample.Metric[model.LabelName("classification")]) { - case "success": + case success: table[key].Stats.ActualSuccessCount += value - case "failure": + case failure: table[key].Stats.ActualFailureCount += value } case promLatencyP50: diff --git a/controller/api/public/top_routes_test.go b/controller/api/public/top_routes_test.go index 732190173b693..7774e12fdf344 100644 --- a/controller/api/public/top_routes_test.go +++ b/controller/api/public/top_routes_test.go @@ -145,7 +145,7 @@ func genRouteSample(route string) *model.Sample { Metric: model.Metric{ "rt_route": model.LabelValue(route), "dst": "books.default.svc.cluster.local", - "classification": "success", + "classification": success, }, Value: 123, Timestamp: 456, @@ -156,7 +156,7 @@ func genDefaultRouteSample() *model.Sample { return &model.Sample{ Metric: model.Metric{ "dst": "books.default.svc.cluster.local", - "classification": "success", + "classification": success, }, Value: 123, Timestamp: 456,