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.

Sign up for GitHub

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

Fix for issues mentioned in PR #3448 #3457

Merged
merged 1 commit into from
Nov 5, 2024
Merged

Fix for issues mentioned in PR #3448 #3457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/changelog1.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@graphql-yoga/plugin-apollo-usage-report": patch
---

### Fixed
- do not set default values for `clientName` and `clientVersion`
6 changes: 6 additions & 0 deletions .changeset/changelog2.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'graphql-yoga': patch
---

### Fixed
- propagation of Yoga `version`
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"apollo"
],
"scripts": {
"build": "pnpm --filter=@graphql-yoga/graphiql run build && pnpm --filter=@graphql-yoga/render-graphiql run build && pnpm --filter=graphql-yoga run generate-graphiql-html&& pnpm --filter=graphql-yoga run inject-version && bob build",
"build": "pnpm --filter=@graphql-yoga/graphiql run build && pnpm --filter=@graphql-yoga/render-graphiql run build && pnpm --filter=graphql-yoga run generate-graphiql-html && bob build && pnpm --filter=graphql-yoga run inject-version",
"build-website": "pnpm build && cd website && pnpm build",
"changeset": "changeset",
"check": "pnpm -r run check",
Expand Down
5 changes: 2 additions & 3 deletions packages/plugins/apollo-usage-report/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ export function useApolloUsageReport(options: ApolloUsageReportOptions = {}): Pl
]),
) as YogaLogger;

let clientNameFactory: StringFromRequestFn = req =>
req.headers.get('apollographql-client-name') || 'graphql-yoga';
let clientNameFactory: StringFromRequestFn = req => req.headers.get('apollographql-client-name');

if (typeof options.clientName === 'string') {
const clientName = options.clientName;
Expand All @@ -99,7 +98,7 @@ export function useApolloUsageReport(options: ApolloUsageReportOptions = {}): Pl
}

let clientVersionFactory: StringFromRequestFn = req =>
req.headers.get('apollographql-client-version') || yoga.version;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't it nicer to have some information about the sender even if it is not dynamic because otherwise the information becomes anonymous?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so. 🙂 It will always be an anonymous call, even if we add some default value here. Additionally, if someone sends only "client name", then some unrelated version value should not be added here. As I mentioned above, it would be a mess. 🙂
It is normal behavior for Apollo Studio that the client name or version does not always appear in the trace and these metrics can be still properly filtered (as "unidentified client"). Even the official implementation of this plugin does not assign any default values. Check it out here.

req.headers.get('apollographql-client-version');

if (typeof options.clientVersion === 'string') {
const clientVersion = options.clientVersion;
Expand Down
Loading