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

contrib/google.golang.org/grpc: add error details to span tags #2228

Merged
merged 9 commits into from
Dec 22, 2023

Conversation

eyasy1217
Copy link
Contributor

What does this PR do?

If grpc error return value contains details, add it to the span tags
Users can enable this with the WithErrorDetailTags option
Fixes #1993

Motivation

See #1993.

Reviewer's Checklist

  • Changed code has unit tests for its functionality at or near 100% coverage.
  • There is a benchmark for any new code, or changes to existing code.
  • If this interacts with the agent in a new way, a system test has been added.

For Datadog employees:

  • If this PR touches code that handles credentials of any kind, such as Datadog API keys, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

@eyasy1217 eyasy1217 marked this pull request as ready for review September 22, 2023 01:41
@eyasy1217 eyasy1217 requested a review from a team September 22, 2023 01:41
@eyasy1217 eyasy1217 marked this pull request as draft September 22, 2023 03:31
if e, ok := status.FromError(err); ok && cfg.withErrorDetailTags {
for i, d := range e.Details() {
if d, ok := d.(proto.Message); ok {
span.SetTag(tagStatusDetailsPrefix+fmt.Sprintf("_%d", i), d.String())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It appears that apm treats them as arrays if they are consecutive numeric strings, otherwise it treats them as objects.
In other words, if the index(i) is off, the array might become an object.
Therefore, I changed the key to a string like "_0" and treated it as an object.

@eyasy1217 eyasy1217 marked this pull request as ready for review September 22, 2023 04:44
@katiehockman katiehockman added the apm:ecosystem contrib/* related feature requests or bugs label Oct 6, 2023
@github-actions
Copy link

This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the stale Stuck for more than 1 month label Oct 27, 2023
@github-actions github-actions bot removed the stale Stuck for more than 1 month label Nov 15, 2023
Copy link

This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the stale Stuck for more than 1 month label Dec 19, 2023
Copy link
Contributor

@zarirhamza zarirhamza left a comment

Choose a reason for hiding this comment

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

LGTM

@zarirhamza zarirhamza enabled auto-merge (squash) December 20, 2023 15:28
@ajgajg1134 ajgajg1134 removed the stale Stuck for more than 1 month label Dec 20, 2023
@zarirhamza zarirhamza merged commit 7938f72 into DataDog:main Dec 22, 2023
100 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:ecosystem contrib/* related feature requests or bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

contrib/google.golang.org/grpc: error details missing in the span
4 participants