[connector/servicegraph] Unit test failure: TestConnectorConsume/test_fix_failed_label_not_work

[crobert-1]

Component(s)

connector/servicegraph

Describe the issue you're reporting

Failing CI/CD action link

Failure occurred in unrelated PR, running on ubuntu.

Failure output:

Running target 'test-with-cover' in module 'connector/servicegraphconnector' as part of group 'connector'
make --no-print-directory -C connector/servicegraphconnector test-with-cover
mkdir -p /home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/coverage/unit
/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/.tools/gotestsum --rerun-fails=1 --packages="./..." -- -race -timeout 600s -parallel 4 --tags="" -cover -covermode=atomic -args -test.gocoverdir="/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/coverage/unit"
✖  . (79ms) (coverage: 87.3% of statements)
✓  internal/metadata (1.02s) (coverage: 83.3% of statements)
✓  internal/store (1.121s) (coverage: 92.7% of statements)

DONE 31 tests, 2 failures in 7.887s

✖  . (38ms) (coverage: 87.3% of statements)

=== Failed
make[2]: *** [../../Makefile.Common:131: test-with-cover] Error 1
=== FAIL: . TestConnectorConsume/test_fix_failed_label_not_work (0.01s)
make[1]: *** [Makefile:179: connector/servicegraphconnector] Error 2
    logger.go:146: 2024-07-09T18:45:05.793Z	INFO	Started servicegraphconnector
    logger.go:146: 2024-07-09T18:45:05.793Z	INFO	Shutting down servicegraphconnector
    logger.go:146: 2024-07-09T18:45:05.796Z	DEBUG	edge completed	{"client_service": "foo", "server_service": "bar", "connection_type": "", "trace_id": "b3a53e9c2ac533afc48b511c2f368dc1"}
    logger.go:146: 2024-07-09T18:45:05.796Z	DEBUG	edge completed	{"client_service": "foo", "server_service": "bar", "connection_type": "", "trace_id": "c6aafb2dcf4d71c81ae52e50e8f805ee"}
make: *** [Makefile:131: gotest-with-cover] Error 2
    logger.go:146: 2024-07-09T18:45:05.796Z	DEBUG	edge completed	{"client_service": "foo", "server_service": "bar", "connection_type": "", "trace_id": "7af657de319d6a57967bc241882ea94f"}
    connector_test.go:129: 
        	Error Trace:	/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/connector/servicegraphconnector/connector_test.go:129
        	Error:      	Received unexpected error:
        	            	the following errors occurred:
        	            	 -  resource "map[]": scope "traces_service_graph": metric "traces_service_graph_request_client_seconds": missing expected datapoint: map[client:foo connection_type: failed:false server:bar]
        	            	 -  resource "map[]": scope "traces_service_graph": metric "traces_service_graph_request_client_seconds": unexpected datapoint: map[client:foo connection_type: failed:true server:bar]
        	            	 -  resource "map[]": scope "traces_service_graph": metric "traces_service_graph_request_client_seconds": missing expected datapoint: map[client:foo connection_type: failed:true server:bar]
        	            	 -  resource "map[]": scope "traces_service_graph": metric "traces_service_graph_request_client_seconds": unexpected datapoint: map[client:foo connection_type: failed:false server:bar]
        	            	 -  resource "map[]": scope "traces_service_graph": metric "traces_service_graph_request_server_seconds": missing expected datapoint: map[client:foo connection_type: failed:false server:bar]
        	            	 -  resource "map[]": scope "traces_service_graph": metric "traces_service_graph_request_server_seconds": unexpected datapoint: map[client:foo connection_type: failed:true server:bar]
        	            	 -  resource "map[]": scope "traces_service_graph": metric "traces_service_graph_request_server_seconds": missing expected datapoint: map[client:foo connection_type: failed:true server:bar]
        	            	 -  resource "map[]": scope "traces_service_graph": metric "traces_service_graph_request_server_seconds": unexpected datapoint: map[client:foo connection_type: failed:false server:bar]
        	Test:       	TestConnectorConsume/test_fix_failed_label_not_work

=== FAIL: . TestConnectorConsume (0.01s)

=== FAIL: . TestConnectorConsume/test_fix_failed_label_not_work (re-run 1) (0.01s)
    logger.go:146: 2024-07-09T18:45:07.902Z	INFO	Started servicegraphconnector
    logger.go:146: 2024-07-09T18:45:07.902Z	INFO	Shutting down servicegraphconnector
    logger.go:146: 2024-07-09T18:45:07.904Z	DEBUG	edge completed	{"client_service": "foo", "server_service": "bar", "connection_type": "", "trace_id": "b3a53e9c2ac533afc48b511c2f368dc1"}
    logger.go:146: 2024-07-09T18:45:07.905Z	DEBUG	edge completed	{"client_service": "foo", "server_service": "bar", "connection_type": "", "trace_id": "c6aafb2dcf4d71c81ae52e50e8f805ee"}
    logger.go:146: 2024-07-09T18:45:07.905Z	DEBUG	edge completed	{"client_service": "foo", "server_service": "bar", "connection_type": "", "trace_id": "7af657de[319](https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/9854192691/job/27233186145?pr=33458#step:9:320)d6a57967bc241882ea94f"}
    connector_test.go:129: 
        	Error Trace:	/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/connector/servicegraphconnector/connector_test.go:129
        	Error:      	Received unexpected error:
        	            	the following errors occurred:
        	            	 -  resource "map[]": scope "traces_service_graph": metric "traces_service_graph_request_server_seconds": missing expected datapoint: map[client:foo connection_type: failed:false server:bar]
        	            	 -  resource "map[]": scope "traces_service_graph": metric "traces_service_graph_request_server_seconds": unexpected datapoint: map[client:foo connection_type: failed:true server:bar]
        	            	 -  resource "map[]": scope "traces_service_graph": metric "traces_service_graph_request_server_seconds": missing expected datapoint: map[client:foo connection_type: failed:true server:bar]
        	            	 -  resource "map[]": scope "traces_service_graph": metric "traces_service_graph_request_server_seconds": unexpected datapoint: map[client:foo connection_type: failed:false server:bar]
        	Test:       	TestConnectorConsume/test_fix_failed_label_not_work

=== FAIL: . TestConnectorConsume (re-run 1) (0.01s)

DONE 2 runs, 33 tests, 4 failures in 8.913s
make[1]: Leaving directory '/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib'
Error: Process completed with exit code 2.

[github-actions]

Pinging code owners:

• connector/servicegraph: @jpkrohling @mapno @JaredTan95

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[crobert-1]

+1 freq: https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/9861657624/job/27230596801

[crobert-1]

+1 freq: https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/9869760614/job/27254198175?pr=34014

[crobert-1]

+1 freq: https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/9874452305/job/27269074185?pr=33727

[jpkrohling]

I talked to @t00mas and he'll be looking into this soon.

[evantorrie]

The problem is due to the servicegraph connector unit tests making use of pmetrictest.CompareMetrics() with a golden set of metrics using IgnoreMetricsOrder() option, but where there are two different instances of a metric with the same name but different datapoint attributes (this seems to mostly (only?) affect comparison of Histograms).

pmetrictest.CompareScopeMetrics() considers two metrics from the same scope to be "matched" simply by comparing their names.

When the servicegraph connector test fails, it's because the names of consecutive metrics are the same, but the datapoint attributes are different between the actual metric and the expected metric from the golden dataset.

Perhaps IgnoreMetricAttributeValue() should be passed during this test?

Alternatively, perhaps pmetrictest.CompareScopeMetrics() with IgnoreMetricOrder() should consider more than just the name when deciding whether their identity is the same.

[jpkrohling]

I think that IgnoreMetricOrder() should also ignore the order of the attributes.

[evantorrie]

I don't completely understand the servicegraph metrics implementation, but on further investigation, I do wonder if the way the metrics are created in calls like collectServerLatencyMetrics and collectClientLatencyMetrics is incorrect.

In each case it ranges over a key, and creates a new metric each time, even though the name of the metric is the same in all cases.

My understanding was that if this were inside an application using the Otel SDK that the SDK would aggregate all those keys (which I think represent different attribute sets) into a single metric entry but with multiple datapoints. The servicegraphconnector code on the other hand, seems to create multiple entries for the same metric, with just one datapoint per metric entry.

Perhaps this is breaking the preconditions which pmetrictest.CompareScopeMetrics() is expecting - which appear to be only one entry per unique metric name in a single ScopeMetrics list of metrics.

[crobert-1]

+1 freq: https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/9903266660/job/27359747148?pr=33224