[usm] service discovery, replace the args list with a struct in detect functions #30076
Conversation
yuri-lipnesh
added
changelog/no-changelog
component/system-probe
team/usm
Test changes on VMUse this command from test-infra-definitions to manually test this PR changes on a VM: inv create-vm --pipeline-id=46937424 --os-family=ubuntuNote: This applies to commit bdf4a72 |
Regression Detector |
| @@ -38,7 +38,7 @@ const ( | |||
| Injected Instrumentation = "injected" | |||
| ) | |||
|
|
|||
| type detector func(pid int, args []string, envs envs.Variables, contextMap usm.DetectorContextMap) Instrumentation | |||
| type detector func(dc usm.DetectionContext) Instrumentation | |||
There was a problem hiding this comment.
Maybe call the parameter context or ctx instead of dc which is not so transparent?
| @@ -50,7 +50,8 @@ func TestServiceNameFromCLI(t *testing.T) { | |||
| expected: "", | |||
| }, | |||
| } | |||
| instance := &phpDetector{ctx: NewDetectionContext(nil, envs.NewVariables(nil), nil)} | |||
| dc := NewDetectionContext(0, nil, envs.NewVariables(nil), nil, nil) | |||
There was a problem hiding this comment.
Maybe if you leave the signature of NewDetectionContext() as-is but add a new constructor for the callsite which need it then you don't have to go in and another nil everywhere?
There was a problem hiding this comment.
I've restored signature of NewDetectionContext()
| @@ -619,12 +619,13 @@ func TestExtractServiceMetadata(t *testing.T) { | |||
| t.Skip("Not supported on Windows") | |||
| } | |||
|
|
|||
| var fs fs.SubFS | |||
| fs = RealFs{} | |||
| var f fs.SubFS | |||
There was a problem hiding this comment.
Why was this variable name changed?
There was a problem hiding this comment.
my IDE showed warning that name collides with imported package, I restored name as it was
yuri-lipnesh
changed the title
… 'dc' to 'ctx usm.DetectionContext'
What does this PR do?
This code replaces the argument list with a structure in the detect() function and sub-functions in system-probe service discovery.
Motivation
make service discovery functions flexible for future additions
Describe how to test/QA your changes
Possible Drawbacks / Trade-offs
Additional Notes
current implementation replaces list of arguments with existing structure usm.DetectionContext.
The size of this struct is 64 bytes, which is most likely equal to CPU cache line (64 bytes).
This struct is passed by value, consider passing the structure as a pointer instead of a value.