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
contrib/google.golang.org/grpc: add hostname tag #2361
Conversation
BenchmarksBenchmark execution time: 2023-12-11 13:30:02 Comparing candidate commit 862878a in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 39 metrics, 2 unstable metrics. |
if host, _, err := net.SplitHostPort(cc.Target()); err == nil { | ||
span.SetTag(ext.PeerHostname, host) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any way that cc
could be nil? If so, this could panic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think cc
should be nil
in any case where this code is being executed, but added a check just in case.
What does this PR do?
Adds
peer.hostname
tag to the google.golang.org/grpc client integration containing the hostname of the server.Motivation
The existing
out.host
tag actually contains the resolved hostname, so its always an IP address. We want to have a separate tag that contains the actual hostname.Reviewer's Checklist
For Datadog employees:
@DataDog/security-design-and-guidance
.Unsure? Have a question? Request a review!