Skip to content

Conversation

@rgaignault
Copy link
Contributor

@rgaignault rgaignault commented Jan 27, 2026

Motivation

When using Apollo's Automatic Persisted Queries (APQ) with useGETForHashedQueries: true, GraphQL requests are sent as GET instead of POST. In this case, operationName, variables, and query are passed as URL query params instead of request body.

The extractGraphQlRequestMetadata function only inspects the request body, so it misses GraphQL metadata for GET requests in this particular context. As it was requested by a customer we could support it by parsing the url.

Changes

We could support it by parsing the url.

FR:#4063

Test instructions

Checklist

  • Tested locally
  • Tested on staging
  • Added unit tests for this change.
  • Added e2e/integration tests for this change.
  • Updated documentation and/or relevant AGENTS.md file

@rgaignault rgaignault requested a review from a team as a code owner January 27, 2026 10:38
@cit-pr-commenter-54b7da
Copy link

cit-pr-commenter-54b7da bot commented Jan 27, 2026

Bundles Sizes Evolution

📦 Bundle Name Base Size Local Size 𝚫 𝚫% Status
Rum 167.88 KiB 168.26 KiB +386 B +0.22%
Rum Profiler 4.33 KiB 4.33 KiB 0 B 0.00%
Rum Recorder 24.48 KiB 24.48 KiB 0 B 0.00%
Logs 56.26 KiB 56.26 KiB 0 B 0.00%
Flagging 944 B 944 B 0 B 0.00%
Rum Slim 124.86 KiB 125.23 KiB +379 B +0.30%
Worker 23.63 KiB 23.63 KiB 0 B 0.00%
🚀 CPU Performance
Action Name Base CPU Time (ms) Local CPU Time (ms) 𝚫%
RUM - add global context 0.0046 0.004 -13.04%
RUM - add action 0.0126 0.0139 +10.32%
RUM - add error 0.0134 0.0136 +1.49%
RUM - add timing 0.0027 0.0029 +7.41%
RUM - start view 0.003 0.0034 +13.33%
RUM - start/stop session replay recording 0.0007 0.0007 0.00%
Logs - log message 0.014 0.0156 +11.43%
🧠 Memory Performance
Action Name Base Memory Consumption Local Memory Consumption 𝚫
RUM - add global context 26.01 KiB 27.04 KiB +1.03 KiB
RUM - add action 49.35 KiB 53.55 KiB +4.20 KiB
RUM - add timing 27.02 KiB 26.75 KiB -271 B
RUM - add error 54.36 KiB 58.50 KiB +4.14 KiB
RUM - start/stop session replay recording 26.51 KiB 26.12 KiB -408 B
RUM - start view 423.18 KiB 431.22 KiB +8.05 KiB
Logs - log message 45.20 KiB 50.89 KiB +5.68 KiB

🔗 RealWorld

graphQlConfig: GraphQlUrlOption
): GraphQlMetadata | undefined {
const metadata = extractGraphQlRequestMetadata(request.requestBody, graphQlConfig.trackPayload)
const metadata = extractGraphQlRequestMetadata(request.requestBody, graphQlConfig.trackPayload, request.url)
Copy link
Collaborator

Choose a reason for hiding this comment

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

💬 suggestion: ‏what about passing directly the full request to this method? instead of passing every needed request attributes

Comment on lines 88 to 93
if (requestBody && typeof requestBody === 'string') {
return extractFromBody(requestBody, trackPayload)
}
// Fallback for persisted queries
if (url) {
return extractFromUrlQueryParams(url, trackPayload)
Copy link
Collaborator

Choose a reason for hiding this comment

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

💬 suggestion: ‏Instead of having the extract methods that call the build method internally, we could:

  • rename the build method as sanitize since it seems to be the intent
  • call the sanitize method from extractGraphQlRequestMetadata , so the specific extract methods would be more focused and the higher level method would manage the execution flow
  • move the parseVariableParams to the sanitize function as well

@datadog-official
Copy link

datadog-official bot commented Jan 29, 2026

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

🎯 Code Coverage
Patch Coverage: 70.59%
Overall Coverage: 77.33% (-0.01%)

View detailed report

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 3fb3a92 | Docs | Datadog PR Page | Was this helpful? Give us feedback!

Comment on lines +160 to +167
if (rawMetadata.variables) {
try {
// Parse to validate it's valid JSON, then keep the string
JSON.parse(rawMetadata.variables)
variables = rawMetadata.variables
} catch {
// Invalid JSON in variables, ignore
}
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: do we need to validate this? Maybe we can just trust that it is correctly encoded?

If we still want this validation, I suggest to:

  • move it to extractFromUrlQueryParams, since it's only useful in this case
  • maybe introduce a tryJsonParse helper, because we already have a few usages in this file. Ex:
function tryJsonParse<T = unknown>(text: string): T | undefined {
  try { return JSON.parse(text) } catch {}
}

Then

  let graphqlBody: {
    query?: string
    operationName?: string
    variables?: unknown
  }

  try {
    graphqlBody = JSON.parse(requestBody)
  } catch {
    // Not valid JSON
    return
  }

  if (!graphqlBody) {
    return
  }

// becomes

  const graphqlBody = tryJsonParse<{
    query?: string
    operationName?: string
    variables?: unknown
  }>(requestBody)

  if (!graphqlBody) {
    return
  }

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