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

Kafka extract header metada #24367

Merged
merged 19 commits into from
Sep 29, 2023

Conversation

VihasMakwana
Copy link
Contributor

@VihasMakwana VihasMakwana commented Jul 18, 2023

Description: Enable support to extract headers from Kafka Messages and attach them to resource attributes. In later stages in the pipeline, different exporters can utilize this information.

Link to tracking Issue: 21729

Testing: Added test cases for logs, traces, and metrics.

Documentation:

@VihasMakwana
Copy link
Contributor Author

@MovieStoreGuy if yo can take a look at this POC

@VihasMakwana
Copy link
Contributor Author

^^ There's lot to do in case of readability, will address it later on after the discussion.

Copy link
Member

@frzifus frzifus left a comment

Choose a reason for hiding this comment

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

hi @VihasMakwana, just a few form things

receiver/kafkareceiver/header_extraction.go Outdated Show resolved Hide resolved
receiver/kafkareceiver/header_extraction.go Outdated Show resolved Hide resolved
receiver/kafkareceiver/header_extraction_test.go Outdated Show resolved Hide resolved
@VihasMakwana
Copy link
Contributor Author

VihasMakwana commented Jul 20, 2023

@frzifus thanks for your review!! Does the core logic look alright to you?

@frzifus
Copy link
Member

frzifus commented Jul 24, 2023

Just had a breef look, but at least I didn't notice anything bad 😅. Better to wait for the other reviews too.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 8, 2023

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Aug 8, 2023
@VihasMakwana
Copy link
Contributor Author

@MovieStoreGuy ping

@VihasMakwana VihasMakwana marked this pull request as ready for review August 8, 2023 05:44
@VihasMakwana VihasMakwana requested a review from a team August 8, 2023 05:44
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.

A good start so far, I've left comments with regards my concerns.

What I need to see is:

  • Changelog entry
  • User facing docs updated (With expected use case)
  • Comments addressed.

Once these are done, I am happy to approve.

receiver/kafkareceiver/config.go Outdated Show resolved Hide resolved
receiver/kafkareceiver/header_extraction.go Outdated Show resolved Hide resolved
receiver/kafkareceiver/header_extraction.go Outdated Show resolved Hide resolved
receiver/kafkareceiver/header_extraction_test.go Outdated Show resolved Hide resolved
receiver/kafkareceiver/header_extraction_test.go Outdated Show resolved Hide resolved
receiver/kafkareceiver/header_extraction_test.go Outdated Show resolved Hide resolved
receiver/kafkareceiver/kafka_receiver.go Outdated Show resolved Hide resolved
@VihasMakwana VihasMakwana force-pushed the kafka-header-extract branch from 955afb7 to a374e27 Compare August 8, 2023 13:15
@VihasMakwana VihasMakwana force-pushed the kafka-header-extract branch from a374e27 to d7e367f Compare August 8, 2023 13:16
@VihasMakwana
Copy link
Contributor Author

@MovieStoreGuy

  • Changelog entry
  • User-facing docs updated (With expected use case)
  • Comments addressed.

@VihasMakwana
Copy link
Contributor Author

Sure. Will rebase

@VihasMakwana
Copy link
Contributor Author

@MovieStoreGuy I have resolved all the conflicts and merged the main.
I guess we can move forward.

@VihasMakwana
Copy link
Contributor Author

@MovieStoreGuy ^^

@VihasMakwana
Copy link
Contributor Author

@djaglowski can you have a look as well please, if it's okay for you?

@MovieStoreGuy MovieStoreGuy added the ready to merge Code review completed; ready to merge by maintainers label Sep 19, 2023
@VihasMakwana
Copy link
Contributor Author

@codeboten can we merge this one before the 0.86 release? it looks like an excellent feature to have.

@evan-bradley evan-bradley merged commit 31df678 into open-telemetry:main Sep 29, 2023
91 checks passed
@github-actions github-actions bot added this to the next release milestone Sep 29, 2023
@VihasMakwana VihasMakwana deleted the kafka-header-extract branch September 29, 2023 14:00
jmsnll pushed a commit to jmsnll/opentelemetry-collector-contrib that referenced this pull request Nov 12, 2023
**Description:** Enable support to extract headers from Kafka Messages
and attach them to resource attributes. In later stages in the pipeline,
different exporters can utilize this information.

**Link to tracking Issue:**
[21729](open-telemetry#21729)

**Testing:** Added test cases for logs, traces, and metrics.

**Documentation:** <Describe the documentation added.>

---------

Co-authored-by: Alex Boten <[email protected]>
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 receiver/kafka
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants