Add process minimum age to filter out short lived processes if needed

[corneliu-calciu]

Added minimum process age to be considered for discovery.

The new process detection logic will ignore a new process until its age reach a minimum value. As an example we may set a 3 seconds threshold as part of the monitoring configuration.
Any process will be considered as suitable for monitoring once is passing that configurable limit.
The noise of short lived processes can be filtered out.

Configuration example:
discovery:
exclude_services:

• k8s_namespace: (istio-system|kube-node-lease)
  services:
• k8s_namespace: (acmefitness)
  min_process_age: 5s

Moved the RP from grafana/beyla#2066

[mariomac]

Thanks for your contribution @corneliu-calciu !

I have one question: what happens if a long-lived process starts while Beyla is running? Beyla will get the notice of that new process but it will detect it as too young, so will it discard it?

[corneliu-calciu]

Hi Mario

This is exactly the scenario I want to address. The Beyla PODs are already running. A new process will be created on a node.
Now we can specify what is the minimum life span needed to accept and instrument the process.
If we have a large number of processes created and destroyed within a 1-2 seconds interval are just noise, especially on a Kubernetes environment where we expect long lived micro-services.
This capability can mitigate another problem. Please check bug grafana/beyla#2046.
When a new replica is created for a Kubernetes deployments sometimes the process filtering logic fails to recognize the inner processes of the container as are in the initialization phase where some new processes are created and some initialization processes will exit soon.

Best Regards
Corneliu

[grcevski]

This is looking good already, I left a few comments that I think we should address. Thanks for working on this!!!

[codecov]

Codecov Report

Attention: Patch coverage is 77.77778% with 6 lines in your changes missing coverage. Please review.

Project coverage is 43.15%. Comparing base (fa8739b) to head (b92c1b4).
Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/components/discover/watcher_proc.go	77.77%	3 Missing and 3 partials ⚠️

❗ There is a different number of reports uploaded between BASE (fa8739b) and HEAD (b92c1b4). Click for more details.

HEAD has 3 uploads less than BASE

Flag	BASE (fa8739b)	HEAD (b92c1b4)
unittests	1	0
k8s-integration-test	1	0
integration-test	1	0

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #194       +/-   ##
===========================================
- Coverage   75.23%   43.15%   -32.08%     
===========================================
  Files         177      170        -7     
  Lines       19167    18574      -593     
===========================================
- Hits        14420     8016     -6404     
- Misses       3997     9901     +5904     
+ Partials      750      657       -93     

Flag	Coverage Δ
integration-test	?
integration-test-arm	36.00% <77.77%> (+0.08%)	⬆️
k8s-integration-test	?
oats-test	35.98% <55.55%> (-0.24%)	⬇️
unittests	?

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.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[grcevski]

I looked at the test failures, I think we need to extend our default timeout interval since it takes a bit longer now for the services to be instrumented.

These functions:

func waitForTestComponentsSub(t *testing.T, url, subpath string) {
	waitForTestComponentsSubWithTime(t, url, subpath, 1)
}

func waitForTestComponentsSubStatus(t *testing.T, url, subpath string, status int) {
	waitForTestComponentsSubWithTimeAndCode(t, url, subpath, status, 1)
}

in test_utils.go (integration tests). Right now we wait a minute, I think we should make that a constant and change it to 2 minutes.

[grcevski]

Hm, it looks like the problem is somewhere else, I'm going to take a look myself, but I can't before Tuesday next week.

[mariomac]

Thanks @corneliu-calciu for the reply!

This is exactly the scenario I want to address. The Beyla PODs are already running. A new process will be created on a node.
Now we can specify what is the minimum life span needed to accept and instrument the process.
If we have a large number of processes created and destroyed within a 1-2 seconds interval are just noise, especially on a > Kubernetes environment where we expect long lived micro-services.

But imagine a new long-lived process is created (e.g. a web server). Beyla gets the notification of a new process/port, then before deciding whether it has to be instrumented, checks the age of the process, and since it's very recent, the process will be discarded. So our new web server is never instrumented.

Am I right, or am I missing something?

[corneliu-calciu]

Thanks @corneliu-calciu for the reply!

This is exactly the scenario I want to address. The Beyla PODs are already running. A new process will be created on a node.
Now we can specify what is the minimum life span needed to accept and instrument the process.
If we have a large number of processes created and destroyed within a 1-2 seconds interval are just noise, especially on a > Kubernetes environment where we expect long lived micro-services.

But imagine a new long-lived process is created (e.g. a web server). Beyla gets the notification of a new process/port, then before deciding whether it has to be instrumented, checks the age of the process, and since it's very recent, the process will be discarded. So our new web server is never instrumented.

Am I right, or am I missing something?

Hi Mario

The process scanning is a periodical activity. Basically the same process will be ignored in the first 5 seconds, and once will pass the minimum age will be handled.

Best Regards
Corneliu

[grcevski]

Beyla gets the notification of a new process/port, then before deciding whether it has to be instrumented, checks the age of the

So as you noticed, there's a problem with the new port event, since those are async and reported by eBPF, but @corneliu-calciu added a check to accept the process regardless of the age if the event was port open.