Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
to make it harder to loose in on the way
peter-leonov-ch
force-pushed
the
structured_logs
branch
from
acd74c1 to
97d53c2
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates logging to be more “structured” by moving variable context into args (e.g., IDs and operation metadata) while keeping message relatively stable, and adjusts configuration/tests to match the updated logging setup.
Changes:
- Enriches
NodeBaseConnectionlogs with structuredargsfields such asconnection_id,operation,request_id, andsocket_id. - Refactors log ID variables to snake_case (
request_id,socket_id) and removes some high-cardinality identifiers from log messages. - Updates
LogWriterconstruction and related tests/config wiring to pass an explicitClickHouseLogLevel.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/client-node/src/connection/node_base_connection.ts | Adds more structured context to HTTP/socket lifecycle logs and reduces message cardinality. |
| packages/client-node/tests/utils/http_stubs.ts | Updates test stubs to construct LogWriter with an explicit log level and set related params. |
| packages/client-node/tests/unit/node_default_logger.test.ts | Updates tests to pass an explicit ClickHouseLogLevel to LogWriter. |
| packages/client-node/tests/unit/node_config.test.ts | Updates config-related unit test to pass explicit ClickHouseLogLevel to LogWriter. |
| packages/client-node/tests/unit/node_client.test.ts | Updates client unit test connection params to pass explicit ClickHouseLogLevel to LogWriter. |
| packages/client-node/tests/integration/node_exec.test.ts | Updates integration test logger setup to pass explicit ClickHouseLogLevel to LogWriter. |
| packages/client-common/src/logger.ts | Changes LogWriter constructor to require a log level argument. |
| packages/client-common/src/config.ts | Normalizes default log_level computation and passes it to both log_level and LogWriter. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/client-node/__tests__/unit/node_default_logger.test.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Before moving forward with OTEL, let's use as much from the current logging subsystem as possible.
argsas possibleargskeys to ease sorting raw logsmessageallowing the end user to decide on the final format