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

GraphQL: Capture user-provided error extension values #4325

Open
wants to merge 3 commits into
base: graphql-error-event
Choose a base branch
from

Conversation

marcotc
Copy link
Member

@marcotc marcotc commented Jan 28, 2025

A follow up from #4177, this PR allows users to capture custom error fields (called extensions in GraphQL).

Here's an example of an error with extensions (link):

{
  "errors": [
    {
      "message": "Cannot query field \"__typenam\" on type \"Query\".",
      "locations": [
        {
          "line": 1,
          "column": 2
        }
      ],
      "extensions": {
        "code": "GRAPHQL_VALIDATION_FAILED",
        "stacktrace": [
          "GraphQLError: Cannot query field \"__typenam\" on type \"Query\".",
          "    at Object.Field (/my_project/node_modules/graphql/validation/rules/FieldsOnCorrectTypeRule.js:48:31)",
          "    ...additional lines..."
        ]
      }
    }
  ]
}

extensions can contain any arbitrary data, but are often important for error debugging.
Because extensions can contain PII, or pretty much any arbitrary user-provided information, the extensions that are allowed to be capture by dd-trace-rb have to be added to a list in the DD_TRACE_GRAPHQL_ERROR_EXTENSIONS configuration.

Change log entry
No. This will be covered by #4177.

How to test the change?
All changes have automated tests.

Copy link

github-actions bot commented Jan 28, 2025

Thank you for updating Change log entry section 👏

Visited at: 2025-01-31 22:26:52 UTC

@github-actions github-actions bot added integrations Involves tracing integrations tracing labels Jan 28, 2025
@marcotc marcotc changed the title GraphQL: Report multiple query errors GraphQL: Capture user-provided error extension values Jan 28, 2025
@marcotc marcotc changed the title GraphQL: Capture user-provided error extension values GraphQL: Capture user-provided error extension values Jan 28, 2025
@datadog-datadog-prod-us1
Copy link
Contributor

datadog-datadog-prod-us1 bot commented Jan 28, 2025

Datadog Report

Branch report: graphql-error-event-ext
Commit report: 1209efb
Test service: dd-trace-rb

✅ 0 Failed, 22132 Passed, 1477 Skipped, 5m 39.79s Total Time
⌛ 1 Performance Regression

⌛ Performance Regressions vs Default Branch (1)

  • Rails integration tests for an application with a basic route POST request with an event-triggering request in application/x-www-form-url-encoded body behaves like normal with tracing disable is expected to have 0 items - rspec 3.59s (+3.03s, +542%) - Details

@pr-commenter
Copy link

pr-commenter bot commented Jan 28, 2025

Benchmarks

Benchmark execution time: 2025-01-31 22:30:13

Comparing candidate commit 1209efb in PR branch graphql-error-event-ext with baseline commit cf6efdc in branch graphql-error-event.

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

scenario:profiler - sample timeline=false

  • 🟩 throughput [+0.389op/s; +0.403op/s] or [+6.960%; +7.200%]

scenario:tracing - Propagation - Trace Context

  • 🟩 throughput [+3738.461op/s; +3845.088op/s] or [+10.971%; +11.283%]

@marcotc marcotc force-pushed the graphql-error-event-ext branch from 2e6fa76 to 0c4de1a Compare January 28, 2025 22:47
@marcotc marcotc marked this pull request as ready for review January 31, 2025 22:20
@marcotc marcotc requested review from a team as code owners January 31, 2025 22:20
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.73%. Comparing base (cf6efdc) to head (1209efb).

Additional details and impacted files
@@                   Coverage Diff                   @@
##           graphql-error-event    #4325      +/-   ##
=======================================================
+ Coverage                97.72%   97.73%   +0.01%     
=======================================================
  Files                     1368     1370       +2     
  Lines                    83046    83127      +81     
  Branches                  4222     4228       +6     
=======================================================
+ Hits                     81157    81245      +88     
+ Misses                    1889     1882       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@buraizu buraizu left a comment

Choose a reason for hiding this comment

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

Approving with a minor update requested

@@ -883,6 +883,8 @@ The `instrument :graphql` method accepts the following parameters. Additional op
| `with_unified_tracer` | `DD_TRACE_GRAPHQL_WITH_UNIFIED_TRACER` | `Bool` | (Recommended) Enable to instrument with `UnifiedTrace` tracer for `graphql` >= v2.2, **enabling support for Endpoints list** in the Service Catalog. `with_deprecated_tracer` has priority over this. Default is `false`, using `GraphQL::Tracing::DataDogTrace` instead | `false` |
| `with_deprecated_tracer` | | `Bool` | Enable to instrument with deprecated `GraphQL::Tracing::DataDogTracing`. This has priority over `with_unified_tracer`. Default is `false`, using `GraphQL::Tracing::DataDogTrace` instead | `false` |
| `service_name` | | `String` | Service name used for graphql instrumentation | `'ruby-graphql'` |
| `error_extensions` | `DD_TRACE_GRAPHQL_ERROR_EXTENSIONS` | `Array` | List of extensions keys to include in the span event reported for GraphQL queries with errors. | `[]` |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| `error_extensions` | `DD_TRACE_GRAPHQL_ERROR_EXTENSIONS` | `Array` | List of extensions keys to include in the span event reported for GraphQL queries with errors. | `[]` |
| `error_extensions` | `DD_TRACE_GRAPHQL_ERROR_EXTENSIONS` | `Array` | List of extension keys to include in the span event reported for GraphQL queries with errors. | `[]` |

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrations Involves tracing integrations tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants