Use the new collector internal service for process information

[Molter73]

Description

This PR makes it so collector starts using the new unified internal service for communicating with sensor. For now the implementation only handles process information, but lays the groundwork to also move network flows into this new internal service.

In order to better abstract how we forward messages, a new CollectorOutput class has been added. The responsibility for this class is to hold all gRPC clients that we might need for sending messages to collector, both through the new internal service and the legacy ones, as well as ensuring the connection between sensor and collector is up. This change means the responsibility for the ProcessSignalClient class has been reduced to simply sending the message through the grpc client it holds. A new client has also been added for the new service.

Alongside this new CollectorOutput class, a new formatter has been added to generate messages that will be sent through the new service and the existing process signal handler has been modified to start using it.

Sibling PR stackrox/stackrox#14652

Checklist

• Investigated and inspected CI test results
• Updated documentation accordingly

Automated testing

• Added unit tests
• Added integration tests
• Added regression tests

If any of these don't apply, please comment below.

Testing Performed

Deployed this version of collector with the sensor version from stackrox/stackrox#14652 and checked process information is still available in the Risk tab.
Modified the existing integration tests to use the new service.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 15.88089% with 339 lines in your changes missing coverage. Please review.

Project coverage is 27.73%. Comparing base (215e5d4) to head (81a7b0c).

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
collector/lib/SensorClientFormatter.cpp	17.89%	112 Missing and 44 partials ⚠️
collector/lib/CollectorOutput.cpp	18.29%	62 Missing and 5 partials ⚠️
collector/lib/ProcessSignalHandler.cpp	0.00%	51 Missing ⚠️
collector/lib/SensorClient.cpp	0.00%	29 Missing ⚠️
collector/lib/SensorClient.h	16.66%	10 Missing ⚠️
collector/lib/ProcessSignalFormatter.cpp	40.00%	5 Missing and 1 partial ⚠️
collector/lib/CollectorOutput.h	54.54%	2 Missing and 3 partials ⚠️
collector/lib/SignalServiceClient.h	16.66%	5 Missing ⚠️
collector/lib/CollectorConfig.cpp	0.00%	2 Missing ⚠️
collector/lib/CollectorConfig.h	0.00%	2 Missing ⚠️
... and 3 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2063      +/-   ##
==========================================
- Coverage   28.40%   27.73%   -0.68%     
==========================================
  Files          94       99       +5     
  Lines        5717     6050     +333     
  Branches     2531     2701     +170     
==========================================
+ Hits         1624     1678      +54     
- Misses       3383     3611     +228     
- Partials      710      761      +51     

Flag	Coverage Δ
collector-unit-tests	27.73% <15.88%> (-0.68%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Molter73]

/retest

[Molter73]

This file is essentially a copy of ProcessSignalFormatter.cpp, but building a sensor::MsgFromCollector instead of a storage::ProcessSignal.

[Molter73]

This file is essentially a copy of ProcessSignalFormatter.h, but building a sensor::MsgFromCollector instead of a storage::ProcessSignal.

[JoukoVirtanen]

It seems that SignalHandler::NEEDS_REFRESH is returned when first_write_ is true and that is the case only after Refresh has been called. So, SignalHandler::NEEDS_REFRESH is returned after a refresh has been done and returning SignalHandler::NEEDS_REFRESH does not result in a refresh, as the name implies. It does result in SendMsg being called again. In which case first_write_ will be true and this will be skipped and the message will be sent the second time. I don't understand what returning here instead of sending the message the first time accomplishes.

[Molter73]

This is poor naming conventions on my part. NEEDS_REFRESH is used when the process stream has just started and it causes collector to send all processes that are stored in the threadinfo table, at startup this sends all scraped processes, while running it will ensure any processes that were not sent as part of a disconnect to be sent on reconnection.

I probably need to come up with a better term than Refresh for the method called after grpc is reconnected. I'll think it over and rename the method.

[Molter73]

I've renamed the Refresh method to Recreate, hopefully that makes it less ambiguous.

[JoukoVirtanen]

This needs more explanation of how you validated your change. You should mention your stackrox/stackrox PR and how that passing CI validates the changes here. Since you are requiring that this works with old versions of sensor, you should also validate that. You could create a stackrox/stackrox PR that just updates the collector version and sets the environment variable that you introduced for the qa e2e tests. Having those tests pass should be enough.

Other than that, this looks good.

[JoukoVirtanen]

LGTM!