Add API spec for logger client integration into cdp-client#25
Add API spec for logger client integration into cdp-client#25stefanrammo wants to merge 1 commit intomainfrom
Conversation
Specifies two changes to integrate the CDP Logger Client into the main JavaScript CDP Client package: 1. Bundle logger client as studio.logger — move the cdplogger-client code into the cdp-client package so users only need one require() 2. Add client.logger(path) method — auto-discovers the logger's WebSocket endpoint by reading ServerPort and ServerIP from the node's children, then creates and caches a logger client connection Uses ServerPort tree navigation for discovery (works with all CDP versions). Service-based discovery and proxy transport via ServicesNotification are documented as future work for CDP 5.1+. Includes 5 use case examples, failure/timeout semantics, caching and lifecycle behavior, and migration notes.
|
"Uses ServerPort tree navigation for discovery (works with all CDP versions). Service-based discovery and proxy transport via ServicesNotification are documented as future work for CDP 5.1+." The reason CDP Studio moved to service-based discovery is that this tree scanning to find loggers and the ServerPort property caused performance issues for larger projects. Also, this here is a new feature, so I think it is fine if it only works with CDP 5.1+. People can use the existing logger client for old versions if needed. All in all, in my opinion cdp-client logger support should only use service-based discovery without any tree scanning. |
| }); | ||
| ``` | ||
|
|
||
| This requires knowing the `ServerPort` child convention, extracting the hostname, and managing two independent client lifecycles. The logger client (`cdplogger-client`) is a separate npm package with its own protobuf schema and WebSocket connection, making the setup even more cumbersome. |
There was a problem hiding this comment.
Also, one must know "App.CDPLogger" is the path and this must be updated when CDP project is restructured.
|
|
||
| Instead of opening a direct WebSocket to the logger port, the logger connection could tunnel through the existing StudioAPI WebSocket — the same way sibling app connections work via `connectViaProxy()`. This avoids firewall and port-opening issues when the logger port is not directly reachable from the client. | ||
|
|
||
| Service-based discovery and proxy transport are the preferred approach for CDP 5.1+ and should be added in a follow-up. The `ServerPort` approach is implemented first for backward compatibility with all CDP versions. |
There was a problem hiding this comment.
I would enable proxy transport by default because browser security settings often reject connections to other ports, especially when using https. And I think supporting only CDP 5.1 is fine
There was a problem hiding this comment.
And if it makes implementation easier, I don't think we need to support direct connection. The proxy performance is quite good.
There was a problem hiding this comment.
And one more reason the direct connection is not so useful - the logger server api does not support authentication, so it is recommended to block that port with firewall. This proxy thing allows to serve logged data behind studioapi authentication.
Ok, I now read the rest of the spec and there is no tree scanning. User must hard-code the path, e.g. "App.CDPLogger". Still, I don't think this is useful enough at the moment. If one hard-codes the path to logger, they might as well hard-code the ServerPort. Better to not think about older CDP versions and directly move to implementing service-based discovery. |
|
Something to consider for the future (I think not needed for the first step) is to make it easier to ask historic data for a real-time value. Kind of like CDP Studio Analyze mode auto-discovers if added node has historic data available and lists the history sources when right-clicking on the legend. |
|
|
||
| ```javascript | ||
| const loggers = await client.loggers(); // returns all discovered logger services | ||
| const logger = loggers[0]; // or find by name/metadata |
There was a problem hiding this comment.
"or find by name/metadata" - yes, as the metadata includes the logger path in the metadata name field, should allow to refer to logger using that as well .
|
Spec reviews should also have 2 reviewers |
2 participants
Specifies two changes to integrate the CDP Logger Client into the main JavaScript CDP Client package:
Bundle logger client as studio.logger — move the cdplogger-client code into the cdp-client package so users only need one require()
Add client.logger(path) method — auto-discovers the logger's WebSocket endpoint by reading ServerPort and ServerIP from the node's children, then creates and caches a logger client connection
Uses ServerPort tree navigation for discovery (works with all CDP versions). Service-based discovery and proxy transport via ServicesNotification are documented as future work for CDP 5.1+.
Includes 5 use case examples, failure/timeout semantics, caching and lifecycle behavior, and migration notes.