feat(browser): Add `graphqlClientIntegration` #13783

Zen-cronic · 2024-09-25T02:39:03Z

Resolves #13399

Todo:

~~Support fetch spans and tests~~
~~Support shorthand graphql queries (i.e., without a name)~~ Moved to later
~~Enhance breadcrumb data~~

If you've added code that should be tested, please add tests.
Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

packages/browser/src/integrations/graphqlClient.ts

packages/browser/src/tracing/request.ts

packages/utils/src/instrument/fetch.ts

Zen-cronic · 2024-09-26T16:07:17Z

~~Todo: fix failing fetch tests by removing body from non-graphql requests~~

packages/utils/src/graphql.ts

packages/browser/src/integrations/graphqlClient.ts

packages/browser/src/tracing/request.ts

Zen-cronic · 2024-10-18T00:46:23Z

Just wanted to inform about the failing tests:

size limit exceeded by 3 bytes! (CDN Bundle (incl. Tracing, Replay))
all the playwright tests related to bundle (e.g., bundle_min, bundle_replay) have been failing with a timeout

dev-packages/browser-integration-tests/suites/integrations/graphqlClient/xhr/test.ts

packages/browser/src/integrations/breadcrumbs.ts

packages/browser/src/tracing/request.ts

packages/browser/src/integrations/graphqlClient.ts

mydea

I finally found the time to review this in depth. Overall, I really like it, I left some comments with requests for changes. Mostly it is about moving stuff around, and avoiding any graphql-specific code in the "general" places. Also, please note that if you rebase this in develop, there are/will be a bunch of conflicts - this is mostly because the utils package is deprecated and all that code was moved to core.

Zen-cronic · 2024-12-20T03:54:07Z

noted, i'll implement the feedback. thanks for the comprehensive review!

update span hook
update breadcrumb hook
resolve rebase conflicts

will move on with the conflicts if the current implementation checks out

billyvg · 2025-01-07T16:41:34Z

packages/browser/src/integrations/graphqlClient.ts

@@ -0,0 +1,121 @@
+import { SENTRY_XHR_DATA_KEY } from '@sentry-internal/browser-utils';
+import { getBodyString } from '@sentry-internal/replay';


@mydea should this util be in the replay package?

if we use/need it here, I would move it to @sentry-internal/browser-utils, and use it from there both in replay and here!

otherwise, this will lead to circular dependency issues :)

packages/browser/src/integrations/graphqlClient.ts

packages/types/src/client.ts

Signed-off-by: Kaung Zin Hein <[email protected]>

- Moved fetch-related operations into the grahqlClient integration. - Used Hint types for `beforeOutgoingRequestSpan` hooks. Signed-off-by: Kaung Zin Hein <[email protected]>

Signed-off-by: Kaung Zin Hein <[email protected]>

- Also moved `NetworkMetaWarning` type. Signed-off-by: Kaung Zin Hein <[email protected]>

Signed-off-by: Kaung Zin Hein <[email protected]>

…ntegration' into feat/graphqlClientIntegration

packages/browser-utils/tsconfig.test.json

packages/browser/src/integrations/graphqlClient.ts

Signed-off-by: Kaung Zin Hein <[email protected]>

…s other than string - Moved internal `getFetchRequestArgBody` to `browser-utils`. - Added unit tests. Signed-off-by: Kaung Zin Hein <[email protected]>

Signed-off-by: Kaung Zin Hein <[email protected]>

Zen-cronic · 2025-01-29T21:42:42Z

i'm not sure what's making the webkit tests flaky for the added integration tests; locally they all pass on consecutive runs via yarn test --project='webkit' graphQLClient

- Moved replay-specific `_serializeFormData` to `browser-utils` and added test. Signed-off-by: Kaung Zin Hein <[email protected]>

Signed-off-by: Kaung Zin Hein <[email protected]>

mydea reviewed Sep 25, 2024

View reviewed changes

packages/browser/src/integrations/graphqlClient.ts Outdated Show resolved Hide resolved

mydea reviewed Sep 25, 2024

View reviewed changes

packages/browser/src/integrations/graphqlClient.ts Outdated Show resolved Hide resolved

mydea reviewed Sep 25, 2024

View reviewed changes

packages/browser/src/tracing/request.ts Outdated Show resolved Hide resolved

mydea reviewed Sep 25, 2024

View reviewed changes

packages/browser/src/tracing/request.ts Outdated Show resolved Hide resolved

Zen-cronic commented Sep 26, 2024

View reviewed changes

packages/utils/src/instrument/fetch.ts Outdated Show resolved Hide resolved

Zen-cronic commented Sep 26, 2024

View reviewed changes

packages/utils/src/instrument/fetch.ts Outdated Show resolved Hide resolved

Zen-cronic requested a review from mydea September 26, 2024 15:05

Zen-cronic commented Sep 27, 2024

View reviewed changes

packages/utils/src/graphql.ts Outdated Show resolved Hide resolved

mydea reviewed Sep 27, 2024

View reviewed changes

packages/browser/src/integrations/graphqlClient.ts Outdated Show resolved Hide resolved

mydea reviewed Sep 27, 2024

View reviewed changes

packages/browser/src/tracing/request.ts Outdated Show resolved Hide resolved

Zen-cronic requested a review from mydea September 30, 2024 13:02

mydea reviewed Dec 11, 2024

View reviewed changes

dev-packages/browser-integration-tests/suites/integrations/graphqlClient/xhr/test.ts Outdated Show resolved Hide resolved

mydea reviewed Dec 11, 2024

View reviewed changes

packages/browser/src/integrations/breadcrumbs.ts Outdated Show resolved Hide resolved

mydea reviewed Dec 11, 2024

View reviewed changes

packages/browser/src/integrations/breadcrumbs.ts Outdated Show resolved Hide resolved

mydea reviewed Dec 11, 2024

View reviewed changes

packages/browser/src/tracing/request.ts Outdated Show resolved Hide resolved

mydea reviewed Dec 11, 2024

View reviewed changes

packages/browser/src/integrations/graphqlClient.ts Outdated Show resolved Hide resolved

mydea reviewed Dec 11, 2024

View reviewed changes

packages/browser/src/integrations/graphqlClient.ts Outdated Show resolved Hide resolved

mydea reviewed Dec 11, 2024

View reviewed changes

packages/browser/src/integrations/graphqlClient.ts Show resolved Hide resolved

mydea reviewed Dec 11, 2024

View reviewed changes

packages/browser/src/integrations/graphqlClient.ts Outdated Show resolved Hide resolved

mydea requested changes Dec 11, 2024

View reviewed changes

Zen-cronic requested a review from a team as a code owner January 7, 2025 15:53

billyvg reviewed Jan 7, 2025

View reviewed changes

Zen-cronic commented Jan 7, 2025

View reviewed changes

packages/browser/src/integrations/graphqlClient.ts Outdated Show resolved Hide resolved

Zen-cronic requested a review from mydea January 7, 2025 18:58

mydea reviewed Jan 13, 2025

View reviewed changes

packages/browser/src/integrations/graphqlClient.ts Outdated Show resolved Hide resolved

mydea reviewed Jan 13, 2025

View reviewed changes

packages/types/src/client.ts Outdated Show resolved Hide resolved

mydea reviewed Jan 13, 2025

View reviewed changes

packages/types/src/client.ts Outdated Show resolved Hide resolved

Zen-cronic and others added 19 commits January 23, 2025 19:15

fix(browser): Refactor to reduce type assertions

4cb51b4

Signed-off-by: Kaung Zin Hein <[email protected]>

chore(browser): Fix lint errors

95dcbe8

Signed-off-by: Kaung Zin Hein <[email protected]>

fix(browser): Refactor span handler to use hint

efbfe1f

- Moved fetch-related operations into the grahqlClient integration. - Used Hint types for `beforeOutgoingRequestSpan` hooks. Signed-off-by: Kaung Zin Hein <[email protected]>

fix(browser): Refactor breadcrumb handlers to use hint

787b18c

Signed-off-by: Kaung Zin Hein <[email protected]>

test(browser): Add tests for getRequestPayloadXhrOrFetch

93a4853

Signed-off-by: Kaung Zin Hein <[email protected]>

refactor(browser): Remove type assertions

859a1ee

Signed-off-by: Kaung Zin Hein <[email protected]>

fix(browser): Remove unnecessary FetchInput type

ad6bc30

Signed-off-by: Kaung Zin Hein <[email protected]>

chore(browser): Remove deleted import

f4c151a

Signed-off-by: Kaung Zin Hein <[email protected]>

fix(browser): Resolve rebase conflicts

8d5550e

Signed-off-by: Kaung Zin Hein <[email protected]>

fix(core): Revert resolved rebase conflict

3e3322d

Signed-off-by: Kaung Zin Hein <[email protected]>

fix: Lint

8ec59ce

Signed-off-by: Kaung Zin Hein <[email protected]>

refactor(browser-utils): Move getBodyString

59c8fcd

- Also moved `NetworkMetaWarning` type. Signed-off-by: Kaung Zin Hein <[email protected]>

refactor(core): Move hasProp

ee8b1d7

Signed-off-by: Kaung Zin Hein <[email protected]>

chore: Merge import statements

6d0d3c5

Signed-off-by: Kaung Zin Hein <[email protected]>

refactor(browser-utils): Move FetchHint and XhrHint

db865bf

Signed-off-by: Kaung Zin Hein <[email protected]>

chore(e2e): Replace deprecated imports

2e8d2ae

Signed-off-by: Kaung Zin Hein <[email protected]>

Merge branch 'getsentry:develop' into feat/graphqlClientIntegration

07af617

Merge remote-tracking branch 'refs/remotes/origin/feat/graphqlClientI…

1405707

…ntegration' into feat/graphqlClientIntegration

Merge branch 'getsentry:develop' into feat/graphqlClientIntegration

129a774

mydea reviewed Jan 29, 2025

View reviewed changes

packages/browser-utils/tsconfig.test.json Outdated Show resolved Hide resolved

mydea reviewed Jan 29, 2025

View reviewed changes

packages/browser/src/integrations/graphqlClient.ts Outdated Show resolved Hide resolved

Zen-cronic added 4 commits January 29, 2025 14:15

Merge branch 'getsentry:develop' into feat/graphqlClientIntegration

d3ba78f

chore(browser-utils): Remove jest

4d06a2f

Signed-off-by: Kaung Zin Hein <[email protected]>

fix(browser): Change parsing the fetch req payload to allow more type…

67e2971

…s other than string - Moved internal `getFetchRequestArgBody` to `browser-utils`. - Added unit tests. Signed-off-by: Kaung Zin Hein <[email protected]>

ref(browser-utils): Use undefined assertion

200d810

Signed-off-by: Kaung Zin Hein <[email protected]>

Zen-cronic added 4 commits January 30, 2025 18:39

ref(replay): Extend ReplayLogger from core Logger

514ef47

- Moved replay-specific `_serializeFormData` to `browser-utils` and added test. Signed-off-by: Kaung Zin Hein <[email protected]>

ref(browser-utils): Revert getBodyString tests to original case

a3a36e3

Signed-off-by: Kaung Zin Hein <[email protected]>

fix(core): Make endTimestamp of FetchBreadcrumbHint optional

22c2c92

Signed-off-by: Kaung Zin Hein <[email protected]>

feat(browser): Add support for queries with operation name

e7876d7

Signed-off-by: Kaung Zin Hein <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(browser): Add `graphqlClientIntegration` #13783

feat(browser): Add `graphqlClientIntegration` #13783

Zen-cronic commented Sep 25, 2024 •

edited

Loading

Zen-cronic commented Sep 26, 2024 •

edited

Loading

Zen-cronic commented Oct 18, 2024

mydea left a comment

Zen-cronic commented Dec 20, 2024 •

edited

Loading

billyvg Jan 7, 2025

mydea Jan 13, 2025

mydea Jan 13, 2025

Zen-cronic commented Jan 29, 2025

		@@ -0,0 +1,121 @@
		import { SENTRY_XHR_DATA_KEY } from '@sentry-internal/browser-utils';
		import { getBodyString } from '@sentry-internal/replay';

feat(browser): Add graphqlClientIntegration #13783

Are you sure you want to change the base?

feat(browser): Add graphqlClientIntegration #13783

Conversation

Zen-cronic commented Sep 25, 2024 • edited Loading

Zen-cronic commented Sep 26, 2024 • edited Loading

Zen-cronic commented Oct 18, 2024

mydea left a comment

Choose a reason for hiding this comment

Zen-cronic commented Dec 20, 2024 • edited Loading

billyvg Jan 7, 2025

Choose a reason for hiding this comment

mydea Jan 13, 2025

Choose a reason for hiding this comment

mydea Jan 13, 2025

Choose a reason for hiding this comment

Zen-cronic commented Jan 29, 2025

feat(browser): Add `graphqlClientIntegration` #13783

feat(browser): Add `graphqlClientIntegration` #13783

Zen-cronic commented Sep 25, 2024 •

edited

Loading

Zen-cronic commented Sep 26, 2024 •

edited

Loading

Zen-cronic commented Dec 20, 2024 •

edited

Loading