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

Handle empty zap.Field{} to avoid panics #581

Merged
merged 3 commits into from
Oct 4, 2024

Conversation

marclop
Copy link
Contributor

@marclop marclop commented Oct 4, 2024

This commit fixes a panic that occurs when a zap.Field{} is passed to the logger. This is done by wrapping the passed TopicLogFieldFunc in a function that checks if the field is empty and returns zap.Skip() if it is.

This commit fixes a panic that occurs when a zap.Field{} is passed to
the logger. This is done by wrapping the passed `TopicLogFieldFunc` in a
function that checks if the field is empty and returns `zap.Skip()` if
it is.

Signed-off-by: Marc Lopez Rubio <marc5.12@outlook.com>
@marclop marclop added the bug Something isn't working label Oct 4, 2024
@marclop
Copy link
Contributor Author

marclop commented Oct 4, 2024

With commit 10c4c75 the logger's encoder panics due to using an empty zap.Field{} (https://github.com/elastic/apm-queue/actions/runs/11174157829/job/31063376953?pr=581):

2024-10-04T04:47:44.8534949Z go test -race  -count=10 -timeout=60s ./...
2024-10-04T04:47:51.7799855Z ?   	github.com/elastic/apm-queue/v2	[no test files]
2024-10-04T04:48:11.7757225Z ?   	github.com/elastic/apm-queue/v2/cmd/queuebench	[no test files]
2024-10-04T04:48:13.5729793Z ?   	github.com/elastic/apm-queue/v2/metrictest	[no test files]
2024-10-04T04:48:14.9044084Z panic: unknown field type: { 0 0  <nil>}
2024-10-04T04:48:14.9044758Z 
2024-10-04T04:48:14.9045015Z goroutine 11807 [running]:
2024-10-04T04:48:14.9049320Z go.uber.org/zap/zapcore.Field.AddTo({{0x0, 0x0}, 0x0, 0x0, {0x0, 0x0}, {0x0, 0x0}}, {0x7ff5d782bf38, 0xc000a3e640})
2024-10-04T04:48:14.9053620Z 	/home/runner/go/pkg/mod/go.uber.org/zap@v1.27.0/zapcore/field.go:180 +0xe99
2024-10-04T04:48:14.9054888Z go.uber.org/zap/zapcore.addFields(...)
2024-10-04T04:48:14.9056101Z 	/home/runner/go/pkg/mod/go.uber.org/zap@v1.27.0/zapcore/field.go:210
2024-10-04T04:48:14.9057783Z go.uber.org/zap/zapcore.(*ioCore).With(0xc000792b70, {0xc000a3e600, 0x1, 0x44aa85?})
2024-10-04T04:48:14.9059133Z 	/home/runner/go/pkg/mod/go.uber.org/zap@v1.27.0/zapcore/core.go:83 +0x329
2024-10-04T04:48:14.9059878Z go.uber.org/zap.(*Logger).With(0xc000a70480, {0xc000a3e600, 0x1, 0x1})
2024-10-04T04:48:14.9060700Z 	/home/runner/go/pkg/mod/go.uber.org/zap@v1.27.0/logger.go:185 +0xde
2024-10-04T04:48:14.9061934Z github.com/elastic/apm-queue/v2/kafka.(*consumer).assigned(0xc000acef50, {0xec6925?, 0x135ff00?}, 0xc000554808, 0xc000792b40)
2024-10-04T04:48:14.9063046Z 	/home/runner/work/apm-queue/apm-queue/kafka/consumer.go:422 +0x616
2024-10-04T04:48:14.9064003Z github.com/twmb/franz-go/pkg/kgo.(*consumer).initGroup.func2({0x1677488, 0xc000ae9130}, 0xc000554808, 0xc0007929c0)
2024-10-04T04:48:14.9065121Z 	/home/runner/go/pkg/mod/github.com/twmb/franz-go@v1.17.0/pkg/kgo/consumer_group.go:343 +0x56f
2024-10-04T04:48:14.9066099Z github.com/twmb/franz-go/pkg/kgo.(*assignRevokeSession).assign.func1()
2024-10-04T04:48:14.9067669Z 	/home/runner/go/pkg/mod/github.com/twmb/franz-go@v1.17.0/pkg/kgo/consumer_group.go:795 +0x2df
2024-10-04T04:48:14.9068957Z created by github.com/twmb/franz-go/pkg/kgo.(*assignRevokeSession).assign in goroutine 11720
2024-10-04T04:48:14.9070364Z 	/home/runner/go/pkg/mod/github.com/twmb/franz-go@v1.17.0/pkg/kgo/consumer_group.go:784 +0x11d
2024-10-04T04:48:14.9071294Z FAIL	github.com/elastic/apm-queue/v2/kafka	1.325s
2024-10-04T04:48:14.9072021Z ok  	github.com/elastic/apm-queue/v2/queuecontext	1.026s
2024-10-04T04:48:14.9072847Z ok  	github.com/elastic/apm-queue/v2/systemtest	1.017s

Signed-off-by: Marc Lopez Rubio <marc5.12@outlook.com>
@marclop marclop marked this pull request as ready for review October 4, 2024 04:57
@marclop marclop requested a review from a team as a code owner October 4, 2024 04:57
Signed-off-by: Marc Lopez Rubio <marc5.12@outlook.com>
@marclop marclop merged commit 5aa25ad into main Oct 4, 2024
6 checks passed
@marclop marclop deleted the b/fix-panic-on-empty-zap.Field branch October 4, 2024 05:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants