Skip to content

feat(attributes): expand grpc attributes#460

Open
lucas-zimerman wants to merge 4 commits into
getsentry:mainfrom
lucas-zimerman:lz/grpc/missint-attributes
Open

feat(attributes): expand grpc attributes#460
lucas-zimerman wants to merge 4 commits into
getsentry:mainfrom
lucas-zimerman:lz/grpc/missint-attributes

Conversation

@lucas-zimerman

Copy link
Copy Markdown

Description

Adds attribute conventions for the Google gRPC rich error model (google.rpc.error_details.proto), under the rpc.grpc.error.* namespace:

  • rpc.grpc.error.error_info.{reason,domain,metadata.<key>}
  • rpc.grpc.error.bad_request.field_violations
  • rpc.grpc.error.retry_info.retry_delay_in_ms
  • rpc.grpc.error.debug_info.{detail,stack_entries}
  • rpc.grpc.error.precondition_failure.violations
  • rpc.grpc.error.resource_info.{resource_type,resource_name,description,owner}
  • rpc.grpc.error.quota_failure.violations

Field names and shapes were verified against google/rpc/error_details.proto (AIP-193 error model) for fidelity. Structured/repeated fields (e.g. field_violations, violations) are encoded as JSON-stringified array entries, following the existing mcp.request.argument. precedent. Attributes that may carry user- or resource-identifying data (e.g. error_info.metadata.<key>, debug_info.detail, debug_info.stack_entries, resource_info.resource_name, resource_info.owner, quota_failure.violations) are documented as PII and gated behind sendDefaultPii.

Unblocks getsentry/sentry-dart#3791.

PR Checklist

  • I have run yarn test and verified that the tests pass.
  • I have run yarn generate to generate and format code and docs.

If an attribute was added:

  • The attribute is in a namespace (e.g. nextjs.function_id, not function_id)
  • I have used the correct value for apply_scrubbing (i.e. manual or auto. Use never only for values that should never be scrubbed such as IDs)

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

Semver Impact of This PR

🟡 Minor (new features)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


New Features ✨

  • (ai) Add granular cost attributes for cache and reasoning by vgrozdanic in #461
  • (attributes) Expand grpc attributes by lucas-zimerman in #460

🤖 This preview updates automatically when you update the PR.

@lucas-zimerman

Copy link
Copy Markdown
Author

I used claude to fix the attributes.py, not sure why when I generate it manually it formats the entire file
image

@lucas-zimerman lucas-zimerman marked this pull request as ready for review July 2, 2026 01:36
@lucas-zimerman lucas-zimerman requested review from a team, Lms24, cleptric, mjq and nsdeschenes as code owners July 2, 2026 01:36

@buenaflor buenaflor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure if we should nest under rpc, gRPC themselves use a top level grpc.* (at least for metrics)

https://grpc.io/docs/guides/opentelemetry-metrics/

@Lms24 Lms24 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Theoretically, we have the error namespace (also in OTel) which "defines the shared attributes used to report an error". But I get that the attributes here are very specific to GRPC, with some exceptions.

Also to confirm: We don't capture an exception in the SDK because we explicitly only want to record the error on the span? I assume because the error is not from the application but happened wherever the procedure was executed?

I'm curious on other reviewers opinions here. Should we stay GRPC-specific or would we prefer adding a bunch of specific attributes to the error namespace?

Comment thread model/attributes/rpc/rpc__grpc__error__debug_info__detail.json Outdated
lucas-zimerman and others added 2 commits July 3, 2026 19:41
Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
…pace

gRPC's own OTel metrics conventions use a top-level grpc.* namespace,
so nest the new google.rpc error-detail attributes there instead of
under rpc.grpc.error.*, addressing review feedback on PR getsentry#460.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@lucas-zimerman

lucas-zimerman commented Jul 3, 2026

Copy link
Copy Markdown
Author

I'm not sure if we should nest under rpc, gRPC themselves use a top level grpc.* (at least for metrics)

https://grpc.io/docs/guides/opentelemetry-metrics/

renamed to grpc.* on f0c8eed

Except for rpc.grpc.status_code that already existed on sentry-conventions.

@lucas-zimerman

Copy link
Copy Markdown
Author

Theoretically, we have the error namespace (also in OTel) which "defines the shared attributes used to report an error". But I get that the attributes here are very specific to GRPC, with some exceptions.

Also to confirm: We don't capture an exception in the SDK because we explicitly only want to record the error on the span? I assume because the error is not from the application but happened wherever the procedure was executed?

I'm curious on other reviewers opinions here. Should we stay GRPC-specific or would we prefer adding a bunch of specific attributes to the error namespace?

OTel's error.* namespace is intentionally minimal: it's really just error.type (a low-cardinality classifier like an exception class name or domain code), plus a deprecated error.message. It's explicitly not meant to carry rich, structured payloads. The docs even call out that message-like fields cause "unbounded cardinality" problems on spans.

Also. Capturing an exception client-side felt low-value here. The client only has the serialized google.rpc error detail, while the real exception with full context typically already exists server-side (when that service is also Sentry-instrumented). Turning every non-OK status into a client-side exception would also add noise for cases that are expected/handled control flow rather than bugs. Worth documenting that reasoning, but happy to make it opt-in if reviewers want richer client-side capture for unattended/uninstrumented peers.

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.

3 participants