Skip to content

feat(logging)!: add includeDeprecatedElements to the operation logging plugin - #157

Merged
cuzzlor merged 4 commits into
mainfrom
feat/deprecated-element-usage
Aug 18, 2026
Merged

feat(logging)!: add includeDeprecatedElements to the operation logging plugin#157
cuzzlor merged 4 commits into
mainfrom
feat/deprecated-element-usage

Conversation

@cuzzlor

@cuzzlor cuzzlor commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Note

Depends on MakerXStudio/graphql-core#146, now merged and published as @makerx/graphql-core@3.3.0. The lockfile here resolves it from the registry, so CI is green and this is ready to review.

This is the consumer half of porting an app-local deprecation-usage plugin into the shared libraries. graphql-core#146 adds the collector; this PR wires it into Apollo Server.

What it does

includeDeprecatedElements: true adds the @deprecated schema elements an operation used to its existing log entry:

{
  "type": "query",
  "operationName": "GetWidget",
  "oid": "...", "tid": "...", "requestId": "...",
  "deprecatedElements": [
    { "kind": "output-field", "name": "Widget.legacyName", "path": "widget.legacyName" },
    { "kind": "input-field", "name": "WidgetFilterInput.legacyId", "path": "$input.legacyId" }
  ]
}

Why an option here rather than a second plugin

The app version emitted one audit record per element. Folding into the operation entry instead means:

  • One log write per request rather than N. The originating app's audit transport uploads one HTTPS request to Azure Monitor per record, so a request touching several deprecated elements multiplied round trips.
  • Records inherit request and user metadata from the context logger, so the telemetry answers who is still using an element — the question you actually need answered before deleting a field. The app's module-scoped logger carried none of that.
  • shouldIgnore, ignoreIntrospectionQueries, resolveLogger, logLevel and augmentLogEntry all apply for free, instead of being reimplemented or going without.
  • No type key collision: type stays the OperationTypeNode and the element kind is kind, nested in the array.

Consumers wanting one record per element can still build that in ~10 lines on the public collector.

Apollo-side behaviour

  • Skipped for subsequent payloads of an incremental (@defer/@stream) response — the elements belong to the operation, not each chunk.
  • Guarded on ctx.document and ctx.operation, which are absent when a request fails to parse or validate.
  • Uses the raw ctx.request.variables. graphql-js coerces into its own values and never writes back to the request object, so the collector's pre-coercion premise holds at willSendResponse.
  • Collection failures are caught and reported via the context logger's warn; the operation is still logged, without the key. Telemetry must never turn a served request into a failed one.

Breaking changes (hence 3.0.0)

  1. @makerx/graphql-core peer narrows >=1>=3.3.0. npm 7+ errors on unsatisfiable peers, so consumers currently resolving 1.x/2.x will fail to install — a breaking change to the install contract regardless of exported signatures. Shipping it as a minor with the peer left at >=1 was the alternative, and it resolves to TypeError: collectDeprecatedElementUsage is not a function at request time, inside the one code path whose whole design goal is never to fail a request.
  2. Introspection is detected from ctx.source, not ctx.request.query. An automatic persisted query carries no query on the request once registered, so APQ introspection requests were being logged despite ignoreIntrospectionQueries: true. There is a test that replays a real APQ round trip; it fails against the old detection.

Testing

This repo's first tests — 16 of them — so --pass-with-no-tests is dropped from test:ci. They cover only what the collector's own suite in graphql-core cannot: hook wiring, the deprecatedElements shape, empty-key omission, truncation flagging (from the element cap and from a variable-walk limit), log level routing, resolveLogger precedence, shouldIgnore, introspection skipping (plain and APQ), failure containment, and that a malformed variable still fails on graphql's own coercion message and no other.

vitest.config.ts now inlines @apollo/server alongside graphql-core, so they and the test files share one graphql module instance — graphql-js rejects a GraphQLSchema built in another realm.

graphql-core and graphql are promoted to declared devDependencies; they were previously present only via npm's peer auto-install, so nothing pinned them and check-types relied on an implicit resolution.

Dependency and audit housekeeping

The lockfile now resolves @makerx/graphql-core@3.3.0 from the registry as a declared devDependency, rather than relying on npm's peer auto-install as before.

This also clears the 10 advisories npm run audit gates on, which could not be touched while ^3.3.0 was unresolvable — npm audit fix reconciles package.json against the registry, so it failed outright. Nine went within their existing ranges. The tenth was a low-severity esbuild dev-server file read on Windows, reachable only by bumping the two dev tools that pull esbuild in, so tsx goes to 4.23.12 (its esbuild range moves to ~0.28.0) and vitest to 4.1.10 with @vitest/coverage-v8 kept in lockstep. That leaves one deduped esbuild 0.28.2 and no .nsprc exception to justify later. All dev-only — this package ships rollup output with no runtime dependencies.

🤖 Generated with Claude Code

…g plugin

Adds `includeDeprecatedElements` to `graphqlOperationLoggingPlugin`,
putting the `@deprecated` schema elements an operation used onto its
existing log entry as `deprecatedElements`, so teams can tell whether a
deprecated element is safe to remove.

Riding on the operation log entry rather than emitting a record per
element means one log write per request instead of N — which matters
when the audit transport uploads per record — and each entry already
carries whatever request and user metadata the context logger adds, so
the telemetry answers who is still using an element rather than only
whether anyone is.

The collection itself is `collectDeprecatedElementUsage` from
@makerx/graphql-core, so the Apollo side stays thin: skip subsequent
payloads of an incremental response, skip anything `shouldIgnore` or
`ignoreIntrospectionQueries` filtered out, guard on document and
operation being present, and contain failures so telemetry can never
turn a served request into a failed one.

BREAKING CHANGE: the @makerx/graphql-core peer range narrows from >=1 to
>=3.3.0, since the plugin now imports collectDeprecatedElementUsage.
Consumers resolving graphql-core 1.x or 2.x will fail to install.

BREAKING CHANGE: introspection is now detected from `ctx.source` rather
than `ctx.request.query`. An automatic persisted query carries no
`query` on the request once registered, so APQ introspection requests
were previously logged despite ignoreIntrospectionQueries; they are now
correctly skipped.

Also adds this repo's first tests, and drops --pass-with-no-tests from
test:ci accordingly. graphql-core and graphql are promoted to declared
devDependencies — they were previously present only via npm's peer
auto-install, so nothing pinned them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds optional deprecated GraphQL schema-element usage to operation logs.

Changes:

  • Collects deprecated elements with configurable limits and failure containment.
  • Fixes introspection detection for persisted queries.
  • Adds tests, documentation, and dependency updates.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/plugins/graphql-operation-logging-plugin.ts Implements collection and logging.
src/plugins/graphql-operation-logging-plugin.spec.ts Adds plugin integration tests.
README.md Documents options and output.
vitest.config.ts Aligns GraphQL module instances in tests.
package.json Updates version, dependencies, and test command.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/plugins/graphql-operation-logging-plugin.ts Outdated
`deprecatedElementsTruncated` was inferred from the element count, so it
missed the two variable-walk limits entirely and misread an exactly-at-
the-cap complete result as truncated. graphql-core now reports it
directly, so take it from there.

This matters because an entry that stopped early otherwise looks
complete, and reading "nothing uses this element" off an incomplete list
is how a still-used element gets deleted. `maxVariableNodes` is the
limit a legitimate request can reach — a large batch mutation is shallow
but wide.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cuzzlor
cuzzlor requested a review from mderriey August 17, 2026 11:02
Comment thread src/plugins/graphql-operation-logging-plugin.ts Outdated
Comment thread vitest.config.ts
cuzzlor and others added 2 commits August 18, 2026 18:27
Per review: `maxVariableDepth`, `maxVariableNodes` and `maxElements` gave
no hint they only apply to deprecated element collection. Sitting beside
`adjustVariables` in a plugin whose main job is logging operations,
`maxVariableDepth` reads as a limit on the variables being logged.

  maxVariableDepth -> deprecationMaxVariableDepth
  maxVariableNodes -> deprecationMaxVariableNodes
  maxElements      -> deprecationMaxElements

Each JSDoc now opens by saying it only applies when
`includeDeprecatedElements` is true. The names passed through to
`collectDeprecatedElementUsage` keep their short forms, since that
function name already scopes them.

Also drops `deprecationReason` from the expectations and the documented
output, following its removal from graphql-core.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Now that @makerx/graphql-core 3.3.0 is published, the lockfile resolves
it from the registry as a declared devDependency rather than relying on
npm's peer auto-install. `npm ci` succeeds again, unblocking CI.

Also clears the 10 audit advisories that `npm run audit` gates on and
that could not be addressed while `^3.3.0` was unresolvable — npm audit
fix reconciles package.json against the registry, so it failed outright.

Nine went within their existing ranges. The tenth was a low-severity
esbuild dev-server file read on Windows, reachable only by bumping the
two dev tools that pull esbuild in, so tsx goes to 4.23.12 (its esbuild
range moves to ~0.28.0) and vitest to 4.1.10 with its coverage package
kept in lockstep. That leaves a single deduped esbuild 0.28.2 and no
`.nsprc` exception to justify later.

All dev-only: this package ships rollup output with no runtime
dependencies, so none of it reached consumers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cuzzlor
cuzzlor marked this pull request as ready for review August 18, 2026 10:36
@cuzzlor
cuzzlor merged commit ec6ada1 into main Aug 18, 2026
2 checks passed
@cuzzlor
cuzzlor deleted the feat/deprecated-element-usage branch August 18, 2026 10:43
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