From 5f24bf7068774eac0e54b92dd061568fed6fe300 Mon Sep 17 00:00:00 2001 From: Evans Mungai Date: Mon, 21 Oct 2024 18:04:10 +0100 Subject: [PATCH] Fix how we wrap tracing spans Signed-off-by: Evans Mungai --- pkg/supportbundle/collect.go | 91 ++++++++++++------------------- pkg/supportbundle/collect_test.go | 45 --------------- 2 files changed, 36 insertions(+), 100 deletions(-) delete mode 100644 pkg/supportbundle/collect_test.go diff --git a/pkg/supportbundle/collect.go b/pkg/supportbundle/collect.go index 303b68bcc..ec8e680c2 100644 --- a/pkg/supportbundle/collect.go +++ b/pkg/supportbundle/collect.go @@ -23,21 +23,10 @@ import ( "k8s.io/client-go/kubernetes" ) -type FilteredCollector struct { - Spec troubleshootv1beta2.HostCollect - Collector collect.HostCollector -} - func runHostCollectors(ctx context.Context, hostCollectors []*troubleshootv1beta2.HostCollect, additionalRedactors *troubleshootv1beta2.Redactor, bundlePath string, opts SupportBundleCreateOpts) (collect.CollectorResult, error) { collectSpecs := append([]*troubleshootv1beta2.HostCollect{}, hostCollectors...) collectedData := make(map[string][]byte) - // Filter out excluded collectors - filteredCollectors, err := filterHostCollectors(ctx, collectSpecs, bundlePath, opts) - if err != nil { - return nil, err - } - if opts.RunHostCollectorsInPod { if err := checkRemoteCollectorRBAC(ctx, opts.KubernetesRestConfig, "Remote Host Collectors", opts.Namespace); err != nil { if rbacErr, ok := err.(*RBACPermissionError); ok { @@ -52,11 +41,11 @@ func runHostCollectors(ctx context.Context, hostCollectors []*troubleshootv1beta return nil, err } } - if err := collectRemoteHost(ctx, filteredCollectors, bundlePath, opts, collectedData); err != nil { + if err := collectRemoteHost(ctx, collectSpecs, bundlePath, opts, collectedData); err != nil { return nil, err } } else { - if err := collectHost(ctx, filteredCollectors, opts, collectedData); err != nil { + if err := collectHost(ctx, collectSpecs, bundlePath, opts, collectedData); err != nil { return nil, err } } @@ -219,27 +208,38 @@ func getAnalysisFile(analyzeResults []*analyze.AnalyzeResult) (io.Reader, error) } // collectRemoteHost runs remote host collectors sequentially -func collectRemoteHost(ctx context.Context, filteredCollectors []FilteredCollector, bundlePath string, opts SupportBundleCreateOpts, collectedData map[string][]byte) error { +func collectRemoteHost(ctx context.Context, collectSpecs []*troubleshootv1beta2.HostCollect, bundlePath string, opts SupportBundleCreateOpts, collectedData map[string][]byte) error { opts.KubernetesRestConfig.QPS = constants.DEFAULT_CLIENT_QPS opts.KubernetesRestConfig.Burst = constants.DEFAULT_CLIENT_BURST opts.KubernetesRestConfig.UserAgent = fmt.Sprintf("%s/%s", constants.DEFAULT_CLIENT_USER_AGENT, version.Version()) // Run remote collectors sequentially - for _, c := range filteredCollectors { - collector := c.Collector - spec := c.Spec - - // Send progress event: starting the collector - opts.ProgressChan <- fmt.Sprintf("[%s] Running host collector...", collector.Title()) + for _, spec := range collectSpecs { + collector, ok := collect.GetHostCollector(spec, bundlePath) + if !ok { + opts.ProgressChan <- "Host collector not found" + continue + } // Start a span for tracing _, span := otel.Tracer(constants.LIB_TRACER_NAME).Start(ctx, collector.Title()) span.SetAttributes(attribute.String("type", reflect.TypeOf(collector).String())) + isExcluded, _ := collector.IsExcluded() + if isExcluded { + opts.ProgressChan <- fmt.Sprintf("[%s] Excluding host collector", collector.Title()) + span.SetAttributes(attribute.Bool(constants.EXCLUDED, true)) + span.End() + continue + } + + // Send progress event: starting the collector + opts.ProgressChan <- fmt.Sprintf("[%s] Running host collector...", collector.Title()) + // Parameters for remote collection params := &collect.RemoteCollectParams{ ProgressChan: opts.ProgressChan, - HostCollector: &spec, + HostCollector: spec, BundlePath: bundlePath, ClientConfig: opts.KubernetesRestConfig, Image: "replicated/troubleshoot:latest", @@ -273,10 +273,14 @@ func collectRemoteHost(ctx context.Context, filteredCollectors []FilteredCollect } // collectHost runs host collectors sequentially -func collectHost(ctx context.Context, filteredCollectors []FilteredCollector, opts SupportBundleCreateOpts, collectedData map[string][]byte) error { +func collectHost(ctx context.Context, collectSpecs []*troubleshootv1beta2.HostCollect, bundlePath string, opts SupportBundleCreateOpts, collectedData map[string][]byte) error { // Run local collectors sequentially - for _, c := range filteredCollectors { - collector := c.Collector + for _, spec := range collectSpecs { + collector, ok := collect.GetHostCollector(spec, bundlePath) + if !ok { + opts.ProgressChan <- "Host collector not found" + continue + } // Send progress event: starting the collector opts.ProgressChan <- fmt.Sprintf("[%s] Running host collector...", collector.Title()) @@ -285,6 +289,14 @@ func collectHost(ctx context.Context, filteredCollectors []FilteredCollector, op _, span := otel.Tracer(constants.LIB_TRACER_NAME).Start(ctx, collector.Title()) span.SetAttributes(attribute.String("type", reflect.TypeOf(collector).String())) + isExcluded, _ := collector.IsExcluded() + if isExcluded { + opts.ProgressChan <- fmt.Sprintf("[%s] Excluding host collector", collector.Title()) + span.SetAttributes(attribute.Bool(constants.EXCLUDED, true)) + span.End() + continue + } + // Run local collector sequentially result, err := collector.Collect(opts.ProgressChan) if err != nil { @@ -324,34 +336,3 @@ func getGlobalRedactors(additionalRedactors *troubleshootv1beta2.Redactor) []*tr } return []*troubleshootv1beta2.Redact{} } - -// filterHostCollectors filters out excluded collectors and returns a list of collectors to run -func filterHostCollectors(ctx context.Context, collectSpecs []*troubleshootv1beta2.HostCollect, bundlePath string, opts SupportBundleCreateOpts) ([]FilteredCollector, error) { - var filteredCollectors []FilteredCollector - - for _, desiredCollector := range collectSpecs { - collector, ok := collect.GetHostCollector(desiredCollector, bundlePath) - if !ok { - opts.ProgressChan <- "Host collector not found" - continue - } - - _, span := otel.Tracer(constants.LIB_TRACER_NAME).Start(ctx, collector.Title()) - span.SetAttributes(attribute.String("type", reflect.TypeOf(collector).String())) - - isExcluded, _ := collector.IsExcluded() - if isExcluded { - opts.ProgressChan <- fmt.Sprintf("[%s] Excluding host collector", collector.Title()) - span.SetAttributes(attribute.Bool(constants.EXCLUDED, true)) - span.End() - continue - } - - filteredCollectors = append(filteredCollectors, FilteredCollector{ - Spec: *desiredCollector, - Collector: collector, - }) - } - - return filteredCollectors, nil -} diff --git a/pkg/supportbundle/collect_test.go b/pkg/supportbundle/collect_test.go deleted file mode 100644 index 0dfda3e68..000000000 --- a/pkg/supportbundle/collect_test.go +++ /dev/null @@ -1,45 +0,0 @@ -package supportbundle - -import ( - "context" - "testing" - - v1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" - "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/trace/noop" -) - -func Test_filterHostCollectors(t *testing.T) { - otel.SetTracerProvider(noop.NewTracerProvider()) - - testCases := []struct { - name string - collectSpecs []*v1beta2.HostCollect - bundlePath string - opts SupportBundleCreateOpts - expectedResult []FilteredCollector - expectedError error - }{ - { - name: "nil host collectors spec", - collectSpecs: []*v1beta2.HostCollect{}, - bundlePath: "/tmp", - opts: SupportBundleCreateOpts{ - ProgressChan: make(chan interface{}, 10), - }, - expectedResult: []FilteredCollector{}, - expectedError: nil, - }, - } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - filtered, err := filterHostCollectors(context.TODO(), tc.collectSpecs, tc.bundlePath, tc.opts) - if err != tc.expectedError { - t.Fatalf("expected error %v, got %v", tc.expectedError, err) - } - if len(filtered) != len(tc.expectedResult) { - t.Fatalf("expected %d filtered collectors, got %d", len(tc.expectedResult), len(filtered)) - } - }) - } -}