Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(routingprocessor): route on resource attributes #5694

Conversation

pmalek-sumo
Copy link
Contributor

@pmalek-sumo pmalek-sumo commented Oct 11, 2021

Description: Add a feature to routingprocessor to route traces on resource attributes and make it configurable (retaining previous default to read from context).

Link to tracking Issue: Fixes #5538

Testing: Added unit tests.

Documentation: Changed README.


Huge shout-out to @astencel-sumo for this patch! 🙇

Copy link
Member

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is great! I had only a few minor comments, but overall, this is a great addition and will make the routing processor way more useful than it currently is.

processor/routingprocessor/README.md Outdated Show resolved Hide resolved
processor/routingprocessor/README.md Outdated Show resolved Hide resolved
processor/routingprocessor/config.go Show resolved Hide resolved
processor/routingprocessor/routing_test.go Show resolved Hide resolved
@pmalek-sumo pmalek-sumo force-pushed the routingprocessor-route-on-resource-attribute branch from 0a0579b to 3fb077f Compare October 13, 2021 10:34
@jpkrohling jpkrohling added the ready to merge Code review completed; ready to merge by maintainers label Oct 13, 2021
@andrzej-stencel
Copy link
Member

Fixes #5538

@bogdandrutu
Copy link
Member

Manually triggered CircleCI.

@bogdandrutu
Copy link
Member

Lint and unittest failure.

@pmalek-sumo pmalek-sumo force-pushed the routingprocessor-route-on-resource-attribute branch from 3fb077f to 0bb9559 Compare October 13, 2021 20:19
@pmalek-sumo
Copy link
Contributor Author

Lint and unittest failure.

Looks unrelated. I've rebased on HEAD of main.

@pmalek-sumo
Copy link
Contributor Author

Failed UT: 🤷

make[2]: Entering directory '/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/extension/awsxrayproxy'
go test -race -timeout 300s --tags="" ./...
--- FAIL: TestFactory_CreateExtension (0.00s)
    factory_test.go:71: 
        	Error Trace:	factory_test.go:71
        	Error:      	Received unexpected error:
        	            	Post "http://127.0.0.1:43755/GetSamplingRules": dial tcp 127.0.0.1:43755: connect: connection refused
        	Test:       	TestFactory_CreateExtension
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xff2f1e]

goroutine 7 [running]:
testing.tRunner.func1.2({0x1099860, 0x21ab380})
	/opt/hostedtoolcache/go/1.17.1/x64/src/testing/testing.go:1209 +0x36c
testing.tRunner.func1()
	/opt/hostedtoolcache/go/1.17.1/x64/src/testing/testing.go:1212 +0x3b6
panic({0x1099860, 0x21ab380})
	/opt/hostedtoolcache/go/1.17.1/x64/src/runtime/panic.go:1047 +0x266
github.com/open-telemetry/opentelemetry-collector-contrib/extension/awsxrayproxy.TestFactory_CreateExtension(0x0)
	/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/extension/awsxrayproxy/factory_test.go:74 +0x79e
testing.tRunner(0xc0002d5ba0, 0x11bbe38)
	/opt/hostedtoolcache/go/1.17.1/x64/src/testing/testing.go:1259 +0x230
created by testing.(*T).Run
	/opt/hostedtoolcache/go/1.17.1/x64/src/testing/testing.go:1306 +0x727
FAIL	github.com/open-telemetry/opentelemetry-collector-contrib/extension/awsxrayproxy	0.053s
FAIL
make[2]: *** [../../Makefile.Common:45: test] Error 1
make[1]: *** [Makefile:148: for-all-target-./extension/awsxrayproxy] Error 2

@tigrannajaryan
Copy link
Member

I file a bug report: #5726

@pmalek-sumo pmalek-sumo force-pushed the routingprocessor-route-on-resource-attribute branch from 0bb9559 to 52f5f2b Compare October 14, 2021 13:46
@pmalek-sumo
Copy link
Contributor Author

Rebased and once again it seems there's an unrelated test failure:

make[2]: Entering directory '/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/receiver/prometheusreceiver'
go test -race -timeout 300s --tags="" ./...
--- FAIL: TestStartTimeMetricRegex (12.72s)
    metrics_receiver_test.go:1379: 
        	Error Trace:	metrics_receiver_test.go:1379
        	            				metrics_receiver_test.go:1564
        	            				metrics_receiver_test.go:1520
        	Error:      	Not equal: 
        	            	expected: time.Date(1970, time.January, 1, 0, 6, 40, 800000000, time.UTC)
        	            	actual  : time.Date(1970, time.January, 1, 0, 0, 0, 0, time.UTC)
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,4 +1,4 @@
        	            	 (time.Time) {
        	            	- wall: (uint64) 800000000,
        	            	- ext: (int64) 62135597200,
        	            	+ wall: (uint64) 0,
        	            	+ ext: (int64) 62135596800,
        	            	  loc: (*time.Location)(<nil>)
        	Test:       	TestStartTimeMetricRegex
        	Messages:   	label_values:{value:"400" has_value:true} label_values:{value:"post" has_value:true} points:{timestamp:{seconds:1634220661 nanos:999000000} double_value:nan}
    metrics_receiver_test.go:1379: 
        	Error Trace:	metrics_receiver_test.go:1379
        	            				metrics_receiver_test.go:1564
        	            				metrics_receiver_test.go:1520
        	Error:      	Not equal: 
        	            	expected: time.Date(1970, time.January, 1, 0, 6, 40, 800000000, time.UTC)
        	            	actual  : time.Date(1970, time.January, 1, 0, 0, 0, 0, time.UTC)
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,4 +1,4 @@
        	            	 (time.Time) {
        	            	- wall: (uint64) 800000000,
        	            	- ext: (int64) 62135597200,
        	            	+ wall: (uint64) 0,
        	            	+ ext: (int64) 62135596800,
        	            	  loc: (*time.Location)(<nil>)
        	Test:       	TestStartTimeMetricRegex
        	Messages:   	label_values:{value:"200" has_value:true} label_values:{value:"post" has_value:true} points:{timestamp:{seconds:1634220661 nanos:999000000} double_value:nan}
    metrics_receiver_test.go:1379: 
        	Error Trace:	metrics_receiver_test.go:1379
        	            				metrics_receiver_test.go:1564
        	            				metrics_receiver_test.go:1520
        	Error:      	Not equal: 
        	            	expected: time.Date(1970, time.January, 1, 0, 6, 40, 800000000, time.UTC)
        	            	actual  : time.Date(1970, time.January, 1, 0, 0, 0, 0, time.UTC)
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,4 +1,4 @@
        	            	 (time.Time) {
        	            	- wall: (uint64) 800000000,
        	            	- ext: (int64) 62135597200,
        	            	+ wall: (uint64) 0,
        	            	+ ext: (int64) 62135596800,
        	            	  loc: (*time.Location)(<nil>)
        	Test:       	TestStartTimeMetricRegex
        	Messages:   	points:{timestamp:{seconds:1634220661 nanos:999000000} distribution_value:{count:-9223372036854775808 buckets:{count:-9223372036854775808}}}
    metrics_receiver_test.go:1379: 
        	Error Trace:	metrics_receiver_test.go:1379
        	            				metrics_receiver_test.go:1564
        	            				metrics_receiver_test.go:1520
        	Error:      	Not equal: 
        	            	expected: time.Date(1970, time.January, 1, 0, 6, 40, 800000000, time.UTC)
        	            	actual  : time.Date(1970, time.January, 1, 0, 0, 0, 0, time.UTC)
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,4 +1,4 @@
        	            	 (time.Time) {
        	            	- wall: (uint64) 800000000,
        	            	- ext: (int64) 62135597200,
        	            	+ wall: (uint64) 0,
        	            	+ ext: (int64) 62135596800,
        	            	  loc: (*time.Location)(<nil>)
        	Test:       	TestStartTimeMetricRegex
        	Messages:   	points:{timestamp:{seconds:1634220661 nanos:999000000} summary_value:{count:{value:-9223372036854775808} sum:{} snapshot:{percentile_values:{percentile:1 value:nan}}}}
    metrics_receiver_test.go:1384: 
        	Error Trace:	metrics_receiver_test.go:1384
        	            				metrics_receiver_test.go:1564
        	            				metrics_receiver_test.go:1520
        	Error:      	Not equal: 
        	            	expected: 11
        	            	actual  : 22
        	Test:       	TestStartTimeMetricRegex
FAIL
FAIL	github.com/open-telemetry/opentelemetry-collector-contrib/receiver/prometheusreceiver	38.324s

@tigrannajaryan
Copy link
Member

Rebased and once again it seems there's an unrelated test failure:

Please file a bug and assign to prometheusreceiver codewners to fix.

@pmalek-sumo
Copy link
Contributor Author

Please file a bug and assign to prometheusreceiver codewners to fix.

Created: #5752

prometheusreceiver doesn't seem to have an entry in CODEOWNERS: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/f89d7a10d861bf584302511f7db463247cdc3fca/.github/CODEOWNERS

@bogdandrutu
Copy link
Member

Re-triggered manually CircleCi

@tigrannajaryan tigrannajaryan merged commit 3bf6a49 into open-telemetry:main Oct 15, 2021
@sumo-drosiek sumo-drosiek deleted the routingprocessor-route-on-resource-attribute branch March 1, 2024 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

routingprocessor: use resource attributes instead of context
6 participants