-
Notifications
You must be signed in to change notification settings - Fork 361
Centralize log storage #1315
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
Merged
Merged
Centralize log storage #1315
Changes from all commits
Commits
Show all changes
59 commits
Select commit
Hold shift + click to select a range
ccaa86c
Add context method
bgrgicak 0e8b73a
Collect context
bgrgicak 10f5e25
Collect worker metrics as context data
bgrgicak c127c95
Add context and blueprint to logger API request
bgrgicak 8d89ca4
Rename addFatalErrorListener
bgrgicak be6446c
Add context and blueprint sections to report
bgrgicak 1f07595
Count tabs instead of all clients
bgrgicak 653735a
Remove debug code
bgrgicak 40e3ab5
Revert url to production
bgrgicak 007ac3f
Merge branch 'trunk' into add/1152-logger-context
bgrgicak d0a54b8
Refactor logger
bgrgicak 270c7f8
Add log methods
bgrgicak 26ce2cf
Add arg support
bgrgicak 311dc1e
Move reference
bgrgicak 910539b
Add tests
bgrgicak c37a47e
Address feedback
bgrgicak 868752f
Merge branch 'add/1152-logger-context' into add/log-methods
bgrgicak a7ea3f2
Parent PR feedback
bgrgicak 4ac444e
Add linter rules
bgrgicak 2aeb4c2
Add comment to addServiceWorkerMessageListener
bgrgicak 23cd042
Remove linter rules
bgrgicak 306d3e2
Merge branch 'add/1152-logger-context' into add/log-methods
bgrgicak 7ea4013
Merge branch 'trunk' into add/1152-logger-context
bgrgicak 6786e9a
Update packages/php-wasm/logger/src/lib/logger.ts
bgrgicak 44c688b
Update packages/php-wasm/logger/src/lib/logger.ts
bgrgicak 0d86ce1
Update packages/php-wasm/logger/src/lib/logger.ts
bgrgicak e24aafd
Remove worker claim
bgrgicak 6e0b8ee
Merge branch 'add/1152-logger-context' into add/log-methods
bgrgicak 4c01ceb
Add comment about using includeUncontrolled
bgrgicak 551a4ae
Merge branch 'add/1152-logger-context' into add/log-methods
bgrgicak 49f1e03
Merge branch 'trunk' into add/log-methods
bgrgicak 1d6c7d3
Post metrics to controlled clients only
bgrgicak a5c9ec1
Add log handlers
bgrgicak 5453638
Remove unrelated changes
bgrgicak b002aed
Log to memory
bgrgicak 0e8a3da
Separate collectors
bgrgicak c9f9028
Move formatLogEntry to main logger file
bgrgicak fca0706
Replace context with logs
bgrgicak 89b58bc
Add error to getLogs when logToMemory isn't registered
bgrgicak ed8dbd9
Fix test
bgrgicak 417c7fb
Disable console logs in tests
bgrgicak 8a09663
Merge branch 'trunk' into add/log-methods
bgrgicak 15589c0
Add linter rules
bgrgicak a2e9869
Replace console with logger TODO test and fix issues
bgrgicak de6899d
Remove logger from vite fie
bgrgicak f0ac462
Remove circular dependencies
bgrgicak 32fa3ef
Fix circular dependencies
bgrgicak 857874b
Fix linter config for tests
bgrgicak 541c6ce
Show logs as lines
bgrgicak 4384405
Add PHPRequestErrorEvent type
bgrgicak 9e77966
Apply suggestions from code review
bgrgicak 6943cd8
Update packages/php-wasm/logger/src/lib/collectors/collect-php-logs.ts
bgrgicak 6923ec3
Rename logWindowError
bgrgicak 684b925
Merge branch 'trunk' into add/log-methods
bgrgicak afb6849
Fix type
bgrgicak 8006d32
Merge branch 'add/log-methods' into update/console-to-logger
bgrgicak d68816a
Fix build
bgrgicak 1cca09a
Merge branch 'trunk' into update/console-to-logger
bgrgicak 177583d
Merge branch 'trunk' into update/console-to-logger
bgrgicak File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
5 changes: 1 addition & 4 deletions
5
packages/php-wasm/logger/src/lib/collectors/collect-php-logs.ts
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,29 +1,11 @@ | ||
| import { NodePHP } from '@php-wasm/node'; | ||
| import { LatestSupportedPHPVersion } from '@php-wasm/universal'; | ||
| import { Logger, addCrashListener } from '../lib/logger'; | ||
| import { collectPhpLogs } from '../lib/log-collector'; | ||
| import { Logger } from '../lib/logger'; | ||
| import { clearMemoryLogs, logToMemory } from '../lib/log-handlers'; | ||
|
|
||
| describe('Logger', () => { | ||
| let php: NodePHP; | ||
| const logger = new Logger([logToMemory]); | ||
| beforeEach(async () => { | ||
| php = await NodePHP.load(LatestSupportedPHPVersion); | ||
| clearMemoryLogs(); | ||
| }); | ||
| it('Event listener should work', () => { | ||
| const listener = vi.fn(); | ||
| collectPhpLogs(logger, php); | ||
| addCrashListener(logger, listener); | ||
| php.dispatchEvent({ | ||
| type: 'request.error', | ||
| error: new Error('test'), | ||
| }); | ||
| expect(listener).toBeCalledTimes(1); | ||
|
|
||
| const logs = logger.getLogs(); | ||
| expect(logs.length).toBe(1); | ||
| }); | ||
|
Comment on lines
-14
to
-26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test would require a lot of mocking and it's not that useful, so I removed it. |
||
|
|
||
| it('Log message should be added', () => { | ||
| logger.warn('test'); | ||
|
|
||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| export interface UniversalPHP { | ||
| /** | ||
| * Read the content of a file as text. | ||
| * | ||
| * @param path The path to the file | ||
| * @returns string The content of the file | ||
| */ | ||
| readFileAsText(path: string): Promise<string>; | ||
| /** | ||
| * Check if a file exists. | ||
| * | ||
| * @param path The path to the file | ||
| * @returns boolean Whether the file exists | ||
| */ | ||
| fileExists(path: string): Promise<boolean>; | ||
|
|
||
| addEventListener( | ||
| event: string, | ||
| listener: (event: PHPRequestEndEvent | PHPRequestErrorEvent) => void | ||
| ): void; | ||
| } | ||
|
|
||
| export interface PHPRequestErrorEvent { | ||
| type: 'request.error'; | ||
| error: Error; | ||
| source?: 'request' | 'php-wasm'; | ||
| } |
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
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
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
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
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
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
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
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These imports were causing circular dependency issues, so I added type declarations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not great, not terrible 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can always switch to
any😅