Logs in logger#282
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces W3C traceparent support for structured logging, enabling automatic trace and span ID correlation by extracting data from the execution Zone. Key changes include a new traceparent parser, updates to the structured logger and shelf middleware, and the addition of comprehensive E2E and unit tests. Review feedback highlights critical issues with unreleased dependency versions in the pubspec file and suggests refining the E2E test server logic to avoid unsupported trace header formats.
| googleapis_auth: ^2.0.0 | ||
| http: ^1.3.0 | ||
| test: ^1.31.0 | ||
| test_utils: any |
There was a problem hiding this comment.
The version constraints for googleapis_auth and http appear to be for unreleased versions and will likely cause dart pub get to fail. Please update these to valid and stable version constraints.
googleapis_auth: ^1.6.0
http: ^1.2.1
test: ^1.31.0References
- A wide dependency version range is acceptable if compatibility across the entire range has been verified.
| final traceparent = | ||
| request.headers.value('traceparent') ?? | ||
| request.headers.value('x-cloud-trace-context'); |
There was a problem hiding this comment.
This logic falls back to using the x-cloud-trace-context header if traceparent is not present. However, the CloudLogger.structuredLogger() used in this test server is configured to parse only the W3C traceparent format, not the x-cloud-trace-context format. If an x-cloud-trace-context header is received, its value will be passed to the logger, which will fail to parse it, and no trace correlation will occur. To avoid confusion, it would be clearer to only handle the traceparent header.
| final traceparent = | |
| request.headers.value('traceparent') ?? | |
| request.headers.value('x-cloud-trace-context'); | |
| final traceparent = | |
| request.headers.value('traceparent'); |
No description provided.