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

JSON based logger #642

Merged
merged 10 commits into from
Feb 12, 2025
Merged

JSON based logger #642

merged 10 commits into from
Feb 12, 2025

Conversation

ardatan
Copy link
Member

@ardatan ardatan commented Feb 12, 2025

Blocked by ardatan/graphql-mesh#8375

By default

  • It checks NODE_ENV=production and uses pretty if it is not production, otherwise logs in JSON format

According to LOG_FORMAT=json or LOG_FORMAT=pretty, it chooses the output format.

New JSON based logger compatible with pino-pretty;

With pino-pretty;
image

Without pino-pretty;
image

Copy link

coderabbitai bot commented Feb 12, 2025

Warning

Rate limit exceeded

@ardatan has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 55 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 2dc4c18 and 067ab02.

📒 Files selected for processing (2)
  • .changeset/big-waves-prove.md (1 hunks)
  • packages/gateway/src/commands/handleFork.ts (1 hunks)
📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Introduced a configurable JSON logging option that outputs structured, detailed logs based on environment settings.
  • Chores

    • Updated key dependencies for improved integration with GraphQL Mesh and Hive services.
    • Added version resolutions to ensure consistency across the project.
  • Refactor

    • Standardized the logging approach across components, enhancing context and clarity in error, debug, and process management messages.

Walkthrough

This pull request makes extensive modifications to the project’s logging system. It introduces a new JSON-based logger and updates various logging calls to use object parameters rather than plain strings. The changes replace DefaultLogger with a new flexible logger instantiation using getDefaultLogger(), which conditionally returns either a JSON or a pretty logger based on environment settings. Additionally, several dependency versions and resolutions have been updated, and a new logger package (@graphql-hive/logger-json) has been added with proper path mappings and exports.

Changes

Files Change Summary
.changeset/big-waves-prove.md, .../fusion-runtime/src/federation/supergraph.ts, .../fusion-runtime/src/utils.ts, .../gateway/src/bin.ts, .../gateway/src/cli.ts, .../gateway/src/commands/handleLoggingOption.ts, .../importer/src/debug.ts, .../runtime/src/createGatewayRuntime.ts, .../runtime/src/plugins/useRequestId.ts Updated logger instantiation across the codebase: replaced DefaultLogger with getDefaultLogger(), and restructured logger.child calls to accept objects (e.g., { requestId }, { subgraph }) for more descriptive logging.
.../fusion-runtime/tests/polling.test.ts, .../runtime/tests/graphos.test.ts, .../transports/ws/tests/ws.spec.ts Modified test files to update logger imports/instantiation from DefaultLogger to getDefaultLogger() or JSONLogger.
packages/logger-json/package.json, packages/logger-json/src/index.ts, .../runtime/src/index.ts Introduced a new @graphql-hive/logger-json package featuring a JSONLogger class for structured logging; exported from the runtime index.
.../plugins/opentelemetry/src/plugin.ts, .../runtime/src/fetchers/graphos.ts, .../runtime/src/getReportingPlugin.ts, .../runtime/src/plugins/useDelegationPlanDebug.ts, .../runtime/src/plugins/useFetchDebug.ts, .../runtime/src/plugins/useSubgraphExecuteDebug.ts Enhanced logging context in plugins and fetchers by updating logger.child calls to use object parameters and by adding parameters (e.g., requestId) for improved traceability.
.../transports/http-callback/src/index.ts, .../transports/ws/src/index.ts, .../gateway/src/commands/handleFork.ts, .../gateway/src/commands/proxy.ts Updated transport and command modules to instantiate loggers with enriched context (e.g., { executor, subscriptionId }, { plugin: 'Hive CDN' }, { worker }) for domain-specific logging.
package.json, tsconfig.json, .changeset/@graphql-hive_gateway-runtime-642-dependencies.md, .../runtime/package.json Updated dependency versions and resolutions: added new resolution entries, updated patch versions for GraphQL Mesh/Hive packages, added a dependency for @graphql-hive/logger-json, and included a path mapping in TypeScript configuration.
.../runtime/src/getDefaultLogger.ts Added a new utility function getDefaultLogger() that selects between JSONLogger and DefaultLogger based on environment variables and provided options.

Sequence Diagram(s)

sequenceDiagram
    participant App as Application Code
    participant GDL as getDefaultLogger()
    participant JSON as JSONLogger
    participant Pretty as DefaultLogger

    App->>GDL: Call getDefaultLogger(opts)
    alt LOG_FORMAT is "json" or NODE_ENV is production
        GDL-->>JSON: Instantiate JSONLogger(opts)
        JSON-->>App: Return JSONLogger instance
    else
        GDL-->>Pretty: Instantiate DefaultLogger(opts)
        Pretty-->>App: Return DefaultLogger instance
    end
Loading

Possibly related PRs

Suggested reviewers

  • dotansimha

Poem

Hop through the lines of code, I delight,
New logs formatted crisp and bright.
JSON whispers secrets in each log call,
With keys and objects that enthrall.
In fields of code, I skip with cheer—
A happy rabbit logging far and near!
🐇💻


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@theguild-bot
Copy link
Collaborator

theguild-bot commented Feb 12, 2025

🚀 Snapshot Release (alpha)

The latest changes of this PR are available as alpha on npm (based on the declared changesets):

Package Version Info
@graphql-tools/batch-delegate 9.0.30-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-tools/batch-execute 9.0.12-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-tools/delegate 10.2.12-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-tools/executor-common 0.0.2-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-tools/executor-graphql-ws 2.0.2-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-tools/executor-http 1.2.7-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-tools/federation 3.1.2-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-mesh/fusion-runtime 0.11.0-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-hive/gateway 1.10.0-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-hive/importer 1.0.1-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-hive/logger-json 0.0.1-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-mesh/hmac-upstream-signature 1.2.20-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-mesh/plugin-opentelemetry 1.3.41-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-mesh/plugin-prometheus 1.3.29-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-hive/gateway-runtime 1.4.13-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-tools/stitch 9.4.17-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-tools/stitching-directives 3.1.27-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-mesh/transport-common 0.7.29-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-mesh/transport-http 0.6.33-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-mesh/transport-http-callback 0.5.20-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-mesh/transport-ws 1.0.3-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎
@graphql-tools/wrap 10.0.30-alpha-067ab02c157b3c030fc14630e30301be6a1fd573 npm ↗︎ unpkg ↗︎

@theguild-bot
Copy link
Collaborator

theguild-bot commented Feb 12, 2025

🚀 Snapshot Release (Binary for Linux-ARM64)

The latest changes of this PR are available for download (based on the declared changesets).

Download

@theguild-bot
Copy link
Collaborator

theguild-bot commented Feb 12, 2025

🚀 Snapshot Release (Binary for Linux-X64)

The latest changes of this PR are available for download (based on the declared changesets).

Download

@theguild-bot
Copy link
Collaborator

theguild-bot commented Feb 12, 2025

🚀 Snapshot Release (Binary for macOS-ARM64)

The latest changes of this PR are available for download (based on the declared changesets).

Download

@theguild-bot
Copy link
Collaborator

theguild-bot commented Feb 12, 2025

🚀 Snapshot Release (Bun Docker Image)

The latest changes of this PR are available as image on GitHub Container Registry (based on the declared changesets):

ghcr.io/graphql-hive/gateway:1.10.0-alpha-067ab02c157b3c030fc14630e30301be6a1fd573-bun

@theguild-bot
Copy link
Collaborator

theguild-bot commented Feb 12, 2025

🚀 Snapshot Release (Node Docker Image)

The latest changes of this PR are available as image on GitHub Container Registry (based on the declared changesets):

ghcr.io/graphql-hive/gateway:1.10.0-alpha-067ab02c157b3c030fc14630e30301be6a1fd573

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 14

🔭 Outside diff range comments (1)
packages/transports/http-callback/src/index.ts (1)

1-1: 💡 Verification agent

🧩 Analysis chain

Please add changeset and documentation.

This PR introduces a new JSON-based logging feature across multiple packages. Please ensure:

  1. A changeset file is created using yarn changeset with a minor version bump, describing the new logging feature with examples.
  2. Documentation is added to the console project explaining the new logging format and its integration with pino-pretty.

Let me check if there's already documentation for this feature:

Let me check if there's a Linear issue linked to this PR:


🏁 Script executed:

#!/bin/bash
# Description: Check PR description for Linear issue reference
gh pr view 642 --json body | jq -r '.body' | grep -i 'GW-'

Length of output: 145


🌐 Web query:

Search for documentation about JSON logging in graphql-hive/console repository

💡 Result:

To configure JSON logging with GraphQL Hive's gateway, you can implement a custom logger that outputs in JSON format. While the core Hive documentation doesn't explicitly mention built-in JSON logging, the configuration system allows integration with external logging solutions through these methods:

Logging Configuration

Gateway Setup (via defineConfig):

import { defineConfig } from '@graphql-hive/gateway'
import { createLogger } from 'your-json-logger-library' 

export const gatewayConfig = defineConfig({
  logging: createLogger({ format: 'json' }) // Example with hypothetical JSON logger
})

Key capabilities:

  • Supports log levels: info, debug, warn, error [4]
  • Allows complete logger replacement with custom implementations [4]
  • Disables default console logging when using custom loggers [4]

For CLI-related logging (hive.json configuration), the documentation shows configuration through:

{
  "registry": {
    "endpoint": "http://localhost:3001/graphql",
    "accessToken": "<your-token>"
  }
}

Though not directly mentioning JSON logging here, you could pipe CLI output through standard JSON conversion tools in your execution environment[7].

The most straightforward path for JSON logging would involve:

  1. Choosing a Node.js JSON logger like Pino or Winston
  2. Configuring it to output structured logs
  3. Passing the logger instance to Hive Gateway's configuration[4]

This approach maintains compatibility with Hive's logging interface while enabling JSON format through external tooling.

Citations:


Missing Changeset and Documentation for New JSON Logging Feature

The PR introduces a JSON-based logging feature that affects multiple packages but currently lacks:

  • A changeset file (using yarn changeset) with a minor version bump and description of the new feature.
  • Updates to the console project’s documentation detailing the new JSON logging format and its integration with pino-pretty.
  • A reference to a Linear issue (prefixed with GW-*).

Please address these items.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 26a9c5b and 44d18a9.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (27)
  • .changeset/@graphql-hive_gateway-642-dependencies.md (1 hunks)
  • .changeset/big-waves-prove.md (1 hunks)
  • package.json (1 hunks)
  • packages/fusion-runtime/src/federation/supergraph.ts (1 hunks)
  • packages/fusion-runtime/src/utils.ts (4 hunks)
  • packages/fusion-runtime/tests/polling.test.ts (2 hunks)
  • packages/gateway/package.json (1 hunks)
  • packages/gateway/src/bin.ts (2 hunks)
  • packages/gateway/src/cli.ts (4 hunks)
  • packages/gateway/src/commands/handleLoggingOption.ts (2 hunks)
  • packages/importer/src/debug.ts (1 hunks)
  • packages/logger-json/package.json (1 hunks)
  • packages/logger-json/src/index.ts (1 hunks)
  • packages/plugins/opentelemetry/src/plugin.ts (1 hunks)
  • packages/runtime/src/createGatewayRuntime.ts (5 hunks)
  • packages/runtime/src/fetchers/graphos.ts (2 hunks)
  • packages/runtime/src/getReportingPlugin.ts (1 hunks)
  • packages/runtime/src/index.ts (1 hunks)
  • packages/runtime/src/plugins/useDelegationPlanDebug.ts (5 hunks)
  • packages/runtime/src/plugins/useFetchDebug.ts (1 hunks)
  • packages/runtime/src/plugins/useRequestId.ts (1 hunks)
  • packages/runtime/src/plugins/useSubgraphExecuteDebug.ts (1 hunks)
  • packages/runtime/tests/graphos.test.ts (2 hunks)
  • packages/transports/http-callback/src/index.ts (1 hunks)
  • packages/transports/ws/src/index.ts (1 hunks)
  • packages/transports/ws/tests/ws.spec.ts (2 hunks)
  • tsconfig.json (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
`packages/**`: In this directory we keep all packages releva...

packages/**: In this directory we keep all packages relevant to the gateway.

In most cases, when changes are made to the source code / dependencies in these directories, we also require to have a corresponding changeset file, created using yarn changeset with a full description on the changes with an example.
Also, when new features are added the packages, the changeset file should mention minor semver change. When a bug is fixed, patch needs to be used. The major bump is used for a PR that has breaking changes.

  • packages/runtime/src/index.ts
  • packages/gateway/package.json
  • packages/runtime/tests/graphos.test.ts
  • packages/plugins/opentelemetry/src/plugin.ts
  • packages/transports/http-callback/src/index.ts
  • packages/runtime/src/getReportingPlugin.ts
  • packages/transports/ws/src/index.ts
  • packages/runtime/src/fetchers/graphos.ts
  • packages/fusion-runtime/tests/polling.test.ts
  • packages/gateway/src/bin.ts
  • packages/transports/ws/tests/ws.spec.ts
  • packages/runtime/src/plugins/useFetchDebug.ts
  • packages/importer/src/debug.ts
  • packages/fusion-runtime/src/utils.ts
  • packages/fusion-runtime/src/federation/supergraph.ts
  • packages/runtime/src/plugins/useRequestId.ts
  • packages/gateway/src/commands/handleLoggingOption.ts
  • packages/logger-json/package.json
  • packages/logger-json/src/index.ts
  • packages/runtime/src/plugins/useSubgraphExecuteDebug.ts
  • packages/gateway/src/cli.ts
  • packages/runtime/src/plugins/useDelegationPlanDebug.ts
  • packages/runtime/src/createGatewayRuntime.ts
`**`: For all PRs, we would like to verify that a Linear iss...

**: For all PRs, we would like to verify that a Linear issue is linked. Check if there's a mention for "GW-*" in the PR description, and the us know when it's missing.

In most cases, when a PR adds a new functionality, we would like to make sure it has documentation in the corresponding console project.
Use the global knowledge feature to search for PRs in graphql-hive/console repository that adds documentation. Suggest to create a follow up issue to add the missing documentation.

  • packages/runtime/src/index.ts
  • packages/gateway/package.json
  • packages/runtime/tests/graphos.test.ts
  • packages/plugins/opentelemetry/src/plugin.ts
  • packages/transports/http-callback/src/index.ts
  • packages/runtime/src/getReportingPlugin.ts
  • packages/transports/ws/src/index.ts
  • packages/runtime/src/fetchers/graphos.ts
  • package.json
  • packages/fusion-runtime/tests/polling.test.ts
  • packages/gateway/src/bin.ts
  • packages/transports/ws/tests/ws.spec.ts
  • tsconfig.json
  • packages/runtime/src/plugins/useFetchDebug.ts
  • packages/importer/src/debug.ts
  • packages/fusion-runtime/src/utils.ts
  • packages/fusion-runtime/src/federation/supergraph.ts
  • packages/runtime/src/plugins/useRequestId.ts
  • packages/gateway/src/commands/handleLoggingOption.ts
  • packages/logger-json/package.json
  • packages/logger-json/src/index.ts
  • packages/runtime/src/plugins/useSubgraphExecuteDebug.ts
  • packages/gateway/src/cli.ts
  • packages/runtime/src/plugins/useDelegationPlanDebug.ts
  • packages/runtime/src/createGatewayRuntime.ts
`packages/gateway/**`: The main runtime for the Hive gateway...

packages/gateway/**: The main runtime for the Hive gateway.
This package is CLI that runs the gateway and configures the internals of the gateway function.

  • packages/gateway/package.json
  • packages/gateway/src/bin.ts
  • packages/gateway/src/commands/handleLoggingOption.ts
  • packages/gateway/src/cli.ts
`packages/plugins/**`: The directories inside this dir are g...

packages/plugins/**: The directories inside this dir are gateway plugins. Plugins adds functionality to the GraphQL gateway, at the runtime level. Plugins included are authorization, authentication, caching, persisted documents and more.

  • packages/plugins/opentelemetry/src/plugin.ts
`packages/transports/**`: The transports configures how to d...

packages/transports/**: The transports configures how to deal with requests that are made from the GraphQL gateway to the GraphQL subgraphs. Transports manipulates the calls, headers, protocols and others.

  • packages/transports/http-callback/src/index.ts
  • packages/transports/ws/src/index.ts
  • packages/transports/ws/tests/ws.spec.ts
🪛 Biome (1.9.4)
packages/runtime/src/plugins/useFetchDebug.ts

[error] 15-15: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)

packages/logger-json/src/index.ts

[error] 158-158: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)


[error] 163-163: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)


[error] 125-125: Use Number.POSITIVE_INFINITY instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.POSITIVE_INFINITY instead.

(lint/style/useNumberNamespace)


[error] 128-128: Reassigning a function parameter is confusing.

The parameter is declared here:

Use a local variable instead.

(lint/style/noParameterAssign)


[error] 131-131: Reassigning a function parameter is confusing.

The parameter is declared here:

Use a local variable instead.

(lint/style/noParameterAssign)


[error] 147-147: This let declares a variable that is only assigned once.

'messageArg' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

packages/runtime/src/plugins/useSubgraphExecuteDebug.ts

[error] 18-18: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)

⏰ Context from checks skipped due to timeout of 90000ms (30)
  • GitHub Check: Examples / Convert operation-field-permissions
  • GitHub Check: Examples / Convert subscriptions-with-transforms
  • GitHub Check: Examples / Convert openapi-subscriptions
  • GitHub Check: Examples / Convert openapi-javascript-wiki
  • GitHub Check: Examples / Convert openapi-arg-rename
  • GitHub Check: Examples / Convert openapi-additional-resolvers
  • GitHub Check: Examples / Convert federation-subscriptions-passthrough
  • GitHub Check: Examples / Convert apq-subgraphs
  • GitHub Check: Examples / Convert federation-mixed
  • GitHub Check: Examples / Convert extra-fields
  • GitHub Check: Binary built on ubuntu-24.04-arm
  • GitHub Check: Binary built on windows-latest
  • GitHub Check: Binary built on macos-14
  • GitHub Check: Binary built on macos-13
  • GitHub Check: Bun Docker image
  • GitHub Check: Binary built on ubuntu-latest
  • GitHub Check: Node Docker image
  • GitHub Check: E2E / Bun Docker on Ubuntu
  • GitHub Check: E2E / Bun on Ubuntu
  • GitHub Check: E2E / Node Binary on Windows
  • GitHub Check: E2E / Node Binary on Ubuntu
  • GitHub Check: Leaks / Node v23
  • GitHub Check: E2E / Node Docker on Ubuntu
  • GitHub Check: Leaks / Node v22
  • GitHub Check: E2E / Node 23 on Ubuntu
  • GitHub Check: Leaks / Node v20
  • GitHub Check: E2E / Node 22 on Ubuntu
  • GitHub Check: E2E / Node 20 on Ubuntu
  • GitHub Check: E2E / Node 18 on Ubuntu
  • GitHub Check: Leaks / Node v18
🔇 Additional comments (30)
packages/logger-json/src/index.ts (1)

1-10: Consider adding a changeset and linking a Linear issue.

This new package adds a feature (JSON-based logging) and resides in the packages/ directory. According to the project’s guidelines, please ensure that:

  1. A changeset file is created (using yarn changeset) describing these new changes with at least a minor version bump.
  2. A corresponding Linear issue (prefixed with "GW-") is linked in the PR description to track this new functionality.
packages/runtime/src/index.ts (1)

3-3: LGTM!

The export of JSONLogger makes the new logging mechanism available to consumers.

packages/gateway/src/commands/handleLoggingOption.ts (1)

1-32: LGTM!

The changes properly handle the transition to JSONLogger while maintaining backward compatibility through the logLevel property check.

packages/runtime/src/plugins/useFetchDebug.ts (2)

16-23: LGTM! Improved logging structure.

The changes enhance logging context by:

  • Using child loggers with specific contexts
  • Including signal abort reason
  • Structured logging of request details

26-33: LGTM! Enhanced response logging.

The response logging improvements:

  • Use specific context for response logs
  • Structure headers as an object
  • Include duration metrics
packages/runtime/src/getReportingPlugin.ts (1)

21-21: LGTM! Structured logging update.

The change to use object notation for the logger child context aligns with the new JSON-based logging mechanism.

packages/runtime/src/plugins/useSubgraphExecuteDebug.ts (1)

16-19: LGTM! Well-structured logging implementation.

The changes introduce a clear hierarchy of loggers with specific contexts, improving the traceability of subgraph execution:

  • Root logger with execution IDs
  • Specific loggers for different execution phases
  • Structured data logging for queries and variables

Also applies to: 21-36, 40-42, 46-48

🧰 Tools
🪛 Biome (1.9.4)

[error] 18-18: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)

packages/runtime/src/plugins/useDelegationPlanDebug.ts (1)

26-27: LGTM! Clear logging hierarchy.

The changes establish a clear logging structure for different stages of delegation planning:

  • delegation-plan-start
  • delegation-plan-done
  • delegation-stage-execute
  • delegation-stage-execute-done

Also applies to: 56-57, 88-88, 111-114

packages/runtime/src/fetchers/graphos.ts (1)

30-30: LGTM! Enhanced logging context.

The changes improve the logging context by:

  • Adding source identification for GraphOS logs
  • Including attempt number and uplink information for retry tracking

Also applies to: 60-63

packages/transports/ws/src/index.ts (1)

79-84: LGTM! Enhanced logging with structured context.

The logger instantiation now includes relevant WebSocket connection details, improving observability.

packages/runtime/tests/graphos.test.ts (1)

134-134: LGTM! Updated logger in tests.

The test logger has been updated to use JSONLogger while maintaining the existing conditional instantiation logic.

packages/transports/ws/tests/ws.spec.ts (1)

1-1: LGTM! Updated logger in WebSocket transport tests.

The test file has been updated to use JSONLogger, maintaining consistency with the new logging mechanism.

Also applies to: 82-82

packages/transports/http-callback/src/index.ts (1)

110-113: LGTM! Enhanced subscription logging with structured context.

The subscription logger now includes relevant identifiers, improving the ability to track and debug HTTP callback subscriptions.

packages/gateway/src/cli.ts (3)

18-18: LGTM! Import and interface changes look good.

The import of JSONLogger and its use as the default logger in CLIContext interface aligns with the PR's objective of introducing structured JSON logging.

Also applies to: 180-181


350-350: LGTM! Logger instantiation looks good.

The default logger instantiation using JSONLogger is consistent with the interface definition.


365-365: LGTM! Logger child instantiation looks good.

The change to use object format { worker: cluster.worker.id } improves log structure and readability.

packages/fusion-runtime/tests/polling.test.ts (1)

2-2: LGTM! Test file changes look good.

The import and instantiation of JSONLogger in the test file maintains consistency with the new logging approach.

Also applies to: 338-338

packages/fusion-runtime/src/federation/supergraph.ts (1)

314-314: LGTM! Logger child instantiation looks good.

The changes to use object format for both requestId and subgraph improve log structure and consistency.

Also applies to: 318-320

packages/fusion-runtime/src/utils.ts (1)

105-105: LGTM! Logger child instantiations look good.

The changes consistently apply object format for logger child instantiation across multiple functions:

  • getTransportExecutor: { subgraph: subgraphName }
  • onSubgraphExecute: { requestId } and { subgraph: subgraphName }
  • wrapMergedTypeResolver: { requestId } and { subgraph: subschema.name }

This improves log structure and maintainability.

Also applies to: 198-198, 201-201, 551-551, 555-555

packages/runtime/src/createGatewayRuntime.ts (4)

8-8: LGTM! Logger initialization updated to use JSONLogger.

The changes consistently replace DefaultLogger with JSONLogger while preserving all configuration options.

Also applies to: 140-140, 143-146, 148-150


196-198: LGTM! Enhanced logging context structure.

Changed from string-based (logger.child('Hive')) to object-based (logger.child({ plugin: 'Hive Persisted Documents' })), improving log context clarity.


262-262: LGTM! Enhanced CDN logging context structure.

Changed from string-based (logger.child('Hive CDN')) to object-based (logger.child({ source: 'Hive CDN' })), improving log context clarity.


1024-1024: LGTM! GraphQL Yoga integration maintained.

The JSONLogger is correctly integrated with GraphQL Yoga's logging system.

.changeset/@graphql-hive_gateway-642-dependencies.md (1)

1-7: LGTM! Changeset correctly documents dependency changes.

The changeset appropriately marks this as a patch update and documents the addition of the JSON logger dependency.

.changeset/big-waves-prove.md (1)

1-11: LGTM! Changeset clearly documents the feature and dependency updates.

The changeset properly describes the introduction of the JSON-based logger and lists all related dependency updates.

packages/logger-json/package.json (2)

16-18: LGTM! Node.js version requirement is appropriate.

The Node.js >=18.0.0 requirement aligns with the LTS schedule.


20-32: LGTM! Proper dual CJS/ESM module configuration.

The exports field correctly configures both CommonJS and ES Module formats with their respective type definitions.

tsconfig.json (1)

61-62: LGTM!

The path mapping for @graphql-hive/logger-json is correctly added and follows the existing pattern.

packages/gateway/package.json (1)

61-61: LGTM!

The @graphql-hive/logger-json dependency is correctly added as a workspace dependency.

packages/plugins/opentelemetry/src/plugin.ts (1)

197-197: LGTM!

The logger initialization has been updated to use an object-based child logger, which aligns with the new JSON-based logging approach and improves log structure.

@theguild-bot
Copy link
Collaborator

theguild-bot commented Feb 12, 2025

🚀 Snapshot Release (Binary for macOS-X64)

The latest changes of this PR are available for download (based on the declared changesets).

Download

@theguild-bot
Copy link
Collaborator

theguild-bot commented Feb 12, 2025

🚀 Snapshot Release (Binary for Windows-X64)

The latest changes of this PR are available for download (based on the declared changesets).

Download

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🔭 Outside diff range comments (1)
packages/runtime/src/plugins/useFetchDebug.ts (1)

1-1: 💡 Verification agent

🧩 Analysis chain

Verify Linear issue and documentation requirements.

Based on the coding guidelines:

  1. Ensure a Linear issue (GW-*) is linked in the PR description.
  2. Documentation for this new logging feature should be added to the console project.

🌐 Web query:

Search for PRs in graphql-hive/console repository that add documentation for JSON-based logging

💡 Result:

Based on the search results provided, there are no direct mentions of pull requests (PRs) in the graphql-hive/console repository specifically related to adding documentation for JSON-based logging. However, here’s what can be inferred from the available information:

  1. Issue #2037 explicitly requests documentation for hive.json configuration options ([source 8][9]). This issue highlights the lack of examples or guidance on structuring hive.json, which could include logging configurations.

  2. The Hive CLI documentation ([source 3][10]) references hive.json as a configuration file for storing tokens and settings, but it does not detail JSON-based logging features.

For definitive information about PRs addressing JSON logging documentation, you would need to:

  • Check the GitHub repository directly for PRs linked to Issue #2037.
  • Review merged PRs with titles or descriptions mentioning "documentation," "hive.json," or "logging."

The provided search results do not include explicit references to such PRs.

Citations:


Missing Linear Issue Link and Documentation Updates

  • The PR does not appear to include a Linear issue reference (GW-*) in its description.
  • Documentation for the new JSON-based logging feature in the console project is missing.

Please add the necessary Linear issue link and update the console documentation accordingly.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 44d18a9 and 3a8acc2.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (13)
  • .changeset/@graphql-hive_gateway-runtime-642-dependencies.md (1 hunks)
  • .changeset/big-waves-prove.md (1 hunks)
  • packages/fusion-runtime/tests/polling.test.ts (3 hunks)
  • packages/gateway/src/bin.ts (2 hunks)
  • packages/gateway/src/cli.ts (4 hunks)
  • packages/gateway/src/commands/handleFork.ts (1 hunks)
  • packages/gateway/src/commands/handleLoggingOption.ts (2 hunks)
  • packages/gateway/src/commands/proxy.ts (1 hunks)
  • packages/runtime/package.json (1 hunks)
  • packages/runtime/src/createGatewayRuntime.ts (5 hunks)
  • packages/runtime/src/getDefaultLogger.ts (1 hunks)
  • packages/runtime/src/plugins/useFetchDebug.ts (1 hunks)
  • packages/runtime/src/plugins/useSubgraphExecuteDebug.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
`packages/**`: In this directory we keep all packages releva...

packages/**: In this directory we keep all packages relevant to the gateway.

In most cases, when changes are made to the source code / dependencies in these directories, we also require to have a corresponding changeset file, created using yarn changeset with a full description on the changes with an example.
Also, when new features are added the packages, the changeset file should mention minor semver change. When a bug is fixed, patch needs to be used. The major bump is used for a PR that has breaking changes.

  • packages/gateway/src/bin.ts
  • packages/runtime/package.json
  • packages/runtime/src/plugins/useFetchDebug.ts
  • packages/runtime/src/getDefaultLogger.ts
  • packages/gateway/src/commands/handleFork.ts
  • packages/fusion-runtime/tests/polling.test.ts
  • packages/runtime/src/plugins/useSubgraphExecuteDebug.ts
  • packages/gateway/src/commands/proxy.ts
  • packages/gateway/src/cli.ts
  • packages/gateway/src/commands/handleLoggingOption.ts
  • packages/runtime/src/createGatewayRuntime.ts
`packages/gateway/**`: The main runtime for the Hive gateway...

packages/gateway/**: The main runtime for the Hive gateway.
This package is CLI that runs the gateway and configures the internals of the gateway function.

  • packages/gateway/src/bin.ts
  • packages/gateway/src/commands/handleFork.ts
  • packages/gateway/src/commands/proxy.ts
  • packages/gateway/src/cli.ts
  • packages/gateway/src/commands/handleLoggingOption.ts
`**`: For all PRs, we would like to verify that a Linear iss...

**: For all PRs, we would like to verify that a Linear issue is linked. Check if there's a mention for "GW-*" in the PR description, and the us know when it's missing.

In most cases, when a PR adds a new functionality, we would like to make sure it has documentation in the corresponding console project.
Use the global knowledge feature to search for PRs in graphql-hive/console repository that adds documentation. Suggest to create a follow up issue to add the missing documentation.

  • packages/gateway/src/bin.ts
  • packages/runtime/package.json
  • packages/runtime/src/plugins/useFetchDebug.ts
  • packages/runtime/src/getDefaultLogger.ts
  • packages/gateway/src/commands/handleFork.ts
  • packages/fusion-runtime/tests/polling.test.ts
  • packages/runtime/src/plugins/useSubgraphExecuteDebug.ts
  • packages/gateway/src/commands/proxy.ts
  • packages/gateway/src/cli.ts
  • packages/gateway/src/commands/handleLoggingOption.ts
  • packages/runtime/src/createGatewayRuntime.ts
🪛 LanguageTool
.changeset/big-waves-prove.md

[uncategorized] ~13-~13: Possible missing preposition found.
Context: ...still to the console unless NODE_ENV is production. For JSON output, set the LOG_FORMAT ...

(AI_HYDRA_LEO_MISSING_IN)

⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Binary built on windows-latest
  • GitHub Check: E2E / Node Binary on Windows
  • GitHub Check: E2E / Node Binary on Ubuntu
🔇 Additional comments (19)
packages/gateway/src/bin.ts (1)

23-23: Consider passing a logger name for clarity.

When instantiating the logger, you can add a name property to help distinguish logs in multi-service environments.

Apply this diff:

-const log = getDefaultLogger();
+const log = getDefaultLogger({ name: 'gateway' });
packages/runtime/src/plugins/useFetchDebug.ts (1)

1-1: Add changeset file for package changes.

The PR introduces changes to multiple packages and adds a new logging feature. Per the coding guidelines, a changeset file is required.

Run the following command to create a changeset:

✅ Verification successful

🏁 Script executed:

#!/bin/bash
# Check if changeset exists for the PR
find .changeset -name "*.md" -type f -exec cat {} \;

Length of output: 7134


Changeset file is present and up-to-date.
The changeset file check confirms that multiple packages, including the new JSON-based logger feature, have corresponding changeset entries. No further changes are needed regarding changeset additions.

packages/gateway/src/commands/handleFork.ts (3)

13-13: LGTM! Good practice creating a dedicated logger per worker.

Creating a child logger with worker ID context helps in correlating logs across worker processes.


16-19: LGTM! Improved structured logging.

The debug log now uses structured format with explicit code and signal fields, making it easier to parse and analyze.


21-27: LGTM! Enhanced error logging with actionable message.

The error log now:

  1. Uses structured format with code and signal fields
  2. Includes a helpful recommendation for service stability
packages/runtime/src/plugins/useSubgraphExecuteDebug.ts (3)

16-22: LGTM! Well-structured logging metadata.

Good practice:

  1. Creating a base metadata object with subgraph execution ID
  2. Conditionally adding request ID for tracing

24-39: LGTM! Excellent structured logging for execution start.

Good practices:

  1. Creating a dedicated child logger for the start phase
  2. Using lazy evaluation with arrow function
  3. Conditionally including query and variables
  4. Using structured format for better parsing

43-64: LGTM! Well-organized execution logging.

Good practices:

  1. Separate loggers for next and end phases
  2. Consistent structured format
  3. Including duration metrics
packages/gateway/src/commands/proxy.ts (2)

68-68: LGTM! Good practice creating a dedicated logger for Hive CDN.

Creating a child logger with source context helps in filtering and analyzing CDN-specific logs.


72-74: LGTM! Improved error message.

The error message is now more concise and clearer.

🧰 Tools
🪛 Biome (1.9.4)

[error] 73-74: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

packages/gateway/src/cli.ts (2)

350-350: LGTM! Good switch to flexible logger factory.

Using getDefaultLogger() allows for dynamic logger selection based on environment.


365-365: LGTM! Improved worker context logging.

Using structured format for worker context improves log parsing and filtering capabilities.

packages/fusion-runtime/tests/polling.test.ts (1)

8-8: LGTM! The logger changes look good.

The replacement of DefaultLogger with getDefaultLogger() aligns with the new logging mechanism being introduced across the codebase. The logger is appropriately used for debugging in the test context.

Also applies to: 20-20, 338-338

packages/runtime/src/createGatewayRuntime.ts (2)

81-81: LGTM! Logger initialization changes look good.

The changes standardize the logger initialization using getDefaultLogger() with appropriate configuration based on the logging options. The implementation correctly handles all cases:

  • No logging config (null)
  • Boolean logging config (true/false)
  • Numeric logging level config

Also applies to: 140-150


196-198: Improved logging context with structured metadata.

The child loggers now use structured context objects instead of string prefixes, which enhances log filtering and analysis capabilities:

  • logger.child('Hive')logger.child({ plugin: 'Hive Persisted Documents' })
  • logger.child('Hive CDN')logger.child({ source: 'Hive CDN' })

Also applies to: 262-262, 620-620

.changeset/@graphql-hive_gateway-runtime-642-dependencies.md (1)

1-7: LGTM! Dependency addition is properly documented.

The addition of @graphql-hive/logger-json as a workspace dependency is correctly marked as a patch version update, which is appropriate since it enhances functionality without breaking changes.

.changeset/big-waves-prove.md (1)

1-14: LGTM! Clear documentation of the logger changes.

The changeset properly documents:

  • Package version updates
  • New logger functionality
  • Environment variable configuration
🧰 Tools
🪛 LanguageTool

[uncategorized] ~13-~13: Possible missing preposition found.
Context: ...still to the console unless NODE_ENV is production. For JSON output, set the LOG_FORMAT ...

(AI_HYDRA_LEO_MISSING_IN)

packages/runtime/package.json (2)

57-57: LGTM! The new logger dependency is correctly added.

The workspace-scoped dependency @graphql-hive/logger-json aligns with the PR's objective of implementing JSON-based logging.


1-100: Verify Linear issue reference and documentation.

Please ensure:

  1. A Linear issue (GW-*) is referenced in the PR description
  2. Documentation is added to the console project for the new JSON logging feature

Let's search for related documentation PRs:

Let's verify the changeset:

❌ Verification inconclusive

🏁 Script executed:

#!/bin/bash
# Search for documentation PRs in the console repository
gh search prs --repo graphql-hive/console --json title,url \
  --jq '.[] | select(.title | test("(?i)json.*log|log.*json"))' \
  --state all

Length of output: 3451


🏁 Script executed:

#!/bin/bash
# Check for changeset files related to logger changes
fd changeset$ .changeset -x cat {}

Length of output: 40


Action: Please manually verify the Linear issue reference and JSON logging documentation.

The automated scripts did not return any evidence that a Linear issue (GW-*) is cited in the PR description, nor did they show any recent documentation changes (or changeset file updates) for the new JSON logging feature in the console project. It appears we need to manually confirm:

  • A Linear issue (GW-*) is referenced in the PR description.
  • The console project includes updated documentation for the JSON logging feature.

@@ -60,6 +60,8 @@
"vitest": "^3.0.1"
},
"resolutions": {
"@graphql-mesh/types": "0.103.16-alpha-20250212125252-6167fff12fe4017202b35363ff2769aafe28f98a",
Copy link
Member

Choose a reason for hiding this comment

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

update this before merging?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes!

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🔭 Outside diff range comments (1)
packages/logger-json/src/index.ts (1)

1-186: 🧹 Nitpick (assertive)

Add tests and link a Linear issue.

  1. Consider writing unit tests for this logger to ensure correctness and guard against regressions.
  2. For changes under packages/**, confirm a changeset has been created.
  3. Verify that a Linear ticket (e.g., "GW-*") is linked in the PR description.
🧰 Tools
🪛 Biome (1.9.4)

[error] 159-159: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)


[error] 164-164: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)


[error] 126-126: Use Number.POSITIVE_INFINITY instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.POSITIVE_INFINITY instead.

(lint/style/useNumberNamespace)


[error] 129-129: Reassigning a function parameter is confusing.

The parameter is declared here:

Use a local variable instead.

(lint/style/noParameterAssign)


[error] 132-132: Reassigning a function parameter is confusing.

The parameter is declared here:

Use a local variable instead.

(lint/style/noParameterAssign)


[error] 148-148: This let declares a variable that is only assigned once.

'messageArg' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3a8acc2 and 2dc4c18.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (2)
  • packages/logger-json/package.json (1 hunks)
  • packages/logger-json/src/index.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`packages/**`: In this directory we keep all packages releva...

packages/**: In this directory we keep all packages relevant to the gateway.

In most cases, when changes are made to the source code / dependencies in these directories, we also require to have a corresponding changeset file, created using yarn changeset with a full description on the changes with an example.
Also, when new features are added the packages, the changeset file should mention minor semver change. When a bug is fixed, patch needs to be used. The major bump is used for a PR that has breaking changes.

  • packages/logger-json/package.json
  • packages/logger-json/src/index.ts
`**`: For all PRs, we would like to verify that a Linear iss...

**: For all PRs, we would like to verify that a Linear issue is linked. Check if there's a mention for "GW-*" in the PR description, and the us know when it's missing.

In most cases, when a PR adds a new functionality, we would like to make sure it has documentation in the corresponding console project.
Use the global knowledge feature to search for PRs in graphql-hive/console repository that adds documentation. Suggest to create a follow up issue to add the missing documentation.

  • packages/logger-json/package.json
  • packages/logger-json/src/index.ts
🪛 Biome (1.9.4)
packages/logger-json/src/index.ts

[error] 159-159: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)


[error] 164-164: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)


[error] 126-126: Use Number.POSITIVE_INFINITY instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.POSITIVE_INFINITY instead.

(lint/style/useNumberNamespace)


[error] 129-129: Reassigning a function parameter is confusing.

The parameter is declared here:

Use a local variable instead.

(lint/style/noParameterAssign)


[error] 132-132: Reassigning a function parameter is confusing.

The parameter is declared here:

Use a local variable instead.

(lint/style/noParameterAssign)


[error] 148-148: This let declares a variable that is only assigned once.

'messageArg' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

⏰ Context from checks skipped due to timeout of 90000ms (29)
  • GitHub Check: E2E / Bun Docker on Ubuntu
  • GitHub Check: E2E / Bun on Ubuntu
  • GitHub Check: E2E / Node Binary on Windows
  • GitHub Check: Unit / Bun
  • GitHub Check: E2E / Node Binary on Ubuntu
  • GitHub Check: E2E / Node Docker on Ubuntu
  • GitHub Check: Leaks / Node v23
  • GitHub Check: E2E / Node 23 on Ubuntu
  • GitHub Check: Unit / Node v23
  • GitHub Check: E2E / Node 22 on Ubuntu
  • GitHub Check: Leaks / Node v22
  • GitHub Check: Unit / Node v22
  • GitHub Check: Unit / Node v20
  • GitHub Check: Missing peer deps
  • GitHub Check: Benchmark / bun / 1000 items
  • GitHub Check: Snapshot / snapshot
  • GitHub Check: E2E / Node 20 on Ubuntu
  • GitHub Check: Benchmark / bun / 100 items
  • GitHub Check: Unit / Node v18
  • GitHub Check: Benchmark / bun / 10 items
  • GitHub Check: Benchmark / node / 1000 items
  • GitHub Check: Leaks / Node v20
  • GitHub Check: Build
  • GitHub Check: E2E / Node 18 on Ubuntu
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Bundle
  • GitHub Check: Benchmark / node / 100 items
  • GitHub Check: Benchmark / node / 10 items
  • GitHub Check: Leaks / Node v18
🔇 Additional comments (7)
packages/logger-json/src/index.ts (5)

126-126: Use Number.POSITIVE_INFINITY instead of Infinity.
This change aligns with ES2015 guidelines and improves consistency.

- .flat(Infinity)
+ .flat(Number.POSITIVE_INFINITY)
🧰 Tools
🪛 Biome (1.9.4)

[error] 126-126: Use Number.POSITIVE_INFINITY instead of the equivalent global.

ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.POSITIVE_INFINITY instead.

(lint/style/useNumberNamespace)


129-132: Avoid reassigning function parameters.
Reassigning messageArg can be confusing. Use a new variable to store intermediate values.

- if (typeof messageArg === 'function') {
-   messageArg = messageArg();
- }
- if (messageArg?.toJSON) {
-   messageArg = messageArg.toJSON();
- }
+ let currentArg = messageArg;
+ if (typeof currentArg === 'function') {
+   currentArg = currentArg();
+ }
+ if (currentArg?.toJSON) {
+   currentArg = currentArg.toJSON();
+ }
+ // Then continue processing using `currentArg` instead of reassigning messageArg.
🧰 Tools
🪛 Biome (1.9.4)

[error] 129-129: Reassigning a function parameter is confusing.

The parameter is declared here:

Use a local variable instead.

(lint/style/noParameterAssign)


[error] 132-132: Reassigning a function parameter is confusing.

The parameter is declared here:

Use a local variable instead.

(lint/style/noParameterAssign)


148-148: Use const instead of let for the iteration variable.
This iteration variable is never reassigned, so const is preferable.

- for (let messageArg of flattenedMessageArgs) {
+ for (const messageArg of flattenedMessageArgs) {
🧰 Tools
🪛 Biome (1.9.4)

[error] 148-148: This let declares a variable that is only assigned once.

'messageArg' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)


159-160: Use template literals instead of string concatenation.
This improves readability and consistency.

- ? finalMessage['msg'] + ', ' + messageArg
+ ? `${finalMessage['msg']}, ${messageArg}`
🧰 Tools
🪛 Biome (1.9.4)

[error] 159-159: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)


164-164: Use template literals instead of string concatenation.
This further enhances readability for error messages.

- ? finalMessage['msg'] + ', ' + messageArg.message
+ ? `${finalMessage['msg']}, ${messageArg.message}`
🧰 Tools
🪛 Biome (1.9.4)

[error] 164-164: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

packages/logger-json/package.json (2)

2-3: Initial package version should be 0.1.0.

Following semver conventions, the initial release should typically start at 0.1.0 rather than 0.0.0.


44-54: Verify dependency versions.

Please verify that you're using the latest compatible versions of dependencies, particularly for security and bug fixes:

✅ Verification successful

🏁 Script executed:

#!/bin/bash
# Check for latest versions of key dependencies
npm view @graphql-mesh/cross-helpers version
npm view @graphql-mesh/types version
npm view @graphql-mesh/utils version
npm view cross-inspect version
npm view tslib version

Length of output: 219


Dependency versions are up-to-date.

  • @graphql-mesh/cross-helpers: Installed version 0.4.10 matches the latest available.
  • @graphql-mesh/types: Installed version 0.103.16 matches the latest available.
  • @graphql-mesh/utils: Installed version 0.103.16 matches the latest available.
  • cross-inspect: Installed version 1.0.1 matches the latest available.
  • tslib: Installed version 2.8.1 matches the latest available.

No further changes are required.

Comment on lines +1 to +56
{
"name": "@graphql-hive/logger-json",
"version": "0.0.0",
"type": "module",
"repository": {
"type": "git",
"url": "git+https://github.com/graphql-hive/gateway.git",
"directory": "packages/logger-json"
},
"author": {
"email": "contact@the-guild.dev",
"name": "The Guild",
"url": "https://the-guild.dev"
},
"license": "MIT",
"engines": {
"node": ">=18.0.0"
},
"main": "./dist/index.js",
"exports": {
".": {
"require": {
"types": "./dist/index.d.cts",
"default": "./dist/index.cjs"
},
"import": {
"types": "./dist/index.d.ts",
"default": "./dist/index.js"
}
},
"./package.json": "./package.json"
},
"types": "./dist/index.d.ts",
"files": [
"dist"
],
"scripts": {
"build": "pkgroll --clean-dist",
"prepack": "yarn build"
},
"peerDependencies": {
"graphql": "^15.9.0 || ^16.9.0"
},
"dependencies": {
"@graphql-mesh/cross-helpers": "^0.4.10",
"@graphql-mesh/types": "^0.103.16",
"@graphql-mesh/utils": "^0.103.16",
"cross-inspect": "^1.0.1",
"tslib": "^2.8.1"
},
"devDependencies": {
"graphql": "^16.9.0",
"pkgroll": "2.8.2"
},
"sideEffects": false
}
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify Linear issue and documentation requirements.

According to the coding guidelines:

  1. Please ensure there's a Linear issue (GW-*) linked to this PR
  2. Since this introduces a new JSON logging feature, documentation should be added to the console project

Let me check for documentation PRs:


🏁 Script executed:

gh search prs --repo graphql-hive/console --json title,url --jq '.[] | select(.title | contains("logger") or contains("logging"))' 

Length of output: 389


Action Required: Address Missing Linear Issue and Documentation Update

  • It appears that the PR does not include a reference to a Linear issue (GW-*) as required.
  • Additionally, while the console project shows some recent updates related to logging, there is no explicit documentation update linking to the new JSON logging feature.

Please update the PR description and/or commit messages to include a Linear issue reference and ensure that the console documentation is updated accordingly.

Comment on lines +37 to +40
"scripts": {
"build": "pkgroll --clean-dist",
"prepack": "yarn build"
},
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Add test script to package.json.

Consider adding a test script to ensure the logger functionality is properly tested:

 "scripts": {
   "build": "pkgroll --clean-dist",
-  "prepack": "yarn build"
+  "prepack": "yarn build",
+  "test": "jest"
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"scripts": {
"build": "pkgroll --clean-dist",
"prepack": "yarn build"
},
"scripts": {
"build": "pkgroll --clean-dist",
"prepack": "yarn build",
"test": "jest"
},

Comment on lines +1 to +4
{
"name": "@graphql-hive/logger-json",
"version": "0.0.0",
"type": "module",
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Missing changeset file for the new package.

According to the coding guidelines, a changeset file is required for new packages. Please create one using yarn changeset with:

  • A description of the new JSON-based logger feature
  • An example of usage
  • A "minor" semver bump since this is a new feature

Let me check if a changeset file exists:


🏁 Script executed:

#!/bin/bash
# Look for changeset files
fd -e md . .changeset/

Length of output: 1716


Missing changeset file for @graphql-hive/logger-json package
No changeset file referencing @graphql-hive/logger-json was found in the repository's .changeset directory. Please add a new changeset file (using yarn changeset) that includes a description of the JSON-based logger feature, an example of usage, and specifies a "minor" semver bump.

@ardatan ardatan merged commit 30e41a6 into main Feb 12, 2025
1 check passed
@ardatan ardatan deleted the json-based-logger branch February 12, 2025 17:51
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