fix: add ability to exclude introspection queries from complexity limits by ysmolski · Pull Request #1342 · wundergraph/graphql-go-tools

ysmolski · 2025-11-10T09:45:37Z

Allow external users to configure if they want to skip intospection querues in the complexity limits.

Follow-up in Cosmo: wundergraph/cosmo#2296

Summary by CodeRabbit

New Features
- Per-operation option to exclude GraphQL introspection fields from complexity calculations for finer-grained analysis.
Breaking Changes
- Removed the legacy built-in complexity calculator and the direct request-level complexity call; callers must use the new estimator-based complexity pathway.
Tests
- Updated and expanded complexity tests to cover introspection-included and introspection-skipped scenarios and to validate the estimator workflow.

Allow external users to configure if they want to skip intospection querues in the complexity limits.

coderabbitai · 2025-11-10T09:45:48Z

Walkthrough

Removed the legacy GraphQL complexity API and Request.CalculateComplexity; tests were updated to call the operation complexity estimator directly. The OperationComplexityEstimator gained a per-instance skipIntrospection flag propagated through its constructor and calculation entrypoints.

Changes

Cohort / File(s)	Summary
GraphQL complexity API removed `execution/graphql/complexity.go`	Deleted file: removed `ComplexityCalculator` interface, `defaultComplexityCalculator`, `ComplexityResult` / `FieldComplexityResult` types, `complexityResult` helper, and `DefaultComplexityCalculator`.
Request surface adjusted `execution/graphql/request.go`	Removed `Request.CalculateComplexity` method and its internal logic that delegated to the removed complexity API.
Request tests updated `execution/graphql/request_test.go`	Replaced calls to `Request.CalculateComplexity` with explicit parse + `OperationComplexityEstimator` usage and adapted assertions to the estimator's result shape.
Operation complexity estimator (behavior change) `v2/pkg/middleware/operation_complexity/operation_complexity.go`	Added per-instance `skipIntrospection bool` on `OperationComplexityEstimator`/visitor; changed `NewOperationComplexityEstimator` and `CalculateOperationComplexity` signatures to accept `skipIntrospection` and wired the flag to conditionally skip `__schema`/`__type` fields.
Estimator tests & benchmark `v2/pkg/middleware/operation_complexity/operation_complexity_test.go`	Tests added/updated for introspection and skip-introspection variants; test helpers refactored to use `NewOperationComplexityEstimator(skipIntrospection)`; benchmark updated to construct estimator per iteration.

Sequence Diagram(s)

sequenceDiagram
  participant RequestLayer as Request / tests
  participant Estimator as OperationComplexityEstimator
  participant Visitor as complexityVisitor
  participant Legacy as CalculateOperationComplexity (legacy)

  rect rgb(230,248,255)
    Note left of RequestLayer: Old flow (before this PR)
    RequestLayer->>Legacy: CalculateOperationComplexity(operation, definition, report)
    Legacy->>Visitor: instantiate visitor (global skip behavior)
    Visitor-->>Legacy: OperationStats, []RootFieldStats
    Legacy-->>RequestLayer: ComplexityResult (wrapper)
  end

  rect rgb(255,243,230)
    Note left of RequestLayer: New flow (after this PR)
    RequestLayer->>Estimator: NewOperationComplexityEstimator(skipIntrospection)
    RequestLayer->>Estimator: estimator.Do(operation, definition, report)
    Estimator->>Visitor: instantiate visitor (skipIntrospection flag set)
    Visitor-->>Estimator: OperationStats, []RootFieldStats
    Estimator-->>RequestLayer: OperationStats, []RootFieldStats
  end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Focus areas:
- Ensure no remaining call sites reference ComplexityCalculator, ComplexityResult, or Request.CalculateComplexity.
- Confirm updated NewOperationComplexityEstimator / CalculateOperationComplexity signatures are used consistently.
- Verify tests' expected shapes match OperationStats / RootFieldStats.
- Validate visitor's skipIntrospection correctly ignores __schema/__type.

Possibly related PRs

fix: omit introspection queries from complexity limits #1332 — Related refactor of operation complexity introspection-skipping logic; touches same operation_complexity internals and skip behavior.

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title clearly and concisely describes the main change: adding the ability to exclude introspection queries from complexity limits, which aligns with the core objectives of making this configurable for external users.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch yury/eng-8046-omit-introspection-queries-from-complexity-limits

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

execution/graphql/request.go (1)
70-85: Document the new parameter and verify backward compatibility.

The skipIntrospection parameter addition is a breaking change to the public API. Consider:

Adding godoc to document the parameter's purpose and default recommendation

Evaluating if a backward-compatible wrapper is needed for existing users

Example documentation:
// CalculateComplexity calculates the complexity of the GraphQL operation.
// If skipIntrospection is true, introspection fields (__schema, __type) are
// excluded from complexity calculations.
func (r *Request) CalculateComplexity(complexityCalculator ComplexityCalculator, schema *Schema, skipIntrospection bool) (ComplexityResult, error) {
execution/graphql/request_test.go (1)
90-150: Consider adding test coverage for skipIntrospection=true.

All existing tests correctly pass false to maintain the default behavior. Consider adding a test case that verifies skipIntrospection=true works correctly at this integration level, perhaps using the introspection query constants defined later in the file.

Example:
t.Run("should skip introspection fields when skipIntrospection is true", func(t *testing.T) {
    schema := StarwarsSchema(t)
    // Use a query with introspection fields
    request := Request{Query: `{ __schema { types { name } } }`}
    result, err := request.CalculateComplexity(DefaultComplexityCalculator, schema, true)
    assert.NoError(t, err)
    assert.Equal(t, 0, result.NodeCount, "introspection should be skipped")
})

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0e3c00f and 3bd8dd7.

📒 Files selected for processing (5)

execution/graphql/complexity.go (1 hunks)
execution/graphql/request.go (2 hunks)
execution/graphql/request_test.go (3 hunks)
v2/pkg/middleware/operation_complexity/operation_complexity.go (4 hunks)
v2/pkg/middleware/operation_complexity/operation_complexity_test.go (3 hunks)

🧰 Additional context used

🧬 Code graph analysis (5)

execution/graphql/complexity.go (2)

v2/pkg/operationreport/operationreport.go (1)

Report (9-12)

v2/pkg/middleware/operation_complexity/operation_complexity.go (1)

CalculateOperationComplexity (118-121)

execution/graphql/request_test.go (1)

execution/graphql/complexity.go (1)

DefaultComplexityCalculator (10-10)

execution/graphql/request.go (1)

execution/graphql/complexity.go (2)

ComplexityCalculator (12-14)

ComplexityResult (26-32)

v2/pkg/middleware/operation_complexity/operation_complexity_test.go (4)

v2/pkg/middleware/operation_complexity/operation_complexity.go (4)

OperationStats (36-40)

RootFieldStats (42-47)

CalculateOperationComplexity (118-121)

NewOperationComplexityEstimator (64-83)

v2/pkg/astparser/parser.go (1)

ParseGraphqlDocumentString (37-45)

v2/pkg/operationreport/operationreport.go (1)

Report (9-12)

v2/pkg/astnormalization/astnormalization.go (1)

NormalizeOperation (81-87)

v2/pkg/middleware/operation_complexity/operation_complexity.go (2)

v2/pkg/astvisitor/visitor.go (2)

Walker (65-98)

NewWalker (111-118)

v2/pkg/operationreport/operationreport.go (1)

Report (9-12)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

GitHub Check: Build and test (go 1.25 / ubuntu-latest)
GitHub Check: Build and test (go 1.25 / windows-latest)
GitHub Check: Build and test (go 1.25 / ubuntu-latest)
GitHub Check: Build and test (go 1.25 / windows-latest)

🔇 Additional comments (8)

v2/pkg/middleware/operation_complexity/operation_complexity_test.go (3)

468-497: LGTM! Comprehensive test coverage for introspection skipping.

The tests correctly verify both scenarios: counting introspection queries when skipIntrospection=false and excluding them when skipIntrospection=true. The expected complexity values appropriately reflect the different behaviors.

500-524: LGTM! Clean test helper design.

The helper function hierarchy is well-structured: runConfig provides the core implementation while run and runSkipIntrospection offer convenient wrappers for the two common test scenarios.

526-555: LGTM! Helpful comment clarifies benchmark design.

The comment explaining the relationship between the benchmark usage and production usage is valuable context. Using skipIntrospection=false as the baseline for benchmarking is appropriate.

v2/pkg/middleware/operation_complexity/operation_complexity.go (4)

54-57: LGTM! Good use of constants for introspection field names.

The constants appropriately capture the GraphQL introspection field names and eliminate magic strings in the comparison logic.

64-82: Verify backward compatibility strategy.

This is a breaking API change to a public function. Ensure that external users of NewOperationComplexityEstimator are prepared for the signature change, or consider providing a backward-compatible wrapper.

Do you want to maintain backward compatibility by providing a NewOperationComplexityEstimatorWithDefaults() that calls NewOperationComplexityEstimator(false) to preserve the existing default behavior?

118-121: Verify backward compatibility for public API.

This function signature change is also breaking. Ensure all external call sites can be updated, or consider providing a backward-compatible alternative that defaults skipIntrospection to false.

195-210: LGTM! Introspection skipping logic is correct.

The implementation correctly checks both the skipIntrospection flag and whether the field is an introspection field (__schema or __type) before calling SkipNode(). The logic placement after the nodeCountSkip directive check is appropriate.

execution/graphql/complexity.go (1)

12-24: Verify external implementations can handle interface change.

The ComplexityCalculator interface signature change is breaking for any external implementations. Ensure:

This breaking change is documented in release notes

Consider if a major version bump is needed per semantic versioning

External users implementing this interface are aware of the required update

The implementation itself is correct and properly threads the skipIntrospection parameter through to the underlying complexity calculation.

… yury/eng-8046-omit-introspection-queries-from-complexity-limits

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3bd8dd7 and 763f99c.

📒 Files selected for processing (5)

execution/graphql/complexity.go (0 hunks)
execution/graphql/request.go (0 hunks)
execution/graphql/request_test.go (2 hunks)
v2/pkg/middleware/operation_complexity/operation_complexity.go (4 hunks)
v2/pkg/middleware/operation_complexity/operation_complexity_test.go (3 hunks)

💤 Files with no reviewable changes (2)

execution/graphql/request.go
execution/graphql/complexity.go

🚧 Files skipped from review as they are similar to previous changes (1)

v2/pkg/middleware/operation_complexity/operation_complexity.go

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-29T09:35:47.969Z

Learnt from: ysmolski
Repo: wundergraph/graphql-go-tools PR: 1282
File: v2/pkg/engine/plan/visitor.go:5-5
Timestamp: 2025-08-29T09:35:47.969Z
Learning: The wundergraph/graphql-go-tools project does not support Go versions < 1.23, so compatibility concerns for features available in Go 1.21+ (like cmp.Or) should not be raised.

Applied to files:

execution/graphql/request_test.go

🧬 Code graph analysis (2)

execution/graphql/request_test.go (2)

v2/pkg/middleware/operation_complexity/operation_complexity.go (3)

NewOperationComplexityEstimator (64-83)

RootFieldStats (42-47)

OperationStats (36-40)

v2/pkg/starwars/starwars.go (1)

FileHeroWithAliasesQuery (15-15)

v2/pkg/middleware/operation_complexity/operation_complexity_test.go (4)

v2/pkg/middleware/operation_complexity/operation_complexity.go (3)

OperationStats (36-40)

RootFieldStats (42-47)

NewOperationComplexityEstimator (64-83)

v2/pkg/astparser/parser.go (1)

ParseGraphqlDocumentString (37-45)

v2/pkg/operationreport/operationreport.go (1)

Report (9-12)

v2/pkg/astnormalization/astnormalization.go (1)

NormalizeOperation (81-87)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

GitHub Check: Build and test (go 1.25 / ubuntu-latest)
GitHub Check: Build and test (go 1.25 / windows-latest)
GitHub Check: Build and test (go 1.25 / windows-latest)
GitHub Check: Build and test (go 1.25 / ubuntu-latest)

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

v2/pkg/middleware/operation_complexity/operation_complexity_test.go (1)
509-509: Consider using require.NoError for more informative test failures.

While require.False(t, report.HasErrors()) is functional, using require.NoError(t, &report) would provide more informative error messages when tests fail, as it would print the actual error details from the report.

Apply this diff:
-	require.False(t, report.HasErrors())
+	if report.HasErrors() {
+		require.NoError(t, &report)
+	}
This approach addresses the concern from the previous review comment while providing better error diagnostics.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 763f99c and 9ed60c4.

📒 Files selected for processing (1)

v2/pkg/middleware/operation_complexity/operation_complexity_test.go (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

v2/pkg/middleware/operation_complexity/operation_complexity_test.go (4)

v2/pkg/middleware/operation_complexity/operation_complexity.go (3)

OperationStats (36-40)

RootFieldStats (42-47)

NewOperationComplexityEstimator (64-83)

v2/pkg/astparser/parser.go (1)

ParseGraphqlDocumentString (37-45)

v2/pkg/operationreport/operationreport.go (1)

Report (9-12)

v2/pkg/astnormalization/astnormalization.go (1)

NormalizeOperation (81-87)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

GitHub Check: Build and test (go 1.25 / ubuntu-latest)
GitHub Check: Build and test (go 1.25 / windows-latest)
GitHub Check: Build and test (go 1.25 / ubuntu-latest)
GitHub Check: Build and test (go 1.25 / windows-latest)

🔇 Additional comments (4)

v2/pkg/middleware/operation_complexity/operation_complexity_test.go (4)

468-487: LGTM! Good coverage for introspection with complexity calculation.

The test properly validates that introspection queries contribute to complexity metrics when skipIntrospection is disabled (the default behavior).

488-497: LGTM! Proper coverage for the skip introspection feature.

The test correctly validates that introspection queries produce zero complexity when skipIntrospection is enabled, with no root field stats recorded.

517-523: LGTM! Clean test helper design.

The delegation pattern properly separates concerns, making runConfig the single source of truth while providing convenient wrappers for the two common scenarios.

533-536: LGTM! Benchmark properly reflects production usage.

The comment helpfully clarifies why the estimator is instantiated inside the benchmark loop, ensuring the measurements include the initialization cost that production code would incur.

ysmolski · 2025-11-17T08:23:19Z

@coderabbitai why i cannot merge this pr?

coderabbitai · 2025-11-17T08:24:08Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# Get detailed PR information including mergeable state and status checks
echo "=== PR Status ==="
gh pr view 1342 --json mergeable,mergeStateStatus,statusCheckRollup,reviewDecision,reviews,commits

echo -e "\n=== PR Checks ==="
gh pr checks 1342

echo -e "\n=== Branch Protection Rules ==="
gh api repos/wundergraph/graphql-go-tools/branches/master/protection --jq '{required_status_checks, required_pull_request_reviews, enforce_admins, required_linear_history, allow_force_pushes, allow_deletions, required_conversation_resolution}' 2>&1 || echo "Could not fetch branch protection rules (may require higher permissions)"

Length of output: 25103

@ysmolski The PR is blocked from merging because required reviews are still pending. Here's what I found:

Current Status:

✅ All CI checks are passing (builds, tests, lints)
✅ No merge conflicts
✅ jensneuse has approved the PR
❌ Review decision shows "REVIEW_REQUIRED"

The Issue:
You have three reviewers assigned to this PR:

devsergiy
StarpTech
Noroth

None of them have provided their review/approval yet. The branch protection rules on the master branch likely require:

A minimum number of approvals (probably 2), or
Approval from specific team members/code owners

Even though jensneuse has approved, this doesn't satisfy the requirement because you still need approval from at least one (or more) of the assigned reviewers.

To unblock the merge:

Request reviews from the assigned reviewers (devsergiy, StarpTech, or Noroth) if you haven't already
Wait for at least one of them to approve
Alternatively, if the assigned reviewers are not needed, you can remove them and the PR might become mergeable with jensneuse's approval alone (depending on your repo's branch protection settings)

Would you like me to help you request reviews from specific team members?

🤖 I have created a release *beep* *boop* --- ## [2.0.0-rc.238](v2.0.0-rc.237...v2.0.0-rc.238) (2025-11-17) ### Bug Fixes * add ability to exclude introspection queries from complexity limits ([#1342](#1342)) ([19c8693](19c8693)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).  ## Summary by CodeRabbit * **Bug Fixes** * Fixed an issue that prevented introspection queries from being properly excluded from complexity limits.

🤖 I have created a release *beep* *boop* --- ## [1.7.0](execution/v1.6.0...execution/v1.7.0) (2026-02-06) ### Features * compute static costs ([#1359](#1359)) ([004f68e](004f68e)) * improve memory management & request deduplication ([#1336](#1336)) ([938d1a9](938d1a9)) ### Bug Fixes * add ability to exclude introspection queries from complexity limits ([#1342](#1342)) ([19c8693](19c8693)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).  ## Summary by CodeRabbit * **New Features** * Compute static costs capability * Memory management improvements * Request deduplication * **Bug Fixes** * Improved query complexity filtering for introspection requests

fix: customize if omit introspection queries from complexity limits

3bd8dd7

Allow external users to configure if they want to skip intospection querues in the complexity limits.

ysmolski requested review from a team, Noroth, StarpTech, devsergiy and jensneuse as code owners November 10, 2025 09:45

coderabbitai Bot reviewed Nov 10, 2025

View reviewed changes

ysmolski changed the title ~~fix: customize if omit introspection queries from complexity limits~~ fix: add ability to exclude introspection queries from complexity limits Nov 11, 2025

Merge branch 'master' of github.com:wundergraph/graphql-go-tools into…

5d93cce

… yury/eng-8046-omit-introspection-queries-from-complexity-limits

jensneuse approved these changes Nov 11, 2025

View reviewed changes

Comment thread execution/graphql/request_test.go Outdated

Comment thread v2/pkg/middleware/operation_complexity/operation_complexity.go

clean up code from execution

763f99c

coderabbitai Bot reviewed Nov 12, 2025

View reviewed changes

Comment thread v2/pkg/middleware/operation_complexity/operation_complexity_test.go Outdated

fix error checking

9ed60c4

ysmolski requested a review from jensneuse November 12, 2025 15:56

coderabbitai Bot reviewed Nov 12, 2025

View reviewed changes

jensneuse approved these changes Nov 14, 2025

View reviewed changes

Noroth approved these changes Nov 17, 2025

View reviewed changes

ysmolski merged commit 19c8693 into master Nov 17, 2025
14 checks passed

ysmolski deleted the yury/eng-8046-omit-introspection-queries-from-complexity-limits branch November 17, 2025 08:33

This was referenced Nov 17, 2025

chore(master): release execution 1.7.0 #1343

Merged

chore(master): release 2.0.0-rc.238 #1344

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add ability to exclude introspection queries from complexity limits#1342

fix: add ability to exclude introspection queries from complexity limits#1342
ysmolski merged 4 commits intomasterfrom
yury/eng-8046-omit-introspection-queries-from-complexity-limits

ysmolski commented Nov 10, 2025 •

edited

Loading

Uh oh!

coderabbitai Bot commented Nov 10, 2025 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

ysmolski commented Nov 17, 2025

Uh oh!

coderabbitai Bot commented Nov 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ysmolski commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Pre-merge checks and finishing touches

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ysmolski commented Nov 17, 2025

Uh oh!

coderabbitai Bot commented Nov 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ysmolski commented Nov 10, 2025 •

edited

Loading

coderabbitai Bot commented Nov 10, 2025 •

edited

Loading