[bugfix] Make sure that the interface selector is set on args.Prepare()

pkg/goDB/DBWorkManager.go

@@ -30,8 +30,11 @@ import (
 	"github.com/els0r/goProbe/pkg/types"
 	"github.com/els0r/goProbe/pkg/types/hashmap"
 	"github.com/els0r/telemetry/logging"
+	"github.com/els0r/telemetry/tracing"
 	"github.com/fako1024/gotools/bitpack"
 	"github.com/fako1024/gotools/concurrency"
+	"go.opentelemetry.io/otel/attribute"
+	"go.opentelemetry.io/otel/trace"
 )
 
 const (
@@ -510,6 +513,13 @@ func (w *DBWorkManager) grabAndProcessWorkload(ctx context.Context, wg *sync.Wai
 
 // ExecuteWorkerReadJobs runs the query concurrently with multiple sprocessing units
 func (w *DBWorkManager) ExecuteWorkerReadJobs(ctx context.Context, mapChan chan hashmap.AggFlowMapWithMetadata) {
+	// measure how long it takes to process a single interface. Performance-wise, this is agreable, as
+	// the hot path is within grabAndProcessWorkload
+	// The benefit: more insight on long-running multi-interface queries (a.k.a "any")
+	ctx, span := tracing.Start(ctx, "(*goDB.DBWorkManager).ExecuteWorkerReadJobs",
+		trace.WithAttributes(attribute.String("iface", w.iface)),
+	)
+	defer span.End()
 
 	var wg = new(sync.WaitGroup)
 	wg.Add(w.numProcessingUnits)

pkg/goDB/engine/query.go

@@ -5,11 +5,9 @@ import (
 	"errors"
 	"fmt"
 	"os"
-	"regexp"
 	"runtime"
 	"runtime/debug"
 	"sort"
-	"strings"
 	"sync"
 	"time"
 
@@ -98,6 +96,8 @@ func (qr *QueryRunner) RunStatement(ctx context.Context, stmt *query.Statement)
 		return res, fmt.Errorf("failed to parse query type: %w", err)
 	}
 
+	// make sure that the interface label is added if a query contains
+
 	// build condition tree to check if there is a syntax error before starting processing
 	queryConditional, valFilterNode, parseErr := node.ParseAndInstrument(stmt.Condition, stmt.DNSResolution.Timeout)
 	if parseErr != nil {
@@ -360,34 +360,10 @@ func parseIfaceList(dbPath string, ifaceList string) ([]string, error) {
 		return ifaces, nil
 	}
 
-	ifaces, err := validateIfaceNames(ifaceList)
+	ifaces, err := types.ValidateIfaceNames(ifaceList)
 	if err != nil {
 		return nil, err
 	}
 
 	return ifaces, nil
 }
-
-var ifaceNameRegexp = regexp.MustCompile(`^[a-zA-Z0-9\.:_-]{1,15}$`)
-
-func validateIfaceName(iface string) error {
-	if iface == "" {
-		return errors.New("interface list contains empty interface name")
-	}
-
-	if !ifaceNameRegexp.MatchString(iface) {
-		return fmt.Errorf("interface name `%s` is invalid", iface)
-	}
-
-	return nil
-}
-
-func validateIfaceNames(ifaceList string) ([]string, error) {
-	ifaces := strings.Split(ifaceList, ",")
-	for _, iface := range ifaces {
-		if err := validateIfaceName(iface); err != nil {
-			return nil, err
-		}
-	}
-	return ifaces, nil
-}

pkg/goDB/engine/query_test.go

@@ -161,7 +161,7 @@ func TestInterfaceValidation(t *testing.T) {
 	// run table-driven test
 	for _, test := range tests {
 		t.Run(test.iface, func(t *testing.T) {
-			err := validateIfaceName(test.iface)
+			err := types.ValidateIfaceName(test.iface)
 			if test.expectedErr != nil {
 				if err == nil || err.Error() != test.expectedErr.Error() {
 					t.Fatalf("unexpected result for interface name validation: %s", err)

pkg/query/args.go

@@ -339,6 +339,7 @@ func (a *Args) LogValue() slog.Value {
 }
 
 const (
+	invalidInterfaceMsg            = "invalid interface name"
 	invalidQueryTypeMsg            = "invalid query type"
 	invalidFormatMsg               = "unknown format"
 	invalidSortByMsg               = "unknown format"
@@ -404,10 +405,30 @@ func (a *Args) Prepare(writers ...io.Writer) (*Statement, error) {
 
 	}
 
+	// set and validate the interfaces
+	if a.Ifaces == "" {
+		err := newArgsError(
+			"iface",
+			invalidInterfaceMsg,
+			&types.ParseError{
+				Description: "empty input",
+			},
+		)
+		return s, err
+	}
+	s.Ifaces, err = types.ValidateIfaceNames(a.Ifaces)
+	if err != nil {
+		return s, newArgsError(
+			"iface",
+			invalidInterfaceMsg,
+			err,
+		)
+	}
+
 	// insert iface attribute here in case multiple interfaces where specified and the
 	// interface column was not added as an attribute
 	if (len(s.Ifaces) > 1 || strings.Contains(a.Ifaces, types.AnySelector)) &&
-		!strings.Contains(a.Query, "iface") {
+		!strings.Contains(a.Query, types.IfaceName) {
 		selector.Iface = true
 	}
 	s.LabelSelector = selector

pkg/query/args_test.go

@@ -225,7 +225,8 @@ func TestPrepareArgs(t *testing.T) {
 		},
 		{"valid query args",
 			&Args{
-				Query: "sip,time", Format: "json", Last: "-7d",
+				Ifaces: "eth0",
+				Query:  "sip,time", Format: "json", Last: "-7d",
 				MaxMemPct: 20, NumResults: 20,
 				outputs: []io.Writer{os.Stdout, os.Stderr},
 			},
@@ -244,16 +245,92 @@ func TestPrepareArgs(t *testing.T) {
 
 			t.Logf("error:\n%v", err)
 
-			actual := new(ArgsError)
-			ok := errors.As(err, &actual)
+			ok := errors.As(err, &test.err)
 			require.Truef(t, ok, "expected error to be of type %T", &ArgsError{})
 
 			// individually compare the struct fields. Why the detour? So we don't have
 			// to re-create (and test) errors that are caught by other packages (such as
 			// parsing errors)
-			require.Equal(t, test.err.Field, actual.Field)
-			require.Equal(t, test.err.Type, actual.Type)
-			require.Equal(t, test.err.Message, actual.Message)
+			require.Equal(t, test.err.Field, test.err.Field)
+			require.Equal(t, test.err.Type, test.err.Type)
+			require.Equal(t, test.err.Message, test.err.Message)
+		})
+	}
+}
+
+func TestSelector(t *testing.T) {
+	var tests = []struct {
+		name     string
+		input    *Args
+		selector types.LabelSelector
+		err      *ArgsError
+	}{
+		{
+			name: "empty interface",
+			input: &Args{
+				Query: "sip", Format: "json", Last: "-7d",
+				MaxMemPct: 20, NumResults: 20,
+				outputs: []io.Writer{os.Stdout, os.Stderr},
+			},
+			selector: types.LabelSelector{},
+			err: &ArgsError{
+				Field:   "iface",
+				Message: invalidInterfaceMsg,
+				Type:    "*types.ParseError",
+			},
+		},
+		{
+			name: "single interface",
+			input: &Args{
+				Ifaces: "eth0",
+				Query:  "sip", Format: "json", Last: "-7d",
+				MaxMemPct: 20, NumResults: 20,
+				outputs: []io.Writer{os.Stdout, os.Stderr},
+			},
+			selector: types.LabelSelector{},
+		},
+		{
+			name: "two interfaces",
+			input: &Args{
+				Ifaces: "eth0,eth2",
+				Query:  "sip", Format: "json", Last: "-7d",
+				MaxMemPct: 20, NumResults: 20,
+				outputs: []io.Writer{os.Stdout, os.Stderr},
+			},
+			selector: types.LabelSelector{
+				Iface: true,
+			},
+		},
+		{
+			name: "invalid interface name",
+			input: &Args{
+				Ifaces: "eth0,eth two",
+				Query:  "sip", Format: "json", Last: "-7d",
+				MaxMemPct: 20, NumResults: 20,
+				outputs: []io.Writer{os.Stdout, os.Stderr},
+			},
+			err: &ArgsError{
+				Field:   "iface",
+				Message: invalidInterfaceMsg,
+				Type:    "*errors.errorString",
+			},
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			stmt, err := test.input.Prepare()
+			if test.err != nil {
+				require.ErrorAs(t, err, &test.err)
+				require.Equal(t, test.err.Field, test.err.Field)
+				require.Equal(t, test.err.Type, test.err.Type)
+
+				t.Log(err)
+				return
+			}
+			require.Nil(t, err)
+			require.Equal(t, test.selector, stmt.LabelSelector)
 		})
 	}
 }

pkg/types/iface.go

@@ -0,0 +1,32 @@
+package types
+
+import (
+	"errors"
+	"fmt"
+	"regexp"
+	"strings"
+)
+
+var ifaceNameRegexp = regexp.MustCompile(`^[a-zA-Z0-9\.:_-]{1,15}$`)
+
+func ValidateIfaceName(iface string) error {
+	if iface == "" {
+		return errors.New("interface list contains empty interface name")
+	}
+
+	if !ifaceNameRegexp.MatchString(iface) {
+		return fmt.Errorf("interface name `%s` is invalid", iface)
+	}
+
+	return nil
+}
+
+func ValidateIfaceNames(ifaceList string) ([]string, error) {
+	ifaces := strings.Split(ifaceList, ",")
+	for _, iface := range ifaces {
+		if err := ValidateIfaceName(iface); err != nil {
+			return nil, err
+		}
+	}
+	return ifaces, nil
+}