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.

Sign up for GitHub

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

Jump to bottom

contrib/google.golang.org/{grpc, grpc.v12}: add support for WithSpanOptions #1159

Merged
merged 8 commits into from
Mar 20, 2023
Merged

contrib/google.golang.org/{grpc, grpc.v12}: add support for WithSpanOptions #1159

merged 8 commits into from
Mar 20, 2023

Conversation

mackjmr
Copy link
Member

@mackjmr mackjmr commented Feb 4, 2022

This PR add's support for WithSpanOptions for both contrib/google.golang.org/grpc and contrib/google.golang.org/grpc.v12

Fixes: #1125

@mackjmr mackjmr requested a review from a team February 4, 2022 14:33
felixge
felixge reviewed Feb 11, 2022
View reviewed changes
Copy link
Member

@felixge felixge 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 this PR. IIRC @talig was looking for something like this in order to add custom tags to all spans.

Anyway, just a quick comment from me. I'll leave it to @DataDog/tracing-go to review the full PR.

contrib/google.golang.org/grpc.v12/grpc.go Outdated Show resolved Hide resolved
@dianashevchenko dianashevchenko added this to the Triage milestone Feb 17, 2022
@pr-commenter
Copy link

pr-commenter bot commented Jan 19, 2023 •
edited

Benchmarks

Comparing candidate commit 61b673f in PR branch mackjmr/grpc-with-span-options with baseline commit 7e75bfd in branch main.

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

dianashevchenko
dianashevchenko approved these changes Mar 16, 2023
View reviewed changes
Copy link
Contributor

@dianashevchenko dianashevchenko left a comment

Choose a reason for hiding this comment

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

LGTM but I will also ask @ajgajg1134 to look through real quick

ajgajg1134
ajgajg1134 approved these changes Mar 16, 2023
View reviewed changes
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.

Also looks good to me!

@dianashevchenko dianashevchenko modified the milestones: Triage, v1.49.0 Mar 16, 2023
@dianashevchenko dianashevchenko merged commit 53225b6 into main Mar 20, 2023
@dianashevchenko dianashevchenko deleted the mackjmr/grpc-with-span-options branch March 20, 2023 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@felixge felixge felixge left review comments

@gbbr gbbr gbbr left review comments

@ajgajg1134 ajgajg1134 ajgajg1134 approved these changes

@dianashevchenko dianashevchenko dianashevchenko approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.49.0
Development

Successfully merging this pull request may close these issues.

proposal: contrib/google.golang.org/grpc: add WithSpanOptions
6 participants
@mackjmr @felixge @ajgajg1134 @gbbr @dianashevchenko @zarirhamza