Skip to content

contrib: peer.service precursors for confluentinc/segmentio integrations #1979

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
merged 9 commits into from
May 12, 2023

Conversation

zarirhamza
Copy link
Contributor

@zarirhamza zarirhamza commented May 10, 2023

What does this PR do?

-This PR sets the peer.service precursor for confluentinc and segementio kafka integration spans which is the messaging.kafka.bootstrap.servers tag.
-Adds according testing and other modifications to meet requirements

zarirhamza added 3 commits May 9, 2023 14:59

Verified

This commit was signed with the committer’s verified signature.
zarirhamza Zarir Hamza
…bootstrap servers

Verified

This commit was signed with the committer’s verified signature.
zarirhamza Zarir Hamza
… tests

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@pr-commenter
Copy link

pr-commenter bot commented May 10, 2023

Benchmarks

Comparing candidate commit 26d69cd in PR branch zarir/kafka-precursor-peer.service with baseline commit dc939eb in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 18 metrics, 0 unstable metrics.

Copy link
Contributor

@rarguelloF rarguelloF left a comment

Choose a reason for hiding this comment

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

LGTM! left some suggestions

Verified

This commit was signed with the committer’s verified signature.
zarirhamza Zarir Hamza

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@zarirhamza zarirhamza marked this pull request as ready for review May 11, 2023 15:43
@zarirhamza zarirhamza requested a review from a team May 11, 2023 15:43

Verified

This commit was signed with the committer’s verified signature.
zarirhamza Zarir Hamza
For confluenctinc, stores value once in struct and adds checks to ensure deprecated wrapper still works
For segmentio, stores value once in struct and adds benchmark confirming improvement by 100 ns/op from previous commit
Fixed grammar on tag description
Copy link
Contributor

@rarguelloF rarguelloF left a comment

Choose a reason for hiding this comment

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

Some nits and changes on kafkaConfig

ajgajg1134 and others added 2 commits May 12, 2023 11:17

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was signed with the committer’s verified signature.
zarirhamza Zarir Hamza
…s formatting in segmentio tests

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Copy link
Contributor

@rarguelloF rarguelloF left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@ajgajg1134 ajgajg1134 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 making those changes!

@ajgajg1134 ajgajg1134 merged commit d1c023c into main May 12, 2023
@ajgajg1134 ajgajg1134 deleted the zarir/kafka-precursor-peer.service branch May 12, 2023 17:21
@ajgajg1134 ajgajg1134 added this to the v1.51.0 milestone May 12, 2023
katiehockman pushed a commit that referenced this pull request Jun 6, 2023
…ations (#1979)

Co-authored-by: Andrew Glaude <andrew.glaude@datadoghq.com>
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.

None yet

3 participants