Skip to content

exporter/kafkaexporter: tidy up; support partitioning in all encodings #39001

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

Merged

Conversation

axw
Copy link
Contributor

@axw axw commented Mar 27, 2025

Description

  • Move marshaler implementations to internal package
  • Simplify code to obtain a marshaler given an encoding name
  • Make marshalers independent of Sarama (Kafka client)
  • Implement a generic Kafka exporter with type parameter for signal-specific parts
  • Create more tightly scoped unit tests for encodings, independent of the general functionality of the exporter
  • Added tests for partitioning logic

Link to tracking issue

Fixes #38999

Testing

Unit tests added.

Documentation

N/A

@axw axw force-pushed the kafkaexporter-refactor-marshalers branch 2 times, most recently from eb11083 to f445e50 Compare March 28, 2025 06:20
- Move marshaler implementations to internal package
- Simplify code to obtain a marshaler given an encoding name
- Make marshalers independent of Sarama (Kafka client)
- Extract partitioning logic out of marshalers, with the exception
  of Jaeger marshalers
- Implement a generic Kafka exporter with type parameter
  for signal-specific parts
- Create more tightly scoped unit tests for encodings, independent of the
  general functionality of the exporter
@axw axw force-pushed the kafkaexporter-refactor-marshalers branch from f445e50 to f0c9700 Compare March 28, 2025 08:11
@axw axw marked this pull request as ready for review March 28, 2025 09:10
@axw axw requested a review from a team as a code owner March 28, 2025 09:10
Copy link
Contributor

@MovieStoreGuy MovieStoreGuy left a comment

Choose a reason for hiding this comment

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

Sorry, not a complete review but I wanted to give a start.

@axw
Copy link
Contributor Author

axw commented Apr 2, 2025

This isn't just a chore, it fixes #38999

I'll add a changelog

@axw axw changed the title [chore] exporter/kafkaexporter: tidy up [chore] exporter/kafkaexporter: tidy up; support partitioning in all encodings Apr 2, 2025
@axw axw changed the title [chore] exporter/kafkaexporter: tidy up; support partitioning in all encodings exporter/kafkaexporter: tidy up; support partitioning in all encodings Apr 2, 2025
Copy link
Contributor

@atoulme atoulme left a comment

Choose a reason for hiding this comment

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

LGTM

@MovieStoreGuy MovieStoreGuy merged commit 0ade7b8 into open-telemetry:main Apr 7, 2025
170 checks passed
@github-actions github-actions bot added this to the next release milestone Apr 7, 2025
@axw axw deleted the kafkaexporter-refactor-marshalers branch April 7, 2025 02:38
dmathieu pushed a commit to dmathieu/opentelemetry-collector-contrib that referenced this pull request Apr 8, 2025
open-telemetry#39001)

#### Description

- Move marshaler implementations to internal package
- Simplify code to obtain a marshaler given an encoding name
- Make marshalers independent of Sarama (Kafka client)
- Implement a generic Kafka exporter with type parameter for
signal-specific parts
- Create more tightly scoped unit tests for encodings, independent of
the general functionality of the exporter
- Added tests for partitioning logic

#### Link to tracking issue

Fixes
open-telemetry#38999

#### Testing

Unit tests added.

#### Documentation

N/A
LucianoGiannotti pushed a commit to LucianoGiannotti/opentelemetry-collector-contrib that referenced this pull request Apr 9, 2025
open-telemetry#39001)

#### Description

- Move marshaler implementations to internal package
- Simplify code to obtain a marshaler given an encoding name
- Make marshalers independent of Sarama (Kafka client)
- Implement a generic Kafka exporter with type parameter for
signal-specific parts
- Create more tightly scoped unit tests for encodings, independent of
the general functionality of the exporter
- Added tests for partitioning logic

#### Link to tracking issue

Fixes
open-telemetry#38999

#### Testing

Unit tests added.

#### Documentation

N/A
Fiery-Fenix pushed a commit to Fiery-Fenix/opentelemetry-collector-contrib that referenced this pull request Apr 24, 2025
open-telemetry#39001)

#### Description

- Move marshaler implementations to internal package
- Simplify code to obtain a marshaler given an encoding name
- Make marshalers independent of Sarama (Kafka client)
- Implement a generic Kafka exporter with type parameter for
signal-specific parts
- Create more tightly scoped unit tests for encodings, independent of
the general functionality of the exporter
- Added tests for partitioning logic

#### Link to tracking issue

Fixes
open-telemetry#38999

#### Testing

Unit tests added.

#### Documentation

N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend the support for partition_logs_by_resource_attributes to raw encoding
4 participants