Skip to content

feat(rivetkit): gcp log level format for rivet compute#4542

Draft
abcxff wants to merge 1 commit intomainfrom
03-31-feat_rivetkit_gcp_log_level_format_for_rivet_compute
Draft

feat(rivetkit): gcp log level format for rivet compute#4542
abcxff wants to merge 1 commit intomainfrom
03-31-feat_rivetkit_gcp_log_level_format_for_rivet_compute

Conversation

@abcxff
Copy link
Copy Markdown
Contributor

@abcxff abcxff commented Mar 31, 2026

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@railway-app
Copy link
Copy Markdown

railway-app bot commented Mar 31, 2026

🚅 Deployed to the rivet-pr-4542 environment in rivet-frontend

Service Status Web Updated (UTC)
frontend-cloud 😴 Sleeping (View Logs) Web Apr 13, 2026 at 5:29 pm
kitchen-sink ❌ Build Failed (View Logs) Web Apr 9, 2026 at 5:58 pm
website 😴 Sleeping (View Logs) Web Apr 8, 2026 at 9:02 am
frontend-inspector 😴 Sleeping (View Logs) Web Apr 4, 2026 at 1:19 pm
mcp-hub ✅ Success (View Logs) Web Mar 31, 2026 at 9:16 pm
ladle ❌ Build Failed (View Logs) Web Mar 31, 2026 at 9:16 pm

Copy link
Copy Markdown
Contributor Author

abcxff commented Mar 31, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Mar 31, 2026

More templates

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@4542

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@4542

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@4542

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@4542

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@4542

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@4542

@rivetkit/sqlite-vfs

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sqlite-vfs@4542

@rivetkit/traces

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/traces@4542

@rivetkit/workflow-engine

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/workflow-engine@4542

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@4542

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@4542

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@4542

commit: 8d0f41e

@claude
Copy link
Copy Markdown

claude bot commented Mar 31, 2026

PR Review: feat(rivetkit): gcp log level format for rivet compute

This PR adds GCP Cloud Logging severity mapping for pino when running on Rivet Compute (_RIVET_COMPUTE_ENABLED=1). The approach is clean and the scope is well-contained. A few items to address:


Issues

Silent formatter override in createGcpLoggingPinoConfig

In gcp-log-helper.ts, if the caller passes formatters.level inside pinoLoggerOptionsMixin, it gets silently overwritten:

```ts
formatters: {
...formattersMixin?.formatters,
level: pinoLevelToGcpSeverity, // always overwrites caller-provided level formatter
},
```

This is probably intentional since GCP severity mapping is the whole point of this function, but a comment would prevent future confusion. Consider:

```ts
// Always override level formatter; GCP severity mapping is the purpose of this config.
level: pinoLevelToGcpSeverity,
```

typeof window === 'undefined' guard is fragile

The check in log.ts uses typeof window === 'undefined' to detect server-side execution. This is a common pattern but can produce unexpected results in some edge runtimes (e.g., Cloudflare Workers, Deno) where window may or may not exist. Since this is gated behind _RIVET_COMPUTE_ENABLED (a Rivet-internal env var that only exists in Rivet's own compute environment), this is probably fine in practice. Worth a brief comment explaining why the window check is there alongside the compute check.


Minor Notes

  • No tests added. pinoLevelToGcpSeverity is a pure function with a fixed lookup table, making it trivial to unit-test. At minimum a short test covering the known mappings and the unknown-level fallback would increase confidence. The behavior change in configureDefaultLogger could be covered by checking that baseLogger uses the GCP formatter when the env var is set.

  • GCP severity mapping looks correct. traceDEBUG, fatalCRITICAL, unknown → INFO are all the right choices per GCP's severity ladder.

  • PR description is empty. The default template was left entirely blank. A one-line summary of the motivation (e.g., "structured GCP log entries require the severity field instead of the pino level field") would help reviewers and future git log readers.


Summary

The implementation is straightforward and correct. The main asks before merging:

  1. Add a comment on the silent formatter override in createGcpLoggingPinoConfig.
  2. Fill in the PR description.
  3. (Optional but recommended) Add unit tests for pinoLevelToGcpSeverity.

@abcxff abcxff force-pushed the 03-31-feat_rivetkit_gcp_log_level_format_for_rivet_compute branch from 8d0f41e to 8c06b67 Compare April 9, 2026 17:58
@abcxff abcxff mentioned this pull request Apr 9, 2026
11 tasks
@abcxff abcxff marked this pull request as ready for review April 9, 2026 17:59
@abcxff abcxff marked this pull request as draft April 9, 2026 17:59
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.

1 participant