refactor: [PRODUCT-465] use logs2 instead of logs REST endpoint#166
Merged
Conversation
Contributor
There was a problem hiding this comment.
Greptile Summary
This PR refactors the VM logs functionality to use a new /v0/vms/logs2 endpoint, representing a significant improvement in robustness and type safety. The changes include:
- Migration from timestamp-based to sequence-number-based log following
- Implementation of proper UTF-8 decoding with incomplete line handling
- Integration with strongly-typed API responses using generated types
- Required instance ID parameter for better UX
- RFC3339 timestamp validation
This change aligns with modern TypeScript practices by leveraging generated types from schema.ts, which provides better compile-time checking and API contract adherence. The new chunk processing logic with TextDecoder is particularly noteworthy as it properly handles UTF-8 encoded binary data and incomplete log lines across chunks, fixing potential character encoding issues that could exist in the previous implementation.
Confidence score: 4/5
- This PR is safe to merge with proper testing of log retrieval scenarios
- High score due to improved type safety, better error handling, and proper text encoding, but held back one point due to the complexity of log processing logic that could benefit from additional test coverage
- Files needing attention:
- src/lib/vm.ts: Focus on testing the new chunk processing logic and sequence-number-based following mechanism
1 file reviewed, 2 comments
Edit PR Review Bot Settings | Greptile
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reimplement
vm logsusingv0/logs2instead ofv0/logs.