kafkaexporter: Opt-in to use franz-go client #40364

marclop · 2025-05-29T10:17:02Z

Description

Adds the option to use the franz-go client in the kafkaexporter. The
main rationale for this change is to allow users to opt-in to use a
more performant Kafka client, as the default Sarama client has been
known to have performance issues in high-throughput scenarios.

The change should be backward compatible, and significantly increases
the production performance of the kafkaexporter when in use.

$ benchstat sarama.txt franz-go.txt
goos: darwin
goarch: arm64
pkg: github.com/open-telemetry/opentelemetry-collector-contrib/exporter/kafkaexporter
cpu: Apple M2 Pro
           │ sarama.txt  │             franz-go.txt             │
           │   sec/op    │    sec/op     vs base                │
Logs-12      55.56µ ± 5%   33.40µ ±  4%  -39.88% (p=0.000 n=10)
Metrics-12   60.69µ ± 6%   39.34µ ± 17%  -35.18% (p=0.001 n=10)
Traces-12    57.09µ ± 9%   35.71µ ± 13%  -37.44% (p=0.000 n=10)
geomean      57.74µ        36.07µ        -37.53%

           │  sarama.txt  │             franz-go.txt              │
           │     B/op     │     B/op       vs base                │
Logs-12      5.792Ki ± 0%   2.207Ki ±  1%  -61.90% (p=0.000 n=10)
Metrics-12   9.545Ki ± 0%   4.339Ki ± 16%  -54.54% (p=0.000 n=10)
Traces-12    7.329Ki ± 0%   2.967Ki ±  5%  -59.52% (p=0.000 n=10)
geomean      7.400Ki        3.051Ki        -58.76%

           │ sarama.txt │            franz-go.txt            │
           │ allocs/op  │ allocs/op   vs base                │
Logs-12      55.00 ± 0%   29.00 ± 0%  -47.27% (p=0.000 n=10)
Metrics-12   58.00 ± 0%   32.00 ± 0%  -44.83% (p=0.000 n=10)
Traces-12    55.00 ± 0%   29.00 ± 0%  -47.27% (p=0.000 n=10)
geomean      55.98        29.97       -46.47%

Link to tracking issue

Improves the exporter's production performance.

Testing

Unit tested, benchmarked against a local Kafka broker.

Documentation

Updated the Readme.

Adds the option to use the franz-go client in the `kafkaexporter`. The main rationale for this change is to allow users to opt-in to use a more performant Kafka client, as the default Sarama client has been known to have performance issues in high-throughput scenarios. The change _should_ be backward compatible, and significantly increases the production performance of the `kafkaexporter` when in use. ``` $ benchstat sarama.txt franz-go.txt <region:us-east-1> goos: darwin goarch: arm64 pkg: github.com/open-telemetry/opentelemetry-collector-contrib/exporter/kafkaexporter cpu: Apple M2 Pro │ sarama.txt │ franz-go.txt │ │ sec/op │ sec/op vs base │ Logs-12 55.56µ ± 5% 33.40µ ± 4% -39.88% (p=0.000 n=10) Metrics-12 60.69µ ± 6% 39.34µ ± 17% -35.18% (p=0.001 n=10) Traces-12 57.09µ ± 9% 35.71µ ± 13% -37.44% (p=0.000 n=10) geomean 57.74µ 36.07µ -37.53% │ sarama.txt │ franz-go.txt │ │ B/op │ B/op vs base │ Logs-12 5.792Ki ± 0% 2.207Ki ± 1% -61.90% (p=0.000 n=10) Metrics-12 9.545Ki ± 0% 4.339Ki ± 16% -54.54% (p=0.000 n=10) Traces-12 7.329Ki ± 0% 2.967Ki ± 5% -59.52% (p=0.000 n=10) geomean 7.400Ki 3.051Ki -58.76% │ sarama.txt │ franz-go.txt │ │ allocs/op │ allocs/op vs base │ Logs-12 55.00 ± 0% 29.00 ± 0% -47.27% (p=0.000 n=10) Metrics-12 58.00 ± 0% 32.00 ± 0% -44.83% (p=0.000 n=10) Traces-12 55.00 ± 0% 29.00 ± 0% -47.27% (p=0.000 n=10) geomean 55.98 29.97 -46.47% ``` Signed-off-by: Marc Lopez Rubio <[email protected]>

Signed-off-by: Marc Lopez Rubio <[email protected]>

MovieStoreGuy · 2025-05-29T23:57:39Z

exporter/kafkaexporter/internal/kafkaclient/franzgo.go

+	var errs []error
+	for _, r := range result {
+		if r.Err != nil {
+			fmt.Println(r.Err.Error())


Avoid calling fmt.Println since it doesn't adhere to the logging configuration set, you must use the provided logger within the collector defined constructors.

Oh I was doing some debugging earlier and forgot to remove it, thanks for the callout

exporter/kafkaexporter/README.md

axw · 2025-05-29T23:51:21Z

exporter/kafkaexporter/metadata.yaml

+  goleak:
+    ignore:
+      top:
+        - github.com/twmb/franz-go/pkg/kfake.(*group).manage


This seems like a bug in kfake. I think we could work around it by deleting the consumer groups prior to shutting down the kfake cluster, but this seems OK. Should we open an issue against franz-go?

twmb/franz-go#1037

internal/kafka/franz_client.go

exporter/kafkaexporter/internal/kafkaclient/franzgo.go

exporter/kafkaexporter/kafka_exporter_bench_test.go

MovieStoreGuy

Speaking with @axw there is still a few things that is to be ironed out, so I'll come back once the PR is ready.

My advice for preparing to adopt a new client is:

Make it easy to review by separating out changes into their own files
- Also provides the ability to exclude them in builds if that is helpful with go build tags
- It also makes it easier to remove the older files once ready
Use the feature gate for this type of change since we are planning for it to be a one way door since there is an advantage in moving.
- It also gives users the ability to find where to reach out to on a github issue if things have gone wrong.

exporter/kafkaexporter/README.md

Signed-off-by: Marc Lopez Rubio <[email protected]>

marclop · 2025-06-02T03:35:35Z

I can address conflicts once the PR is approved

MovieStoreGuy

Everything looks great for me, @axw has already called out some work but I am not sure if it is blocking.

MovieStoreGuy · 2025-06-02T05:41:34Z

exporter/kafkaexporter/internal/kafkaclient/headers.go

+	makeHeader func(key string, value []byte) H,
+	getHeaders func(M) []H,
+	setHeadersFunc func(M, []H),


Is there anyway around needing to pass callback functions here?

I can refactor this once we permanently remove Sarama to not have this abstraction.

Currently, this ensures the same logic is applied for both adapters. I don't love it but it seems like the best approach to apply this generic flow consistently.

exporter/kafkaexporter/kafka_exporter_bench_test.go

Signed-off-by: Marc Lopez Rubio <[email protected]>

…-franz-go-client

axw

Thanks for the changes. I have some more minor questions/suggestions, otherwise LGTM

exporter/kafkaexporter/internal/kafkaclient/franzgo.go

exporter/kafkaexporter/kafka_exporter.go

exporter/kafkaexporter/kafka_exporter_test.go

internal/kafka/franz_client_test.go

Signed-off-by: Marc Lopez Rubio <[email protected]>

axw

Looks great, thank you!

…-franz-go-client

.chloggen/f_add-kafkaexporter-franz-go-client.yaml

Signed-off-by: Marc Lopez Rubio <[email protected]>

#### Description Adds the option to use the franz-go client in the `kafkaexporter`. The main rationale for this change is to allow users to opt-in to use a more performant Kafka client, as the default Sarama client has been known to have performance issues in high-throughput scenarios. The change _should_ be backward compatible, and significantly increases the production performance of the `kafkaexporter` when in use. ``` $ benchstat sarama.txt franz-go.txt goos: darwin goarch: arm64 pkg: github.com/open-telemetry/opentelemetry-collector-contrib/exporter/kafkaexporter cpu: Apple M2 Pro │ sarama.txt │ franz-go.txt │ │ sec/op │ sec/op vs base │ Logs-12 55.56µ ± 5% 33.40µ ± 4% -39.88% (p=0.000 n=10) Metrics-12 60.69µ ± 6% 39.34µ ± 17% -35.18% (p=0.001 n=10) Traces-12 57.09µ ± 9% 35.71µ ± 13% -37.44% (p=0.000 n=10) geomean 57.74µ 36.07µ -37.53% │ sarama.txt │ franz-go.txt │ │ B/op │ B/op vs base │ Logs-12 5.792Ki ± 0% 2.207Ki ± 1% -61.90% (p=0.000 n=10) Metrics-12 9.545Ki ± 0% 4.339Ki ± 16% -54.54% (p=0.000 n=10) Traces-12 7.329Ki ± 0% 2.967Ki ± 5% -59.52% (p=0.000 n=10) geomean 7.400Ki 3.051Ki -58.76% │ sarama.txt │ franz-go.txt │ │ allocs/op │ allocs/op vs base │ Logs-12 55.00 ± 0% 29.00 ± 0% -47.27% (p=0.000 n=10) Metrics-12 58.00 ± 0% 32.00 ± 0% -44.83% (p=0.000 n=10) Traces-12 55.00 ± 0% 29.00 ± 0% -47.27% (p=0.000 n=10) geomean 55.98 29.97 -46.47% ```  #### Link to tracking issue Improves the exporter's production performance.  #### Testing Unit tested, benchmarked against a local Kafka broker.  #### Documentation Updated the Readme.  --------- Signed-off-by: Marc Lopez Rubio <[email protected]>

marclop added 4 commits May 29, 2025 17:34

Add internal/kafkaclient package

43f3153

Signed-off-by: Marc Lopez Rubio <[email protected]>

kafkaexporter changes to use kafkaclient

ea30e66

Signed-off-by: Marc Lopez Rubio <[email protected]>

Update deps for affected extension/receiver

64d8041

Signed-off-by: Marc Lopez Rubio <[email protected]>

github-actions bot added receiver/kafkametrics internal/kafka extension/observer/kafkatopicsobserver exporter/kafka labels May 29, 2025

github-actions bot requested review from axw, dmitryax, MovieStoreGuy and pavolloffay May 29, 2025 10:17

marclop added 2 commits May 29, 2025 18:18

Add changelog

474bec7

Signed-off-by: Marc Lopez Rubio <[email protected]>

Address linting issues

4f95b3f

Signed-off-by: Marc Lopez Rubio <[email protected]>

github-actions bot added receiver/kafka exporter/loki Loki Exporter exporter/loadbalancing labels May 29, 2025

github-actions bot requested review from ArthurSens, dehaansa, gramidt, mar4uk and rlankfo May 29, 2025 10:36

MovieStoreGuy reviewed May 29, 2025

View reviewed changes

axw reviewed May 30, 2025

View reviewed changes

exporter/kafkaexporter/kafka_exporter_bench_test.go Outdated Show resolved Hide resolved

MovieStoreGuy reviewed May 30, 2025

View reviewed changes

exporter/kafkaexporter/README.md Outdated Show resolved Hide resolved

Address review comments

ececad8

Signed-off-by: Marc Lopez Rubio <[email protected]>

marclop changed the title ~~F/add kafkaexporter franz go client~~ kafkaexporter: Opt-in to use franz-go client May 30, 2025

marclop added 2 commits May 30, 2025 17:54

undo unrelated changes

ef132a9

Signed-off-by: Marc Lopez Rubio <[email protected]>

Add FranzSyncProducer tests

8321cd5

Signed-off-by: Marc Lopez Rubio <[email protected]>

marclop requested review from axw and MovieStoreGuy June 2, 2025 03:27

marclop marked this pull request as ready for review June 2, 2025 03:35

marclop requested a review from a team as a code owner June 2, 2025 03:35

github-actions bot assigned dehaansa Jun 2, 2025

MovieStoreGuy reviewed Jun 2, 2025

View reviewed changes

marclop added 2 commits June 2, 2025 14:41

Address more review comments

4cbd517

Signed-off-by: Marc Lopez Rubio <[email protected]>

Merge remote-tracking branch 'upstream/main' into f/add-kafkaexporter…

d2c31f4

…-franz-go-client

axw reviewed Jun 3, 2025

View reviewed changes

axw mentioned this pull request Jun 3, 2025

pkg/kfake: consumer group goroutines don't exit on cluster shutdown twmb/franz-go#1037

Closed

marclop added 2 commits June 4, 2025 09:53

Address axw review comments

a14d9c8

Signed-off-by: Marc Lopez Rubio <[email protected]>

Remove local kafka container address override

f2042fb

Signed-off-by: Marc Lopez Rubio <[email protected]>

axw approved these changes Jun 4, 2025

View reviewed changes

MovieStoreGuy approved these changes Jun 4, 2025

View reviewed changes

Merge remote-tracking branch 'upstream/main' into f/add-kafkaexporter…

1b972cb

…-franz-go-client

marclop commented Jun 4, 2025

View reviewed changes

.chloggen/f_add-kafkaexporter-franz-go-client.yaml Outdated Show resolved Hide resolved

marclop added 2 commits June 4, 2025 10:41

Update .chloggen/f_add-kafkaexporter-franz-go-client.yaml

d608901

undo formatting changes, benchmark files

5e9c85c

Signed-off-by: Marc Lopez Rubio <[email protected]>

MovieStoreGuy merged commit 7a9a390 into open-telemetry:main Jun 4, 2025
177 checks passed

github-actions bot added this to the next release milestone Jun 4, 2025

marclop deleted the f/add-kafkaexporter-franz-go-client branch June 4, 2025 03:36

marclop mentioned this pull request Jun 11, 2025

receiver/kafka: Improve performance and Kafka feature support #40628

Closed

kafkaexporter: Opt-in to use franz-go client #40364

kafkaexporter: Opt-in to use franz-go client #40364

Uh oh!

Conversation

marclop commented May 29, 2025

Description

Link to tracking issue

Testing

Documentation

Uh oh!

MovieStoreGuy May 29, 2025

Choose a reason for hiding this comment

Uh oh!

marclop May 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

axw May 29, 2025

Choose a reason for hiding this comment

Uh oh!

axw Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MovieStoreGuy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marclop commented Jun 2, 2025

Uh oh!

MovieStoreGuy left a comment

Choose a reason for hiding this comment

Uh oh!

MovieStoreGuy Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

marclop Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

axw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

axw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marclop Jun 2, 2025 •

edited

Loading