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

Always send large traces as stats #6897

Merged

Conversation

bonnici
Copy link
Contributor

@bonnici bonnici commented Sep 12, 2022

Send large (>10mb) traces as stats always.

@netlify
Copy link

netlify bot commented Sep 12, 2022

Deploy Preview for apollo-server-docs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit a6bae36
🔍 Latest deploy log https://app.netlify.com/sites/apollo-server-docs/deploys/63374ffa28cabc00080baff7
😎 Deploy Preview https://deploy-preview-6897--apollo-server-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 12, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit a6bae36:

Sandbox Source
Apollo Server Typescript Configuration
Apollo Server Configuration

Copy link
Member

@glasser glasser left a comment

Choose a reason for hiding this comment

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

some minor tweaks suggested here; I can do them or you can, up to you

@@ -61,20 +61,30 @@ export class OurReport implements Required<IReport> {
trace,
asTrace,
referencedFieldsByType,
// The max size a trace can be before it is sent as stats. Note that the
// Apollo reporting ingress server will never store any traces over 10mb
// anyway. They will instead be converted to stats as we would do here.
Copy link
Member

Choose a reason for hiding this comment

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

I guess my caveat is that traces are always converted to stats too by the ingress — it's not an either/or.

@glasser glasser enabled auto-merge (squash) September 30, 2022 20:22
@glasser glasser merged commit e1455d5 into apollographql:version-4 Sep 30, 2022
@github-actions github-actions bot mentioned this pull request Sep 30, 2022
@glasser
Copy link
Member

glasser commented Sep 30, 2022

I don't think this requires a migration guide entry, as we don't claim this heuristic should be stable (and the end-to-end effect is basically a no-op because of what we do in our servers).

glasser pushed a commit that referenced this pull request Sep 30, 2022
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to version-4, this PR
will be updated.

⚠️⚠️⚠️⚠️⚠️⚠️

`version-4` is currently in **pre mode** so this branch has prereleases
rather than normal releases. If you want to exit prereleases, run
`changeset pre exit` on `version-4`.

⚠️⚠️⚠️⚠️⚠️⚠️

# Releases
## @apollo/server-integration-testsuite@4.0.0-rc.15

### Patch Changes

- Updated dependencies
\[[`d20842824`](d208428),
[`e1455d583`](e1455d5)]:
    -   @apollo/usage-reporting-protobuf@4.0.0-rc.2
    -   @apollo/server@4.0.0-rc.15

## @apollo/server@4.0.0-rc.15

### Patch Changes

- [#6897](#6897)
[`e1455d583`](e1455d5)
Thanks [@bonnici](https://github.com/bonnici)! - Usage reporting: always
send traces over 10MB as stats.

- Updated dependencies
\[[`d20842824`](d208428)]:
    -   @apollo/usage-reporting-protobuf@4.0.0-rc.2

## @apollo/usage-reporting-protobuf@4.0.0-rc.2

### Patch Changes

- [#6967](#6967)
[`d20842824`](d208428)
Thanks [@renovate](https://github.com/apps/renovate)! - Update
`@apollo/protobufjs` dependency to avoid false positives in
vulnerability scans
(<#6835>)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@bonnici bonnici deleted the nickmarsh/i1291/cap-trace-size branch October 3, 2022 00:28
@github-actions github-actions bot mentioned this pull request Oct 10, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants