⚠️ always use up-to-date context (#310)

We were using stale context. I observed it when looking at tracer output, some spans were being recorded in the scope of main context, instead of their provider specific context. :warning: changes provider api --------- Signed-off-by: Pranav Gaikwad <[email protected]>
konveyor · Sep 15, 2023 · 94f0459 · 94f0459
1 parent 074bc15
commit 94f0459
Show file tree

Hide file tree

Showing 19 changed files with 109 additions and 96 deletions.
diff --git a/.github/workflows/demo-testing.yml b/.github/workflows/demo-testing.yml
@@ -8,6 +8,14 @@ jobs:
     steps:
       - uses: actions/checkout@v3
 
+      - name: replace analyzer-lsp upstream dep from external providers
+        run: |
+          for dir in ./external-providers/*; do
+            pushd $dir &> /dev/null
+            go mod edit -replace=github.com/konveyor/analyzer-lsp=../../
+            popd &> /dev/null
+          done
+
       - name: build image
         run: podman build -t quay.io/konveyor/analyzer-lsp:latest .
 

diff --git a/cmd/dep/main.go b/cmd/dep/main.go
@@ -103,7 +103,7 @@ func main() {
 		}
 
 		if treeOutput {
-			deps, err := prov.GetDependenciesDAG()
+			deps, err := prov.GetDependenciesDAG(ctx)
 			if err != nil {
 				log.Error(err, "failed to get list of dependencies for provider", "provider", name)
 				continue
@@ -116,7 +116,7 @@ func main() {
 				})
 			}
 		} else {
-			deps, err := prov.GetDependencies()
+			deps, err := prov.GetDependencies(ctx)
 			if err != nil {
 				log.Error(err, "failed to get list of dependencies for provider", "provider", name)
 				continue

diff --git a/engine/engine.go b/engine/engine.go
@@ -175,7 +175,7 @@ func (r *ruleEngine) RunRules(ctx context.Context, ruleSets []RuleSet, selectors
 							rs.Errors[response.Rule.RuleID] = response.Err.Error()
 						}
 					} else if response.ConditionResponse.Matched && len(response.ConditionResponse.Incidents) > 0 {
-						violation, err := r.createViolation(response.ConditionResponse, response.Rule)
+						violation, err := r.createViolation(ctx, response.ConditionResponse, response.Rule)
 						if err != nil {
 							r.logger.Error(err, "unable to create violation from response")
 						}
@@ -388,7 +388,7 @@ func processRule(ctx context.Context, rule Rule, ruleCtx ConditionContext, log l
 
 }
 
-func (r *ruleEngine) createViolation(conditionResponse ConditionResponse, rule Rule) (konveyor.Violation, error) {
+func (r *ruleEngine) createViolation(ctx context.Context, conditionResponse ConditionResponse, rule Rule) (konveyor.Violation, error) {
 	incidents := []konveyor.Incident{}
 	fileCodeSnipCount := map[string]int{}
 	incidentsSet := map[string]struct{}{} // Set of incidents
@@ -409,7 +409,7 @@ func (r *ruleEngine) createViolation(conditionResponse ConditionResponse, rule R
 		// Some violations may not have a location in code.
 		limitSnip := (r.codeSnipLimit != 0 && fileCodeSnipCount[string(m.FileURI)] == r.codeSnipLimit)
 		if !limitSnip {
-			codeSnip, err := r.getCodeLocation(m, rule)
+			codeSnip, err := r.getCodeLocation(ctx, m, rule)
 			if err != nil || codeSnip == "" {
 				r.logger.V(6).Error(err, "unable to get code location")
 			} else {
@@ -496,7 +496,7 @@ func (r *ruleEngine) createViolation(conditionResponse ConditionResponse, rule R
 	}, nil
 }
 
-func (r *ruleEngine) getCodeLocation(m IncidentContext, rule Rule) (codeSnip string, err error) {
+func (r *ruleEngine) getCodeLocation(ctx context.Context, m IncidentContext, rule Rule) (codeSnip string, err error) {
 	if m.CodeLocation == nil {
 		r.logger.V(6).Info("unable to get the code snip", "URI", m.FileURI)
 		return "", nil

diff --git a/external-providers/generic-external-provider/pkg/generic/dependency.go b/external-providers/generic-external-provider/pkg/generic/dependency.go
@@ -1,6 +1,7 @@
 package generic
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"os/exec"
@@ -9,7 +10,7 @@ import (
 	"go.lsp.dev/uri"
 )
 
-func (g *genericServiceClient) GetDependencies() (map[uri.URI][]*provider.Dep, error) {
+func (g *genericServiceClient) GetDependencies(ctx context.Context) (map[uri.URI][]*provider.Dep, error) {
 	cmdStr, isString := g.config.ProviderSpecificConfig["dependencyProviderPath"].(string)
 	if !isString {
 		return nil, fmt.Errorf("dependency provider path is not a string")
@@ -33,6 +34,6 @@ func (g *genericServiceClient) GetDependencies() (map[uri.URI][]*provider.Dep, e
 	return m, err
 }
 
-func (p *genericServiceClient) GetDependenciesDAG() (map[uri.URI][]provider.DepDAGItem, error) {
+func (p *genericServiceClient) GetDependenciesDAG(ctx context.Context) (map[uri.URI][]provider.DepDAGItem, error) {
 	return nil, nil
 }
diff --git a/external-providers/generic-external-provider/pkg/generic/provider.go b/external-providers/generic-external-provider/pkg/generic/provider.go
@@ -110,7 +110,6 @@ func (p *genericProvider) Init(ctx context.Context, log logr.Logger, c provider.
 
 	svcClient := genericServiceClient{
 		rpc:        rpc,
-		ctx:        ctx,
 		cancelFunc: cancelFunc,
 		cmd:        cmd,
 		config:     c,

diff --git a/external-providers/generic-external-provider/pkg/generic/service_client.go b/external-providers/generic-external-provider/pkg/generic/service_client.go
@@ -21,7 +21,6 @@ import (
 
 type genericServiceClient struct {
 	rpc        *jsonrpc2.Conn
-	ctx        context.Context
 	cancelFunc context.CancelFunc
 	cmd        *exec.Cmd
 
@@ -34,7 +33,7 @@ func (p *genericServiceClient) Stop() {
 	p.cancelFunc()
 	p.cmd.Wait()
 }
-func (p *genericServiceClient) Evaluate(cap string, conditionInfo []byte) (provider.ProviderEvaluateResponse, error) {
+func (p *genericServiceClient) Evaluate(ctx context.Context, cap string, conditionInfo []byte) (provider.ProviderEvaluateResponse, error) {
 	var cond genericCondition
 	err := yaml.Unmarshal(conditionInfo, &cond)
 	if err != nil {
@@ -45,11 +44,11 @@ func (p *genericServiceClient) Evaluate(cap string, conditionInfo []byte) (provi
 		return provider.ProviderEvaluateResponse{}, fmt.Errorf("unable to get query info")
 	}
 
-	symbols := p.GetAllSymbols(query)
+	symbols := p.GetAllSymbols(ctx, query)
 
 	incidents := []provider.IncidentContext{}
 	for _, s := range symbols {
-		references := p.GetAllReferences(s.Location)
+		references := p.GetAllReferences(ctx, s.Location)
 		for _, ref := range references {
 			// Look for things that are in the location loaded, //Note may need to filter out vendor at some point
 			if strings.Contains(ref.URI, p.config.Location) {
@@ -145,14 +144,14 @@ func parallelWalk(location string, regex *regexp.Regexp) ([]protocol.TextDocumen
 	return positions, nil
 }
 
-func (p *genericServiceClient) GetAllSymbols(query string) []protocol.WorkspaceSymbol {
+func (p *genericServiceClient) GetAllSymbols(ctx context.Context, query string) []protocol.WorkspaceSymbol {
 	wsp := &protocol.WorkspaceSymbolParams{
 		Query: query,
 	}
 
 	var symbols []protocol.WorkspaceSymbol
 	fmt.Printf("\nrpc call\n")
-	err := p.rpc.Call(context.TODO(), "workspace/symbol", wsp, &symbols)
+	err := p.rpc.Call(ctx, "workspace/symbol", wsp, &symbols)
 	fmt.Printf("\nrpc called\n")
 	if err != nil {
 		fmt.Printf("\n\nerror: %v\n", err)
@@ -165,7 +164,7 @@ func (p *genericServiceClient) GetAllSymbols(query string) []protocol.WorkspaceS
 	if len(symbols) == 0 {
 		// Run empty string query and manually search using the query as a regex
 		var allSymbols []protocol.WorkspaceSymbol
-		err = p.rpc.Call(context.TODO(), "workspace/symbol", &protocol.WorkspaceSymbolParams{Query: ""}, &allSymbols)
+		err = p.rpc.Call(ctx, "workspace/symbol", &protocol.WorkspaceSymbolParams{Query: ""}, &allSymbols)
 		if err != nil {
 			fmt.Printf("\n\nerror: %v\n", err)
 		}
@@ -186,7 +185,7 @@ func (p *genericServiceClient) GetAllSymbols(query string) []protocol.WorkspaceS
 		for _, position := range positions {
 			fmt.Println(position)
 			res := []protocol.Location{}
-			err := p.rpc.Call(p.ctx, "textDocument/definition", position, &res)
+			err := p.rpc.Call(ctx, "textDocument/definition", position, &res)
 			if err != nil {
 				fmt.Printf("Error rpc: %v", err)
 			}
@@ -198,7 +197,7 @@ func (p *genericServiceClient) GetAllSymbols(query string) []protocol.WorkspaceS
 	return symbols
 }
 
-func (p *genericServiceClient) GetAllReferences(location protocol.Location) []protocol.Location {
+func (p *genericServiceClient) GetAllReferences(ctx context.Context, location protocol.Location) []protocol.Location {
 	params := &protocol.ReferenceParams{
 		TextDocumentPositionParams: protocol.TextDocumentPositionParams{
 			TextDocument: protocol.TextDocumentIdentifier{
@@ -209,7 +208,7 @@ func (p *genericServiceClient) GetAllReferences(location protocol.Location) []pr
 	}
 
 	res := []protocol.Location{}
-	err := p.rpc.Call(p.ctx, "textDocument/references", params, &res)
+	err := p.rpc.Call(ctx, "textDocument/references", params, &res)
 	if err != nil {
 		fmt.Printf("Error rpc: %v", err)
 	}

diff --git a/parser/rule_parser_test.go b/parser/rule_parser_test.go
@@ -28,15 +28,15 @@ func (t testProvider) Init(ctx context.Context, log logr.Logger, config provider
 	return nil, nil
 }
 
-func (t testProvider) Evaluate(cap string, conditionInfo []byte) (provider.ProviderEvaluateResponse, error) {
+func (t testProvider) Evaluate(ctx context.Context, cap string, conditionInfo []byte) (provider.ProviderEvaluateResponse, error) {
 	return provider.ProviderEvaluateResponse{}, nil
 }
 
-func (t testProvider) GetDependencies() (map[uri.URI][]*provider.Dep, error) {
+func (t testProvider) GetDependencies(ctx context.Context) (map[uri.URI][]*provider.Dep, error) {
 	return nil, nil
 }
 
-func (t testProvider) GetDependenciesDAG() (map[uri.URI][]provider.DepDAGItem, error) {
+func (t testProvider) GetDependenciesDAG(ctx context.Context) (map[uri.URI][]provider.DepDAGItem, error) {
 	return nil, nil
 }
 

diff --git a/provider/grpc/provider.go b/provider/grpc/provider.go
@@ -44,7 +44,6 @@ func NewGRPCClient(config provider.Config, log logr.Logger) *grpcProvider {
 }
 
 func (g *grpcProvider) ProviderInit(ctx context.Context) error {
-	g.ctx = ctx
 	for _, c := range g.config.InitConfig {
 		s, err := g.Init(ctx, g.log, c)
 		if err != nil {
@@ -102,22 +101,21 @@ func (g *grpcProvider) Init(ctx context.Context, log logr.Logger, config provide
 	}
 	return &grpcServiceClient{
 		id:     r.Id,
-		ctx:    ctx,
 		config: config,
 		client: g.Client,
 	}, nil
 }
 
-func (g *grpcProvider) Evaluate(cap string, conditionInfo []byte) (provider.ProviderEvaluateResponse, error) {
-	return provider.FullResponseFromServiceClients(g.serviceClients, cap, conditionInfo)
+func (g *grpcProvider) Evaluate(ctx context.Context, cap string, conditionInfo []byte) (provider.ProviderEvaluateResponse, error) {
+	return provider.FullResponseFromServiceClients(ctx, g.serviceClients, cap, conditionInfo)
 }
 
-func (g *grpcProvider) GetDependencies() (map[uri.URI][]*provider.Dep, error) {
-	return provider.FullDepsResponse(g.serviceClients)
+func (g *grpcProvider) GetDependencies(ctx context.Context) (map[uri.URI][]*provider.Dep, error) {
+	return provider.FullDepsResponse(ctx, g.serviceClients)
 }
 
-func (g *grpcProvider) GetDependenciesDAG() (map[uri.URI][]provider.DepDAGItem, error) {
-	return provider.FullDepDAGResponse(g.serviceClients)
+func (g *grpcProvider) GetDependenciesDAG(ctx context.Context) (map[uri.URI][]provider.DepDAGItem, error) {
+	return provider.FullDepDAGResponse(ctx, g.serviceClients)
 }
 
 func (g *grpcProvider) Stop() {

diff --git a/provider/grpc/service_client.go b/provider/grpc/service_client.go
@@ -11,20 +11,19 @@ import (
 
 type grpcServiceClient struct {
 	id     int64
-	ctx    context.Context
 	config provider.InitConfig
 	client pb.ProviderServiceClient
 }
 
 var _ provider.ServiceClient = &grpcServiceClient{}
 
-func (g *grpcServiceClient) Evaluate(cap string, conditionInfo []byte) (provider.ProviderEvaluateResponse, error) {
+func (g *grpcServiceClient) Evaluate(ctx context.Context, cap string, conditionInfo []byte) (provider.ProviderEvaluateResponse, error) {
 	m := pb.EvaluateRequest{
 		Cap:           cap,
 		ConditionInfo: string(conditionInfo),
 		Id:            g.id,
 	}
-	r, err := g.client.Evaluate(g.ctx, &m)
+	r, err := g.client.Evaluate(ctx, &m)
 	if err != nil {
 		return provider.ProviderEvaluateResponse{}, err
 	}
@@ -85,8 +84,8 @@ func (g *grpcServiceClient) Evaluate(cap string, conditionInfo []byte) (provider
 }
 
 // We don't have dependencies
-func (g *grpcServiceClient) GetDependencies() (map[uri.URI][]*provider.Dep, error) {
-	d, err := g.client.GetDependencies(g.ctx, &pb.ServiceRequest{Id: g.id})
+func (g *grpcServiceClient) GetDependencies(ctx context.Context) (map[uri.URI][]*provider.Dep, error) {
+	d, err := g.client.GetDependencies(ctx, &pb.ServiceRequest{Id: g.id})
 	if err != nil {
 		return nil, err
 	}
@@ -140,8 +139,8 @@ func recreateDAGAddedItems(items []*pb.DependencyDAGItem) []provider.DepDAGItem
 }
 
 // We don't have dependencies
-func (g *grpcServiceClient) GetDependenciesDAG() (map[uri.URI][]provider.DepDAGItem, error) {
-	d, err := g.client.GetDependenciesDAG(g.ctx, &pb.ServiceRequest{Id: g.id})
+func (g *grpcServiceClient) GetDependenciesDAG(ctx context.Context) (map[uri.URI][]provider.DepDAGItem, error) {
+	d, err := g.client.GetDependenciesDAG(ctx, &pb.ServiceRequest{Id: g.id})
 	if err != nil {
 		return nil, err
 	}

diff --git a/provider/internal/builtin/provider.go b/provider/internal/builtin/provider.go
@@ -103,14 +103,14 @@ func (p *builtinProvider) Capabilities() []provider.Capability {
 	return capabilities
 }
 
-func (p *builtinProvider) ProviderInit(context.Context) error {
+func (p *builtinProvider) ProviderInit(ctx context.Context) error {
 	// First load all the tags for all init configs.
 	for _, c := range p.config.InitConfig {
 		p.loadTags(c)
 	}
 
 	for _, c := range p.config.InitConfig {
-		client, err := p.Init(p.ctx, p.log, c)
+		client, err := p.Init(ctx, p.log, c)
 		if err != nil {
 			return nil
 		}
@@ -124,7 +124,7 @@ func (p *builtinProvider) Init(ctx context.Context, log logr.Logger, config prov
 	if config.AnalysisMode != provider.AnalysisMode("") {
 		p.log.V(5).Info("skipping analysis mode setting for builtin")
 	}
-	return &builtintServiceClient{
+	return &builtinServiceClient{
 		config:                             config,
 		tags:                               p.tags,
 		UnimplementedDependenciesComponent: provider.UnimplementedDependenciesComponent{},
@@ -157,8 +157,8 @@ func (p *builtinProvider) loadTags(config provider.InitConfig) error {
 	return nil
 }
 
-func (p *builtinProvider) Evaluate(cap string, conditionInfo []byte) (provider.ProviderEvaluateResponse, error) {
-	return provider.FullResponseFromServiceClients(p.clients, cap, conditionInfo)
+func (p *builtinProvider) Evaluate(ctx context.Context, cap string, conditionInfo []byte) (provider.ProviderEvaluateResponse, error) {
+	return provider.FullResponseFromServiceClients(ctx, p.clients, cap, conditionInfo)
 }
 
 func (p *builtinProvider) Stop() {

diff --git a/provider/internal/builtin/service_client.go b/provider/internal/builtin/service_client.go
@@ -1,6 +1,7 @@
 package builtin
 
 import (
+	"context"
 	"fmt"
 	"io/fs"
 	"os"
@@ -18,19 +19,19 @@ import (
 	"gopkg.in/yaml.v2"
 )
 
-type builtintServiceClient struct {
+type builtinServiceClient struct {
 	config provider.InitConfig
 	tags   map[string]bool
 	provider.UnimplementedDependenciesComponent
 }
 
-var _ provider.ServiceClient = &builtintServiceClient{}
+var _ provider.ServiceClient = &builtinServiceClient{}
 
-func (p *builtintServiceClient) Stop() {
+func (p *builtinServiceClient) Stop() {
 	return
 }
 
-func (p *builtintServiceClient) Evaluate(cap string, conditionInfo []byte) (provider.ProviderEvaluateResponse, error) {
+func (p *builtinServiceClient) Evaluate(ctx context.Context, cap string, conditionInfo []byte) (provider.ProviderEvaluateResponse, error) {
 	var cond builtinCondition
 	err := yaml.Unmarshal(conditionInfo, &cond)
 	if err != nil {