[chore][extension/observer/k8sobserver] try to fix flaky test (DATA RACE) of TestExtensionObservePods

[sakulali]

Description:
Try to fix flaky test (DATA RACE) of TestExtensionObservePods. Since config.ObservePods is set true by default, for TestExtensionObserveServices and TestExtensionObserveNodes test case, we just focus on services observer and nodes observer, so we don't need to use k8sObserver.podListerWatcher which can avoid data race in the same process.

Link to tracking Issue:
#29448

Testing:
go test for k8sobserver

Preparation:

DIR = opentelemetry-collector-contrib/extension/observer/k8sobserver
DATA_DIR = /path/to/otel/coverage/unit
CMD = go test -run TestExtensionObserve -v -count=1 -race -timeout 300s -parallel 4 --tags="" -cover ./... -covermode=atomic -args -test.gocoverdir=${DATA_DIR}
MULTI_CMD = for i in {1..50}; do echo "Run $i"; ./{CMD} &; done

Tests:

1. I found config.ObservePods is set true by default. Then TestExtensionObserveServices and TestExtensionObserveNodes will enable k8sObserver.podListerWatcher and try to access the same http proxy since they are in the same process.

   opentelemetry-collector-contrib/extension/observer/k8sobserver/factory.go

   Lines 26 to 33 in 39641df

   	// CreateDefaultConfig creates the default configuration for the extension.
   	func createDefaultConfig() component.Config {
   	return &Config{
   	APIConfig: k8sconfig.APIConfig{AuthType: k8sconfig.AuthTypeServiceAccount},
   	ObservePods: true,
   	ObserveNodes: false,
   	}
   	}

2. I add log(as shown below) for TestExtensionObserve* functions and found test case in the same process. Refer to stack overflow, go test are executed as goroutines and executed concurrently.

--------- TestExtensionObserveServices process ID: 38910
--------- TestExtensionObservePods process ID: 38910
--------- TestExtensionObserveNodes process ID: 38910

3. I add log (as shown below) for http proxy setting addr, and found use the same address:

-------------- use proxy connectMethodForRequest t.Proxy addr: 0xc0007142f0 -------------------
-------------- transport t.Proxy (is) nil
-------------- send request: {Method:GET URL:https://mock:12345/api/v1/pods?limit=500&resourceVersion=0 Proto:HTTP/1.1 ProtoMajor:1 ProtoMinor:1 Header:map[Accept:[application/json, /] User-Agent:[k8sobserver.test/v0.0.0 (darwin/amd64) kubernetes/$Format]] Body: GetBody: ContentLength:0 TransferEncoding:[] Close:false Host:mock:12345 Form:map[] PostForm:map[] MultipartForm: Trailer:map[] RemoteAddr: RequestURI: TLS: Cancel: Response: ctx:0xc000410120}

4. So we may find one goroutine is writing http proxy and the other goroutine read with http proxy(HTTP request for Pods Lister). As a result, this has led to data race.

Inference:
I found that config.ObservePods is set true by default. For TestExtensionObserveServices and TestExtensionObserveNodes test case, we just focus on services observer and nodes observer, so we don't need to use k8sObserver.podListerWatcher which can avoid data race in the same process.

Documentation:

[crobert-1]

I'm a bit worried about removing the proxy setting when it's set to nil on purpose. Is it somehow unnecessary now? Otherwise, is there some way to make this operation work with concurrent access?

[sakulali]

I'm a bit worried about removing the proxy setting when it's set to nil on purpose. Is it somehow unnecessary now? Otherwise, is there some way to make this operation work with concurrent access?

Thanks for your reviews @crobert-1! I shouldn't remove this proxy setting, i need to further investigate the specific reason before taking any action.

[sakulali]

Hi @crobert-1, could you mind help to reviews again? After I added debug logs, i discovered some interesting things and maybe we can resolve this issue, please refer to the above Testing section.

[crobert-1]

Looks good to me, thanks for including all of the references as well!