Skip to content

Fix unit tests failing with franz-go update#675

Merged
rubvs merged 5 commits intomainfrom
fix-fanzgo-update
Jul 11, 2025
Merged

Fix unit tests failing with franz-go update#675
rubvs merged 5 commits intomainfrom
fix-fanzgo-update

Conversation

@rubvs
Copy link
Copy Markdown
Contributor

@rubvs rubvs commented Jul 9, 2025 •
edited
Loading

Fixes #661, #664, #665

Fix the following unit tests:

  • TestManagerMetrics
  • TestConsumerDelivery
  • TestHookLogsFailedDial
  • TestProducerMetrics

This PR allows us to update to the latest franz-go version, and thus closes #621

Relevant franz-go PRs that caused some of these issues:

@rubvs
Copy link
Copy Markdown
Contributor Author

rubvs commented Jul 9, 2025 •
edited
Loading

TestProducerMetrics/unknown_topic are only failing in CI:

2025-07-09T18:26:34.8771733Z goroutine 142778 [sync.WaitGroup.Wait]:
2025-07-09T18:26:34.8771864Z sync.runtime_SemacquireWaitGroup(0xc00157ecc0?)
2025-07-09T18:26:34.8772250Z 	/home/runner/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.4.linux-amd64/src/runtime/sema.go:110 +0x25
2025-07-09T18:26:34.8772393Z sync.(*WaitGroup).Wait(0xc00157ecc0)
2025-07-09T18:26:34.8772724Z 	/home/runner/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.4.linux-amd64/src/sync/waitgroup.go:118 +0x89
2025-07-09T18:26:34.8773081Z github.com/elastic/apm-queue/v2/kafka.(*Producer).Produce(0xc0019189a0, {0x18c25e0, 0x205c300}, {0xc0000b8380, 0x3, 0x0?})
2025-07-09T18:26:34.8773290Z 	/home/runner/work/apm-queue/apm-queue/kafka/producer.go:281 +0xa55
2025-07-09T18:26:34.8773875Z github.com/elastic/apm-queue/v2/kafka.TestProducerMetrics.func1({0x18c25e0, 0x205c300}, 0xc000ec6380, {0x18c0658, 0xc0019189a0}, {0x18c4760, 0xc001d75c70}, {0x16e1a5b, 0xd}, {0xc0011e5cb0, ...})
2025-07-09T18:26:34.8774088Z 	/home/runner/work/apm-queue/apm-queue/kafka/metrics_test.go:52 +0x352
2025-07-09T18:26:34.8774306Z github.com/elastic/apm-queue/v2/kafka.TestProducerMetrics.func5(0xc000ec6380)
2025-07-09T18:26:34.8774514Z 	/home/runner/work/apm-queue/apm-queue/kafka/metrics_test.go:200 +0xb78
2025-07-09T18:26:34.8774619Z testing.tRunner(0xc000ec6380, 0xc00178a770)
2025-07-09T18:26:34.8774967Z 	/home/runner/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.4.linux-amd64/src/testing/testing.go:1792 +0x226
2025-07-09T18:26:34.8775083Z created by testing.(*T).Run in goroutine 142723
2025-07-09T18:26:34.8775428Z 	/home/runner/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.4.linux-amd64/src/testing/testing.go:1851 +0x8f3

2025-07-09T18:26:34.8852328Z goroutine 142723 [chan receive]:
2025-07-09T18:26:34.8852510Z testing.(*T).Run(0xc00123efc0, {0x16e1a41, 0xd}, 0xc00178a770)
2025-07-09T18:26:34.8852981Z 	/home/runner/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.4.linux-amd64/src/testing/testing.go:1859 +0x91e
2025-07-09T18:26:34.8853193Z github.com/elastic/apm-queue/v2/kafka.TestProducerMetrics(0xc00123efc0)
2025-07-09T18:26:34.8853441Z 	/home/runner/work/apm-queue/apm-queue/kafka/metrics_test.go:173 +0x1eb
2025-07-09T18:26:34.8853615Z testing.tRunner(0xc00123efc0, 0x171f358)
2025-07-09T18:26:34.8853960Z 	/home/runner/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.4.linux-amd64/src/testing/testing.go:1792 +0x226
2025-07-09T18:26:34.8854066Z created by testing.(*T).Run in goroutine 1
2025-07-09T18:26:34.8854521Z 	/home/runner/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.4.linux-amd64/src/testing/testing.go:1851 +0x8f3

FIX: Related to #568

rubvs
rubvs commented Jul 9, 2025
View reviewed changes
kafka/manager_test.go
attribute.Bool("foo", true),
),
Value: 4, // end offset = 4, nothing committed
Value: 0, // end offset = 4, nothing committed
Copy link
Copy Markdown
Contributor Author

@rubvs rubvs Jul 9, 2025

Choose a reason for hiding this comment

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

With twmb/franz-go#744 lag is now relative rather than the end offset.

rubvs
rubvs commented Jul 9, 2025
View reviewed changes
kafka/metrics_test.go

// Fixes https://github.com/elastic/apm-queue/issues/464
<-time.After(time.Millisecond)
<-time.After(10 * time.Millisecond)
Copy link
Copy Markdown
Contributor Author

@rubvs rubvs Jul 9, 2025

Choose a reason for hiding this comment

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

Related to #568

@rubvs rubvs marked this pull request as ready for review July 9, 2025 19:02
@rubvs rubvs requested a review from a team as a code owner July 9, 2025 19:02
@rubvs rubvs requested review from 1pkg and marclop July 9, 2025 19:02
@rubvs rubvs mentioned this pull request Jul 9, 2025
Closed
rubvs
rubvs commented Jul 9, 2025
View reviewed changes
kafka/consumer_test.go
Topic: "name_space-topic",
Value: []byte("content"),
}

Copy link
Copy Markdown
Contributor Author

@rubvs rubvs Jul 9, 2025

Choose a reason for hiding this comment

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

The cancelled context gets propagated within the record. Create a new record to ensure a new context. Error log can be seen in #661

@rubvs rubvs mentioned this pull request Jul 9, 2025
Closed
1pkg
1pkg previously approved these changes Jul 10, 2025
View reviewed changes
kruskall
kruskall suggested changes Jul 10, 2025
View reviewed changes
go.mod Outdated
go 1.23.6
go 1.23.8

toolchain go1.24.4
Copy link
Copy Markdown
Member

@kruskall kruskall Jul 10, 2025

Choose a reason for hiding this comment

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

please remove the toolchain line

marclop
marclop previously approved these changes Jul 11, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@marclop marclop left a comment

Choose a reason for hiding this comment

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

Thanks for the updates!

@rubvs rubvs dismissed stale reviews from marclop and 1pkg via 21e209a July 11, 2025 13:00
@rubvs rubvs merged commit dece053 into main Jul 11, 2025
6 checks passed
@rubvs rubvs deleted the fix-fanzgo-update branch July 11, 2025 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@marclop marclop marclop left review comments

+2 more reviewers

@kruskall kruskall kruskall approved these changes

@1pkg 1pkg 1pkg left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Updating kfake fails TestConsumerDelivery

4 participants

@rubvs @marclop @1pkg @kruskall