Skip to content

Improvements Confluent Kafka#4297

Draft
mviamari-crexi wants to merge 10 commits intoopen-telemetry:mainfrom
mviamari-crexi:confluent-kafka-improvements
Draft

Improvements Confluent Kafka#4297
mviamari-crexi wants to merge 10 commits intoopen-telemetry:mainfrom
mviamari-crexi:confluent-kafka-improvements

Conversation

@mviamari-crexi
Copy link

Description

This is a fix for some long standing issues with the ConfluentKafka span creation, particularly around consume and poll. In those cases, because the trace context isn't known until after the message is received (the context is extracted from the headers), any spans created are orphaned spans. So when the span is created for the poll operation, there is no parent trace. Then any spans created during message processing are children of that orphaned span, despite the span links, rather than part of the trace provided by the header context.

This is fixed in both cases by not creating a span for the poll or consume operations. According to the chart at the end of this semantics doc https://github.com/open-telemetry/semantic-conventions/blob/main/docs/messaging/kafka.md, the poll and consume operation spans would nominally be created by the client, rather than the producer or consumer. There are some semantics around what constitutes the client in this case, but the poll and consumer network operations are handled by librdkafka for the confluent-kafka producer and consumer.

Fixes one or more of these issues. They are all several years old, and maybe it would have been better to start from their work, but it seemed more expedient to just start from the latest and recreate the fixes.
#1966
#1674
#1678

Type of change

  • [ X ] Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Added unit tests

  • [ X ] Unit Tests w/ Tox

Does This PR Require a Core Repo Change?

  • [ X ] No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • [ X ] Followed the style guidelines of this project
  • Changelogs have been updated
  • [ X ] Unit tests have been added
  • Documentation has been updated

@linux-foundation-easycla
Copy link

CLA Not Signed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant