Skip to content

Forward opaque metrics selectors to the API #1387

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
no-itsbackpack merged 1 commit into from
Aug 26, 2026
Merged

Forward opaque metrics selectors to the API #1387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 31 additions & 33 deletions internal/cmd/metrics/metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,22 @@ func TestShowCmd_JSONPreservesSeries(t *testing.T) {
c.Assert(response.Series[0].Points, qt.HasLen, 3)
}

func TestShowCmd_ForwardsOpaqueQueryID(t *testing.T) {
c := qt.New(t)
service := &mock.MetricsService{
GetSeriesFn: func(ctx context.Context, req *ps.GetMetricSeriesRequest) (*ps.MetricSeries, error) {
c.Assert(req.QueryIDs, qt.DeepEquals, []string{"59801dae501c"})
return sampleSeries(), nil
},
}

var buf bytes.Buffer
cmd := ShowCmd(metricsTestHelper(&buf, printer.JSON, &ps.Client{Metrics: service}))
cmd.SetArgs([]string{"mydb", "main", "--metric", "queries", "--query-id", "59801dae501c"})
c.Assert(cmd.Execute(), qt.IsNil)
c.Assert(service.GetSeriesFnInvoked, qt.IsTrue)
}

func TestShowCmd_HumanSummarizesSeries(t *testing.T) {
c := qt.New(t)
service := &mock.MetricsService{
Expand Down Expand Up @@ -317,16 +333,6 @@ func TestStorageMetricsCommands_PreserveResponse(t *testing.T) {

func TestTabletsCmd_ForwardsSupportedFilters(t *testing.T) {
c := qt.New(t)
workflows := &mock.WorkflowsService{
ListFn: func(ctx context.Context, req *ps.ListWorkflowsRequest) ([]*ps.Workflow, error) {
c.Assert(req.Database, qt.Equals, "mydb")
return []*ps.Workflow{{
ID: "opaque-workflow-id",
Name: "move-tables",
Branch: ps.DatabaseBranch{Name: "main"},
}}, nil
},
}
service := &mock.MetricsService{
GetTabletSeriesFn: func(ctx context.Context, req *ps.GetTabletMetricSeriesRequest) (*ps.MetricSeries, error) {
c.Assert(req.Metrics, qt.DeepEquals, []string{"replication_lag", "pod_cpu_usage", "vreplication_lag"})
Expand All @@ -342,7 +348,7 @@ func TestTabletsCmd_ForwardsSupportedFilters(t *testing.T) {
}

var buf bytes.Buffer
cmd := TabletsCmd(metricsTestHelper(&buf, printer.JSON, &ps.Client{Metrics: service, Workflows: workflows}))
cmd := TabletsCmd(metricsTestHelper(&buf, printer.JSON, &ps.Client{Metrics: service}))
cmd.SetArgs([]string{
"mydb", "main",
"--metric", "replication_lag,pod_cpu_usage,vreplication_lag",
Expand All @@ -356,7 +362,6 @@ func TestTabletsCmd_ForwardsSupportedFilters(t *testing.T) {
})
c.Assert(cmd.Execute(), qt.IsNil)
c.Assert(service.GetTabletSeriesFnInvoked, qt.IsTrue)
c.Assert(workflows.ListFnInvoked, qt.IsTrue)
}

func TestTabletsInstantCmd_UsesNestedUX(t *testing.T) {
Expand All @@ -382,31 +387,25 @@ func TestTabletsInstantCmd_UsesNestedUX(t *testing.T) {
c.Assert(service.GetInstantTabletsFnInvoked, qt.IsTrue)
}

func TestTabletsCmd_RejectsUnknownWorkflow(t *testing.T) {
func TestTabletsCmd_DoesNotPrevalidateWorkflow(t *testing.T) {
c := qt.New(t)
workflows := &mock.WorkflowsService{
ListFn: func(context.Context, *ps.ListWorkflowsRequest) ([]*ps.Workflow, error) {
return []*ps.Workflow{}, nil
},
}
service := &mock.MetricsService{
GetTabletSeriesFn: func(context.Context, *ps.GetTabletMetricSeriesRequest) (*ps.MetricSeries, error) {
c.Fatal("Metrics.GetTabletSeries should not be called")
return nil, nil
GetTabletSeriesFn: func(_ context.Context, req *ps.GetTabletMetricSeriesRequest) (*ps.MetricSeries, error) {
c.Assert(req.Workflow, qt.Equals, "workflow-on-a-later-page")
return sampleSeries(), nil
},
}

cmd := TabletsCmd(metricsTestHelper(&bytes.Buffer{}, printer.JSON, &ps.Client{
Metrics: service,
Workflows: workflows,
Metrics: service,
}))
cmd.SetArgs([]string{
"mydb", "main",
"--metric", "vreplication_lag",
"--workflow", "missing",
"--workflow", "workflow-on-a-later-page",
})
c.Assert(cmd.Execute(), qt.ErrorMatches, "workflow missing does not exist on branch main")
c.Assert(service.GetTabletSeriesFnInvoked, qt.IsFalse)
c.Assert(cmd.Execute(), qt.IsNil)
c.Assert(service.GetTabletSeriesFnInvoked, qt.IsTrue)
}

func TestTagsCmd_ForwardsSupportedFilters(t *testing.T) {
Expand Down Expand Up @@ -444,21 +443,20 @@ func TestTagsCmd_ForwardsSupportedFilters(t *testing.T) {
c.Assert(service.GetTagSeriesFnInvoked, qt.IsTrue)
}

func TestQueriesCmd_RejectsShortQueryID(t *testing.T) {
func TestQueriesCmd_ForwardsOpaqueQueryID(t *testing.T) {
c := qt.New(t)
service := &mock.MetricsService{
GetQuerySeriesFn: func(ctx context.Context, req *ps.GetQueryMetricSeriesRequest) (*ps.MetricSeries, error) {
c.Fatal("Metrics.GetQuerySeries should not be called")
return nil, nil
c.Assert(req.QueryIDs, qt.DeepEquals, []string{"59801dae501c"})
return sampleSeries(), nil
},
}

var buf bytes.Buffer
cmd := QueriesCmd(metricsTestHelper(&buf, printer.JSON, &ps.Client{Metrics: service}))
cmd.SetArgs([]string{"mydb", "main", "--metric", "queries", "--query-id", "59801dae501c"})
err := cmd.Execute()
c.Assert(err, qt.IsNotNil)
c.Assert(err.Error(), qt.Contains, `invalid --query-id "59801dae501c"; expected <fingerprint>-<keyspace>`)
c.Assert(cmd.Execute(), qt.IsNil)
c.Assert(service.GetQuerySeriesFnInvoked, qt.IsTrue)
}

func TestParseTagSets(t *testing.T) {
Expand All @@ -480,7 +478,7 @@ func TestParseTagSets(t *testing.T) {

func TestValidateQuerySelector(t *testing.T) {
c := qt.New(t)
validID := strings.Repeat("a", 64) + "-commerce"
validID := "59801dae501c"

c.Assert(validateQuerySelector([]string{validID}, "", ""), qt.IsNil)
c.Assert(validateQuerySelector(nil, "fingerprint", "commerce"), qt.IsNil)
Expand Down
5 changes: 1 addition & 4 deletions internal/cmd/metrics/show.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,6 @@ func ShowCmd(ch *cmdutil.Helper) *cobra.Command {
if cmd.Flags().Changed("steps") && flags.steps <= 0 {
return fmt.Errorf("--steps must be greater than zero")
}
if err := validateQueryIDs(flags.queryIDs); err != nil {
return err
}

client, err := ch.Client()
if err != nil {
Expand Down Expand Up @@ -115,7 +112,7 @@ func ShowCmd(ch *cmdutil.Helper) *cobra.Command {
cmd.Flags().StringVar(&flags.container, "container", "", "Filter by container")
cmd.Flags().StringVar(&flags.pod, "pod", "", "Filter by one pod")
cmd.Flags().StringSliceVar(&flags.pods, "pods", nil, "Filter by pods (repeat or comma-separate)")
cmd.Flags().StringSliceVar(&flags.queryIDs, "query-id", nil, "Filter by query pattern ID as <fingerprint>-<keyspace> (repeat or comma-separate)")
cmd.Flags().StringSliceVar(&flags.queryIDs, "query-id", nil, "Filter by query pattern ID (repeat or comma-separate)")
cmd.Flags().StringVar(&flags.fingerprint, "fingerprint", "", "Filter by query fingerprint")
cmd.Flags().StringVar(&flags.budgetID, "budget-id", "", "Filter by traffic budget ID")
cmd.Flags().StringVar(&flags.ruleID, "rule-id", "", "Filter by traffic rule ID")
Expand Down
41 changes: 1 addition & 40 deletions internal/cmd/metrics/specialized.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package metrics

import (
"fmt"
"regexp"
"strings"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -44,9 +43,6 @@ func QueriesCmd(ch *cmdutil.Helper) *cobra.Command {
if err := validateSpecializedSeriesFlags(cmd, flags.specializedSeriesFlags); err != nil {
return err
}
if err := validateQueryIDs(flags.queryIDs); err != nil {
return err
}
if err := validateQuerySelector(flags.queryIDs, flags.fingerprint, flags.keyspace); err != nil {
return err
}
Expand Down Expand Up @@ -91,7 +87,7 @@ func QueriesCmd(ch *cmdutil.Helper) *cobra.Command {

addSpecializedSeriesFlags(cmd, &flags.specializedSeriesFlags)
addQueryDimensionFlags(cmd, &flags.queryDimensionFlags)
cmd.Flags().StringSliceVar(&flags.queryIDs, "query-id", nil, "Filter by query pattern ID as <fingerprint>-<keyspace> (repeat or comma-separate)")
cmd.Flags().StringSliceVar(&flags.queryIDs, "query-id", nil, "Filter by query pattern ID (repeat or comma-separate)")
cmd.Flags().StringVar(&flags.fingerprint, "fingerprint", "", "Filter by query fingerprint")
cmd.Flags().StringVar(&flags.keyspace, "keyspace", "", "Keyspace for the query fingerprint")
cmd.MarkFlagRequired("metric") // nolint:errcheck
Expand Down Expand Up @@ -138,27 +134,6 @@ func TabletsCmd(ch *cmdutil.Helper) *cobra.Command {
}

database, branch := args[0], args[1]
if flags.workflow != "" {
workflows, err := client.Workflows.List(cmd.Context(), &ps.ListWorkflowsRequest{
Organization: ch.Config.Organization,
Database: database,
})
if err != nil {
return cmdutil.HandleError(err)
}

found := false
for _, workflow := range workflows {
if workflow.Name == flags.workflow && workflow.Branch.Name == branch {
found = true
break
}
}
if !found {
return fmt.Errorf("workflow %s does not exist on branch %s", printer.BoldBlue(flags.workflow), printer.BoldBlue(branch))
}
}

end := specializedMetricsProgress(ch, database, branch, "tablet")
defer end()

Expand Down Expand Up @@ -316,20 +291,6 @@ func TagsCmd(ch *cmdutil.Helper) *cobra.Command {
return cmd
}

// queryPatternIDPattern matches the API's query pattern ID: a 64 character
// fingerprint joined to the keyspace by a dash. Anything else is silently
// ignored by the API, which returns an empty series instead of an error.
var queryPatternIDPattern = regexp.MustCompile(`^[0-9a-fA-F]{64}-.+$`)

func validateQueryIDs(ids []string) error {
for _, id := range ids {
if !queryPatternIDPattern.MatchString(id) {
return fmt.Errorf("invalid --query-id %q; expected <fingerprint>-<keyspace>, for example %s-mykeyspace. Use --fingerprint and --keyspace instead, or take both from `pscale insights queries`", id, strings.Repeat("a", 64))
}
}
return nil
}

func validateQuerySelector(ids []string, fingerprint, keyspace string) error {
hasIDs := len(ids) > 0
hasFingerprint := fingerprint != ""
Expand Down