agents: add option to dump grpc keylog file #406
Closed
Conversation
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
Only grpc insecure connections were being used in the tests. Also, these new tests allow to cover for the untested `NSOLID_GRPC_CERTS` env variable.
WalkthroughAdds TLS keylog support to the gRPC agent: reads NSOLID_GRPC_KEYLOG, prepares a PID-based keylog file, forwards the path into credentials/channel/stub creation, updates GrpcClient to use experimental TLS APIs for keylogging, increases keepalive, and adds TLS-capable test server and tests. Changes
Sequence DiagramsequenceDiagram
autonumber
participant Env as Environment (env vars)
participant Agent as GrpcAgent
participant Client as GrpcClient
participant Creds as ChannelCredentials
participant Stub as NSolidService::Stub
Note over Env,Agent: Startup / Agent construction
Env-->>Agent: Read NSOLID_GRPC_KEYLOG
alt keylog enabled
Agent->>Agent: Build PID-based keylog path & unlink any existing file
end
Agent->>Client: MakeNSolidServiceStub(options, tls_keylog_file)
Client->>Client: MakeChannel(options, tls_keylog_file)
Client->>Creds: MakeCredentials(options, tls_keylog_file)
alt TLS keylog requested
Creds->>Creds: Create TLS credentials via experimental API\n(configure keylog file)
else no keylog
Creds->>Creds: Create standard SSL or insecure credentials
end
Creds-->>Client: ChannelCredentials
Client->>Client: Create grpc::Channel (keepalive 30s)
Client-->>Stub: Instantiate NSolidService::Stub
Note over Stub,Agent: Agent uses stub for RPCs / exporters
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @agents/grpc/src/grpc_client.cc:
- Around line 13-16: The code imports and uses experimental gRPC TLS APIs
(TlsCredentials, TlsChannelCredentialsOptions, IdentityKeyCertPair,
StaticDataCertificateProvider) in MakeCredentials without acknowledging
instability; replace these with stable alternatives (e.g., use
grpc::SslCredentials and grpc::SslCredentialsOptions in MakeCredentials) or, if
experimental APIs are required, add a clear comment in MakeCredentials
explaining why the experimental APIs are necessary, pin the gRPC dependency to a
specific tested version, and document the compatibility risk; update any related
symbols (TlsCredentials, TlsChannelCredentialsOptions, IdentityKeyCertPair,
StaticDataCertificateProvider) references to the chosen stable APIs or add the
justification and version pin near their use.
🧹 Nitpick comments (4)
agents/grpc/src/grpc_agent.cc (1)
465-472: Consider adding error handling and improving the file path configuration.The current implementation has a few areas that could be improved:
Hardcoded relative path: The keylog file is written to
./nsolid-tls-keylog-<pid>.log(current working directory). This could fail if the CWD is not writable or doesn't exist. Consider using an absolute path or checking write permissions.Silent unlink failures: The
uv_fs_unlinkcall doesn't check for errors. While a missing file is fine, other errors (e.g., permission denied) are silently ignored.Environment variable semantics: The check
keylog.value() != "0"means any non-empty value except "0" enables the feature. Document this behavior or consider stricter validation (e.g., "1", "true", "yes").💡 Suggested improvements
auto keylog = per_process::system_environment->Get(kNSOLID_GRPC_KEYLOG); if (keylog.has_value() && !keylog.value().empty() && keylog.value() != "0") { tls_keylog_file_ = "./nsolid-tls-keylog-" + std::to_string(uv_os_getpid()) + ".log"; uv_fs_t req; - uv_fs_unlink(nullptr, &req, tls_keylog_file_.c_str(), nullptr); + int r = uv_fs_unlink(nullptr, &req, tls_keylog_file_.c_str(), nullptr); + if (r != 0 && r != UV_ENOENT) { + Debug("Warning: Failed to unlink existing keylog file: %s. Error: %d\n", + tls_keylog_file_.c_str(), r); + } uv_fs_req_cleanup(&req); }Consider also documenting the environment variable behavior in user-facing documentation.
agents/grpc/src/grpc_client.h (3)
59-64: Update documentation comment.The documentation comment should mention the new
tls_keylog_fileparameter and its purpose.🔎 Suggested documentation update
/** - * Create gRPC channel. + * Create gRPC channel. + * @param options Client options including endpoint and SSL credentials. + * @param tls_keylog_file Optional path for TLS session key log file (for debugging). */ static std::shared_ptr<::grpc::Channel> MakeChannel(const OtlpGrpcClientOptions& options, const std::string& tls_keylog_file = "");
66-71: Consider enhancing the documentation comment.The documentation could briefly describe the credential selection logic (insecure vs. SslCredentials vs. TlsCredentials with keylog).
🔎 Suggested documentation enhancement
/** - * Create gRPC channel credentials. + * Create gRPC channel credentials. + * Returns InsecureChannelCredentials, SslCredentials, or TlsCredentials + * based on options and tls_keylog_file parameter. */ static std::shared_ptr<::grpc::ChannelCredentials> MakeCredentials(const OtlpGrpcClientOptions& options, const std::string& tls_keylog_file);
79-84: Update documentation comment.The documentation comment should mention the new
tls_keylog_fileparameter.🔎 Suggested documentation update
/** * Create N|Solid service stub to communicate with the N|Solid Console. + * @param options Client options including endpoint and SSL credentials. + * @param tls_keylog_file Path for TLS session key log file (for debugging). */ static std::unique_ptr<grpcagent::NSolidService::StubInterface> MakeNSolidServiceStub(const OtlpGrpcClientOptions& options, const std::string& tls_keylog_file);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
agents/grpc/src/grpc_agent.ccagents/grpc/src/grpc_agent.hagents/grpc/src/grpc_client.ccagents/grpc/src/grpc_client.hdeps/opentelemetry-cpp/otlp-http-exporter.gyptest/agents/test-grpc-basic.mjstest/agents/test-grpc-keylog-file.mjstest/common/nsolid-grpc-agent/index.jstest/common/nsolid-grpc-agent/server.mjs
🧰 Additional context used
🧠 Learnings (41)
📓 Common learnings
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{tls_credentials.h,tls_credentials.cc} : Implement credentials for TLS transport security in `tls_credentials.h` / `tls_credentials.cc`
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:05.695Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/tls_credentials.{h,cc} : Use TLS (Transport Layer Security) credentials to configure TLS security for the Chttp2 transport
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/tsi/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:15.845Z
Learning: Create gRPC-specific implementations of the `tsi_transport_security` interface in dedicated files such as `transport_security_grpc.h` and `transport_security_grpc.cc`
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{local_credentials.h,local_credentials.cc} : Implement credentials for local transport security (e.g., UDS) in `local_credentials.h` / `local_credentials.cc`
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{insecure_credentials.h,insecure_credentials.cc} : Implement credentials for insecure (no security) transport in `insecure_credentials.h` / `insecure_credentials.cc`
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/security/authorization/GEMINI.md:0-0
Timestamp: 2025-12-03T14:33:17.207Z
Learning: Applies to deps/grpc/src/core/lib/security/authorization/**/audit_logging.{h,cc} : Implement audit logging interfaces to record information about authorization decisions for security auditing and debugging purposes
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/tsi/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:16.494Z
Learning: TSI provides a unified interface for performing transport-level security handshakes and protecting data, allowing gRPC to support different security mechanisms without changing core transport code
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{alts_credentials.h,alts_credentials.cc} : Implement credentials for ALTS transport security in `alts_credentials.h` / `alts_credentials.cc`
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/tsi/ssl/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:21.897Z
Learning: Use the Transport Security Interface (TSI) with OpenSSL for securing gRPC connections with TLS
📚 Learning: 2025-12-03T14:29:08.569Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{tls_credentials.h,tls_credentials.cc} : Implement credentials for TLS transport security in `tls_credentials.h` / `tls_credentials.cc`
Applied to files:
agents/grpc/src/grpc_agent.htest/common/nsolid-grpc-agent/server.mjsagents/grpc/src/grpc_agent.ccagents/grpc/src/grpc_client.cctest/agents/test-grpc-basic.mjsdeps/opentelemetry-cpp/otlp-http-exporter.gypagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:35:15.845Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/tsi/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:15.845Z
Learning: Create gRPC-specific implementations of the `tsi_transport_security` interface in dedicated files such as `transport_security_grpc.h` and `transport_security_grpc.cc`
Applied to files:
agents/grpc/src/grpc_agent.hagents/grpc/src/grpc_agent.ccagents/grpc/src/grpc_client.cctest/agents/test-grpc-basic.mjsagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:29:05.695Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:05.695Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/tls_credentials.{h,cc} : Use TLS (Transport Layer Security) credentials to configure TLS security for the Chttp2 transport
Applied to files:
agents/grpc/src/grpc_agent.htest/common/nsolid-grpc-agent/server.mjsagents/grpc/src/grpc_agent.ccagents/grpc/src/grpc_client.cctest/agents/test-grpc-basic.mjsagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:29:08.569Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{alts_credentials.h,alts_credentials.cc} : Implement credentials for ALTS transport security in `alts_credentials.h` / `alts_credentials.cc`
Applied to files:
agents/grpc/src/grpc_agent.htest/common/nsolid-grpc-agent/server.mjsagents/grpc/src/grpc_agent.ccagents/grpc/src/grpc_client.ccdeps/opentelemetry-cpp/otlp-http-exporter.gypagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:29:08.569Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{insecure_credentials.h,insecure_credentials.cc} : Implement credentials for insecure (no security) transport in `insecure_credentials.h` / `insecure_credentials.cc`
Applied to files:
test/common/nsolid-grpc-agent/server.mjsagents/grpc/src/grpc_client.cctest/agents/test-grpc-basic.mjsagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:29:08.569Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{local_credentials.h,local_credentials.cc} : Implement credentials for local transport security (e.g., UDS) in `local_credentials.h` / `local_credentials.cc`
Applied to files:
test/common/nsolid-grpc-agent/server.mjsagents/grpc/src/grpc_agent.ccagents/grpc/src/grpc_client.ccagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:33:17.207Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/security/authorization/GEMINI.md:0-0
Timestamp: 2025-12-03T14:33:17.207Z
Learning: Applies to deps/grpc/src/core/lib/security/authorization/**/audit_logging.{h,cc} : Implement audit logging interfaces to record information about authorization decisions for security auditing and debugging purposes
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:33:20.681Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/security/authorization/GEMINI.md:0-0
Timestamp: 2025-12-03T14:33:20.681Z
Learning: Applies to deps/grpc/src/core/lib/security/authorization/**/audit_logging.{h,cc} : Define audit logging interfaces to record information about authorization decisions for security auditing and debugging purposes
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:30:21.671Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/ext/filters/logging/GEMINI.md:0-0
Timestamp: 2025-12-03T14:30:21.671Z
Learning: Applies to deps/grpc/src/core/ext/filters/logging/**/{logging_filter.h,logging_filter.cc} : Implement gRPC logging filter to log call data including method name, peer address, and call status
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:30:16.982Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/ext/filters/logging/GEMINI.md:0-0
Timestamp: 2025-12-03T14:30:16.982Z
Learning: Applies to deps/grpc/src/core/ext/filters/logging/**/logging_filter.{h,cc} : Implement the logging filter in `logging_filter.h` and `logging_filter.cc` to log gRPC call data including method name, peer address, and call status
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:29:05.695Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:05.695Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/alts_credentials.{h,cc} : Use ALTS (Application Layer Transport Security) credentials to configure ALTS security for the transport
Applied to files:
agents/grpc/src/grpc_agent.ccagents/grpc/src/grpc_client.ccagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:34:40.509Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/server/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:40.509Z
Learning: Applies to deps/grpc/src/core/server/**/{server.h,server.cc} : Define the Server class in server.h / server.cc
Applied to files:
test/common/nsolid-grpc-agent/index.js
📚 Learning: 2025-12-03T14:34:37.445Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/server/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:37.445Z
Learning: Applies to deps/grpc/src/core/server/**/server.{h,cc} : The `Server` class in `server.h`/`server.cc` is the main class for the gRPC server and serves as the primary entry point for all server-side gRPC applications
Applied to files:
test/common/nsolid-grpc-agent/index.js
📚 Learning: 2025-12-03T14:34:35.931Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/server/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:35.931Z
Learning: The gRPC server is responsible for listening for incoming connections, handling requests, and sending responses via the `Server` class
Applied to files:
test/common/nsolid-grpc-agent/index.js
📚 Learning: 2025-12-03T14:34:35.931Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/server/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:35.931Z
Learning: The `Server` class (defined in `server.h` / `server.cc`) is the main entry point for all server-side gRPC applications
Applied to files:
test/common/nsolid-grpc-agent/index.js
📚 Learning: 2025-12-03T14:34:40.509Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/server/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:40.509Z
Learning: The gRPC Server class is the main entry point for all server-side gRPC applications
Applied to files:
test/common/nsolid-grpc-agent/index.js
📚 Learning: 2025-12-03T14:29:05.695Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:05.695Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/*_credentials.{h,cc} : Implement transport-specific credentials in dedicated header/implementation file pairs (e.g., `alts_credentials.h` / `alts_credentials.cc`, `tls_credentials.h` / `tls_credentials.cc`)
Applied to files:
agents/grpc/src/grpc_client.ccagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:28:57.122Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/call/GEMINI.md:0-0
Timestamp: 2025-12-03T14:28:57.122Z
Learning: Applies to deps/grpc/src/core/credentials/call/**/*_credentials.{h,cc} : Implement call credentials for adding per-call security information to gRPC calls, such as OAuth 2.0 access tokens that need periodic refreshing
Applied to files:
agents/grpc/src/grpc_client.ccagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:28:57.122Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/call/GEMINI.md:0-0
Timestamp: 2025-12-03T14:28:57.122Z
Learning: Applies to deps/grpc/src/core/credentials/call/**/call_creds_util.{h,cc} : Use utility functions from `call_creds_util.h` / `call_creds_util.cc` for working with call credentials
Applied to files:
agents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:29:08.569Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{google_iam_credentials.h,google_iam_credentials.cc} : Implement credentials for Google IAM transport security in `google_iam_credentials.h` / `google_iam_credentials.cc`
Applied to files:
agents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:28:57.122Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/call/GEMINI.md:0-0
Timestamp: 2025-12-03T14:28:57.122Z
Learning: Use call credentials in conjunction with channel credentials to provide a complete security solution for gRPC
Applied to files:
agents/grpc/src/grpc_client.ccagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:29:05.695Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:05.695Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/insecure_credentials.{h,cc} : Use insecure credentials implementation when no security is required for the transport
Applied to files:
agents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:31:43.163Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/handshaker/security/GEMINI.md:0-0
Timestamp: 2025-12-03T14:31:43.163Z
Learning: Applies to deps/grpc/src/core/handshaker/security/**/*secure_endpoint*.{h,cc} : Use `SecureEndpoint` class as a wrapper around `grpc_endpoint` to provide a secure channel
Applied to files:
agents/grpc/src/grpc_client.cctest/agents/test-grpc-basic.mjs
📚 Learning: 2025-12-03T14:29:05.695Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:05.695Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/local_credentials.{h,cc} : Use local credentials to configure local credentials security (e.g., Unix Domain Sockets) for the transport
Applied to files:
agents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:28:32.158Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/client_channel/GEMINI.md:0-0
Timestamp: 2025-12-03T14:28:32.158Z
Learning: Applies to deps/grpc/src/core/client_channel/**/client_channel/connector.h : Implement the `Connector` interface to establish transport-level connections to backend addresses
Applied to files:
agents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:35:21.897Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/tsi/ssl/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:21.897Z
Learning: Applies to deps/grpc/src/core/tsi/ssl/**/*.{c,cc,cpp,h,hpp} : Keep OpenSSL up-to-date to ensure that gRPC applications are secure
Applied to files:
test/agents/test-grpc-basic.mjs
📚 Learning: 2025-07-08T14:47:34.724Z
Learnt from: santigimeno
Repo: nodesource/nsolid PR: 339
File: test/addons/nsolid-elf-utils/nsolid-elf-utils.js:16-18
Timestamp: 2025-07-08T14:47:34.724Z
Learning: In the nsolid test suite, external tool invocations (e.g., execSync to run `readelf`) are intentionally left uncaught so that any failure causes the test to fail rather than being skipped.
Applied to files:
test/agents/test-grpc-basic.mjstest/agents/test-grpc-keylog-file.mjs
📚 Learning: 2025-07-08T14:46:47.806Z
Learnt from: santigimeno
Repo: nodesource/nsolid PR: 339
File: test/addons/nsolid-elf-utils/nsolid-elf-utils.js:13-14
Timestamp: 2025-07-08T14:46:47.806Z
Learning: In the nsolid test suite, native addon bindings are expected to be built beforehand; tests do not add fs.existsSync guards to skip when bindings are missing.
Applied to files:
test/agents/test-grpc-basic.mjstest/agents/test-grpc-keylog-file.mjs
📚 Learning: 2025-12-03T14:32:27.692Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:27.692Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/*.{cc,cpp,cxx,c,h,hpp} : Use the `GRPC_TRACE_FLAG_ENABLED(tracer)` macro to check if a trace flag is enabled in gRPC C++ code
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:32:26.248Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:26.248Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/trace_flags.yaml : Add new trace flags to the `trace_flags.yaml` file with the flag name and description before regenerating the trace flag header and implementation files
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:32:24.501Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:24.501Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/*.{cc,h} : Use the `GRPC_TRACE_FLAG_ENABLED(tracer)` macro to check if a trace flag is enabled in gRPC C++ code
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:32:26.248Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:26.248Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/trace*.{h,cc} : Use `GRPC_TRACE_FLAG_ENABLED(tracer)` macro to check if a trace flag is enabled in gRPC core code
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:32:27.692Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:27.692Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/trace_flags.yaml : Add new trace flags as entries in the `trace_flags.yaml` file with the flag name and a brief description of what it does
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:33:50.104Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/plugin_registry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:33:50.104Z
Learning: Applies to deps/grpc/src/core/plugin_registry/**/grpc_plugin_registry*.cc : Use the `GRPC_NO_XDS` macro to control the inclusion of extra plugins and enable lightweight builds of gRPC without XDS dependencies
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:29:08.569Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{xds_credentials.h,xds_credentials.cc} : Implement credentials for XDS transport security in `xds_credentials.h` / `xds_credentials.cc`
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:32:24.501Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:24.501Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/*.{cc,h} : Use `GRPC_TRACE_DLOG(tracer, level)` macro to log trace messages in debug builds when a trace flag is enabled in gRPC C++ code
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:32:10.553Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/channel/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:10.553Z
Learning: Applies to deps/grpc/src/core/lib/channel/**/{channel_args,channel_stack,channel_stack_builder,promise_based_filter,connected_channel}.{h,cc} : Use the `ChannelArgs` class to represent an immutable set of key-value pairs for configuring a channel.
Applied to files:
agents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:28:49.058Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/GEMINI.md:0-0
Timestamp: 2025-12-03T14:28:49.058Z
Learning: Applies to deps/grpc/src/core/credentials/src/**/credentials/call/**/*.{c,h,cc,cpp,hpp} : Implement the `grpc_call_credentials` interface in call credentials modules to add per-call security information to gRPC calls
Applied to files:
agents/grpc/src/grpc_client.h
📚 Learning: 2025-12-02T14:38:52.914Z
Learnt from: santigimeno
Repo: nodesource/nsolid PR: 377
File: test/agents/test-zmq-heap-sampling.mjs:135-201
Timestamp: 2025-12-02T14:38:52.914Z
Learning: In ZMQ agent tests, `playground.client.config(config)` exercises the nsolid.start() config surface by sending an IPC message to the child process, which then calls `nsolid.start(config)` before returning the current config. There is no `playground.child` API in the ZMQ test helpers.
Applied to files:
test/agents/test-grpc-keylog-file.mjs
📚 Learning: 2025-07-08T14:48:04.827Z
Learnt from: santigimeno
Repo: nodesource/nsolid PR: 339
File: test/addons/nsolid-elf-utils/binding.cc:13-28
Timestamp: 2025-07-08T14:48:04.827Z
Learning: In nsolid test native addons (e.g., `test/addons/*/binding.cc`), additional JavaScript-facing argument validation is typically omitted because the tests supply well-formed inputs.
Applied to files:
test/agents/test-grpc-keylog-file.mjs
🧬 Code graph analysis (4)
agents/grpc/src/grpc_agent.cc (1)
agents/grpc/src/grpc_client.cc (4)
MakeNSolidServiceStub(93-97)MakeNSolidServiceStub(94-95)MakeCredentials(27-50)MakeCredentials(28-29)
test/common/nsolid-grpc-agent/index.js (2)
test/common/nsolid-grpc-agent/server.mjs (8)
opts(32-39)args(16-16)args(304-306)args(312-314)args(320-322)args(328-330)args(348-350)args(356-358)test/common/nsolid-grpc-agent/client.js (1)
args(20-20)
test/agents/test-grpc-basic.mjs (1)
test/common/nsolid-grpc-agent/server.mjs (1)
agentId(79-79)
agents/grpc/src/grpc_client.h (1)
agents/grpc/src/grpc_client.cc (6)
MakeChannel(55-73)MakeChannel(56-57)MakeCredentials(27-50)MakeCredentials(28-29)MakeNSolidServiceStub(93-97)MakeNSolidServiceStub(94-95)
🪛 Cppcheck (2.19.0)
agents/grpc/src/grpc_client.cc
[error] 15-15: failed to evaluate #elif condition, undefined function-like macro invocation
(syntaxError)
[error] 16-16: #error "Protobuf C++ gencode is built with an incompatible version of"
(preprocessorErrorDirective)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: coverage-windows
- GitHub Check: coverage-linux
- GitHub Check: coverage-linux-without-intl
- GitHub Check: build-tarball
- GitHub Check: test-linux (ubuntu-24.04)
- GitHub Check: test-linux (ubuntu-24.04-arm)
- GitHub Check: test-macOS
- GitHub Check: lint-js-and-md
🔇 Additional comments (19)
agents/grpc/src/grpc_agent.h (1)
370-370: LGTM!The addition of
tls_keylog_file_is well-placed alongside other TLS-related members and follows the existing naming conventions.test/common/nsolid-grpc-agent/server.mjs (3)
3-3: LGTM!The imports for
parseArgsandfixturesare appropriate for implementing TLS support in the test server.Also applies to: 7-7
9-16: LGTM!The parseArgs configuration is clean and straightforward, with a sensible default of
falsefor the TLS option.
226-238: LGTM!The conditional TLS credential setup is implemented correctly. When TLS is enabled, the server loads test certificates from fixtures and creates SSL credentials with client authentication disabled, which is appropriate for testing scenarios.
agents/grpc/src/grpc_agent.cc (2)
68-68: LGTM!The constant definition for the environment variable name is clear and follows the existing naming convention.
1066-1074: LGTM!The TLS keylog file is correctly passed to the NSolid service stub creation and credential updates. The comment explaining the CommandStream initialization order is helpful for understanding the sequencing.
test/common/nsolid-grpc-agent/index.js (3)
71-71: LGTM!The addition of the private
#optsfield and updated constructor to accept options is clean. The default empty object ensures backward compatibility.Also applies to: 73-76
81-83: LGTM!The conditional TLS flag is correctly passed to the server subprocess when enabled.
88-88: LGTM!Good cleanup removing the unnecessary
+ ''concatenation.deps/opentelemetry-cpp/otlp-http-exporter.gyp (1)
65-65: I need the review comment to be rewritten. Please provide the review comment within<review_comment>tags in your next message, and I will rewrite it in the correct format.test/agents/test-grpc-keylog-file.mjs (2)
10-32: Well-structured test for TLS keylog functionality.The test correctly:
- Creates a TLS-enabled gRPC server
- Establishes a connection with keylog enabled
- Verifies the keylog file is created
- Uses
unlinkSyncto confirm file existence (throws if missing, causing test failure as intended)
41-41: NSOLID_GRPC_KEYLOG: '1' is correct.The implementation checks if the value is non-empty and not equal to "0" (line 466 of agents/grpc/src/grpc_agent.cc), which means it accepts any truthy value. Using '1' to enable keylog file generation is appropriate.
agents/grpc/src/grpc_client.cc (4)
56-73: Good refactoring of credentials creation.The extraction of credential creation logic into
MakeCredentialsimproves code organization and makes the TLS keylog support more maintainable.
94-97: Parameter propagation is correct.The
tls_keylog_fileparameter is correctly propagated through the call chain toMakeChannel.
43-43: The gRPC TLS keylog API is correctly implemented.The
set_tls_session_key_log_file_path()method is the correct API and supports TLS 1.3. The implementation requires OpenSSL 1.1.1 or later (via compile-time checkOPENSSL_VERSION_NUMBER >= 0x10101000), which introduced TLS 1.3 support. gRPC defaults to TLS 1.3 as the maximum version when not otherwise constrained.
34-45: The system CA certificate fallback behavior is correct and documented.When
ssl_credentials_cacert_as_stringis empty buttls_keylog_fileis set,TlsCredentialswill use system CA certificates on the client side. This is the documented default behavior in gRPC: if no certificate provider is set andwatch_root_certs()is not called, the implementation falls back to the root certificates stored in the system's default location.However, this reliance on implicit system CA behavior could be made more explicit in the code with a comment or by unconditionally calling
watch_root_certs()even when the cacert is empty, to make the intent clearer.test/agents/test-grpc-basic.mjs (3)
15-39: Good TLS test coverage addition.The test function signature and implementation are correctly updated to support both TLS and insecure modes.
129-158: Environment configuration correctly distinguishes TLS modes.The test configurations properly set:
NSOLID_GRPC_INSECURE=1for insecure connectionsNSOLID_GRPC_CERTSwith the test certificate path for secure connectionsThis provides comprehensive coverage of both connection modes.
160-168: Excellent test coverage improvement.The test suite now runs each test in both insecure and secure modes, providing comprehensive validation of TLS functionality. The console logging clearly identifies which mode is being tested.
santigimeno
force-pushed
the
santi/tls_keylog_file
branch
from
3943979 to
1e44b9d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @agents/grpc/src/grpc_agent.cc:
- Around line 464-472: The keylog filename is being constructed with a CWD-based
prefix ("./nsolid-tls-keylog-...") but the PR intends it to live next to the
binary; change the path construction for tls_keylog_file_ so it uses the
binary/executable directory rather than "./": compute the binary path (e.g. via
uv_exepath or platform-specific /proc/self/exe) and join that directory with
"nsolid-tls-keylog-<pid>.log" when setting tls_keylog_file_, keep the existing
pid suffix (uv_os_getpid()) and retain the unlink/cleanup logic, and add a safe
fallback to the current working directory if the binary path cannot be resolved.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
agents/grpc/src/grpc_agent.ccagents/grpc/src/grpc_agent.hagents/grpc/src/grpc_client.ccagents/grpc/src/grpc_client.hdeps/opentelemetry-cpp/otlp-http-exporter.gyptest/agents/test-grpc-keylog-file.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- test/agents/test-grpc-keylog-file.mjs
🧰 Additional context used
🧠 Learnings (31)
📓 Common learnings
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{tls_credentials.h,tls_credentials.cc} : Implement credentials for TLS transport security in `tls_credentials.h` / `tls_credentials.cc`
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:05.695Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/tls_credentials.{h,cc} : Use TLS (Transport Layer Security) credentials to configure TLS security for the Chttp2 transport
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/ext/filters/logging/GEMINI.md:0-0
Timestamp: 2025-12-03T14:30:21.671Z
Learning: Applies to deps/grpc/src/core/ext/filters/logging/**/{logging_filter.h,logging_filter.cc} : Implement gRPC logging filter to log call data including method name, peer address, and call status
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/tsi/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:15.845Z
Learning: Create gRPC-specific implementations of the `tsi_transport_security` interface in dedicated files such as `transport_security_grpc.h` and `transport_security_grpc.cc`
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/ext/filters/logging/GEMINI.md:0-0
Timestamp: 2025-12-03T14:30:16.982Z
Learning: Applies to deps/grpc/src/core/ext/filters/logging/**/logging_filter.{h,cc} : Implement the logging filter in `logging_filter.h` and `logging_filter.cc` to log gRPC call data including method name, peer address, and call status
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/security/authorization/GEMINI.md:0-0
Timestamp: 2025-12-03T14:33:17.207Z
Learning: Applies to deps/grpc/src/core/lib/security/authorization/**/audit_logging.{h,cc} : Implement audit logging interfaces to record information about authorization decisions for security auditing and debugging purposes
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/tsi/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:16.494Z
Learning: TSI provides a unified interface for performing transport-level security handshakes and protecting data, allowing gRPC to support different security mechanisms without changing core transport code
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/tsi/ssl/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:21.897Z
Learning: Applies to deps/grpc/src/core/tsi/ssl/**/*.{c,cc,cpp,h,hpp} : Keep OpenSSL up-to-date to ensure that gRPC applications are secure
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{local_credentials.h,local_credentials.cc} : Implement credentials for local transport security (e.g., UDS) in `local_credentials.h` / `local_credentials.cc`
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{alts_credentials.h,alts_credentials.cc} : Implement credentials for ALTS transport security in `alts_credentials.h` / `alts_credentials.cc`
📚 Learning: 2025-12-03T14:29:08.569Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{tls_credentials.h,tls_credentials.cc} : Implement credentials for TLS transport security in `tls_credentials.h` / `tls_credentials.cc`
Applied to files:
agents/grpc/src/grpc_client.hagents/grpc/src/grpc_agent.ccagents/grpc/src/grpc_client.ccagents/grpc/src/grpc_agent.hdeps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:29:05.695Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:05.695Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/tls_credentials.{h,cc} : Use TLS (Transport Layer Security) credentials to configure TLS security for the Chttp2 transport
Applied to files:
agents/grpc/src/grpc_client.hagents/grpc/src/grpc_agent.ccagents/grpc/src/grpc_client.ccagents/grpc/src/grpc_agent.h
📚 Learning: 2025-12-03T14:29:08.569Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{alts_credentials.h,alts_credentials.cc} : Implement credentials for ALTS transport security in `alts_credentials.h` / `alts_credentials.cc`
Applied to files:
agents/grpc/src/grpc_client.hagents/grpc/src/grpc_agent.ccagents/grpc/src/grpc_client.ccagents/grpc/src/grpc_agent.hdeps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:29:05.695Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:05.695Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/*_credentials.{h,cc} : Implement transport-specific credentials in dedicated header/implementation file pairs (e.g., `alts_credentials.h` / `alts_credentials.cc`, `tls_credentials.h` / `tls_credentials.cc`)
Applied to files:
agents/grpc/src/grpc_client.hagents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:29:08.569Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{local_credentials.h,local_credentials.cc} : Implement credentials for local transport security (e.g., UDS) in `local_credentials.h` / `local_credentials.cc`
Applied to files:
agents/grpc/src/grpc_client.hagents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:29:08.569Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{insecure_credentials.h,insecure_credentials.cc} : Implement credentials for insecure (no security) transport in `insecure_credentials.h` / `insecure_credentials.cc`
Applied to files:
agents/grpc/src/grpc_client.hagents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:28:57.122Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/call/GEMINI.md:0-0
Timestamp: 2025-12-03T14:28:57.122Z
Learning: Applies to deps/grpc/src/core/credentials/call/**/*_credentials.{h,cc} : Implement call credentials for adding per-call security information to gRPC calls, such as OAuth 2.0 access tokens that need periodic refreshing
Applied to files:
agents/grpc/src/grpc_client.hagents/grpc/src/grpc_client.ccdeps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:35:15.845Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/tsi/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:15.845Z
Learning: Create gRPC-specific implementations of the `tsi_transport_security` interface in dedicated files such as `transport_security_grpc.h` and `transport_security_grpc.cc`
Applied to files:
agents/grpc/src/grpc_client.hagents/grpc/src/grpc_agent.ccagents/grpc/src/grpc_client.ccagents/grpc/src/grpc_agent.h
📚 Learning: 2025-12-03T14:29:05.695Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:05.695Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/alts_credentials.{h,cc} : Use ALTS (Application Layer Transport Security) credentials to configure ALTS security for the transport
Applied to files:
agents/grpc/src/grpc_client.hagents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:32:10.553Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/channel/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:10.553Z
Learning: Applies to deps/grpc/src/core/lib/channel/**/{channel_args,channel_stack,channel_stack_builder,promise_based_filter,connected_channel}.{h,cc} : Use the `ChannelArgs` class to represent an immutable set of key-value pairs for configuring a channel.
Applied to files:
agents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:28:57.122Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/call/GEMINI.md:0-0
Timestamp: 2025-12-03T14:28:57.122Z
Learning: Use call credentials in conjunction with channel credentials to provide a complete security solution for gRPC
Applied to files:
agents/grpc/src/grpc_client.hagents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:33:17.207Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/security/authorization/GEMINI.md:0-0
Timestamp: 2025-12-03T14:33:17.207Z
Learning: Applies to deps/grpc/src/core/lib/security/authorization/**/audit_logging.{h,cc} : Implement audit logging interfaces to record information about authorization decisions for security auditing and debugging purposes
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:30:21.671Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/ext/filters/logging/GEMINI.md:0-0
Timestamp: 2025-12-03T14:30:21.671Z
Learning: Applies to deps/grpc/src/core/ext/filters/logging/**/{logging_filter.h,logging_filter.cc} : Implement gRPC logging filter to log call data including method name, peer address, and call status
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:33:20.681Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/security/authorization/GEMINI.md:0-0
Timestamp: 2025-12-03T14:33:20.681Z
Learning: Applies to deps/grpc/src/core/lib/security/authorization/**/audit_logging.{h,cc} : Define audit logging interfaces to record information about authorization decisions for security auditing and debugging purposes
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:35:21.897Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/tsi/ssl/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:21.897Z
Learning: Applies to deps/grpc/src/core/tsi/ssl/**/*.{c,cc,cpp,h,hpp} : Keep OpenSSL up-to-date to ensure that gRPC applications are secure
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:30:16.982Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/ext/filters/logging/GEMINI.md:0-0
Timestamp: 2025-12-03T14:30:16.982Z
Learning: Applies to deps/grpc/src/core/ext/filters/logging/**/logging_filter.{h,cc} : Implement the logging filter in `logging_filter.h` and `logging_filter.cc` to log gRPC call data including method name, peer address, and call status
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:32:24.501Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:24.501Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/*.{cc,h} : Use `GRPC_TRACE_LOG(tracer, level)` macro to log trace messages when a trace flag is enabled in gRPC C++ code
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:29:08.569Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{google_iam_credentials.h,google_iam_credentials.cc} : Implement credentials for Google IAM transport security in `google_iam_credentials.h` / `google_iam_credentials.cc`
Applied to files:
agents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:29:08.569Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{xds_credentials.h,xds_credentials.cc} : Implement credentials for XDS transport security in `xds_credentials.h` / `xds_credentials.cc`
Applied to files:
agents/grpc/src/grpc_client.ccdeps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:32:41.659Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/experiments/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:41.659Z
Learning: Applies to deps/grpc/src/core/lib/experiments/**/*.{cc,h} : Use `grpc_core::IsExperimentEnabled` function to check if an experiment is enabled
Applied to files:
agents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:31:43.163Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/handshaker/security/GEMINI.md:0-0
Timestamp: 2025-12-03T14:31:43.163Z
Learning: Applies to deps/grpc/src/core/handshaker/security/**/*secure_endpoint*.{h,cc} : Use `SecureEndpoint` class as a wrapper around `grpc_endpoint` to provide a secure channel
Applied to files:
agents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:29:05.695Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:05.695Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/insecure_credentials.{h,cc} : Use insecure credentials implementation when no security is required for the transport
Applied to files:
agents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:29:05.695Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:05.695Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/local_credentials.{h,cc} : Use local credentials to configure local credentials security (e.g., Unix Domain Sockets) for the transport
Applied to files:
agents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:28:32.158Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/client_channel/GEMINI.md:0-0
Timestamp: 2025-12-03T14:28:32.158Z
Learning: Applies to deps/grpc/src/core/client_channel/**/client_channel/connector.h : Implement the `Connector` interface to establish transport-level connections to backend addresses
Applied to files:
agents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:32:27.692Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:27.692Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/*.{cc,cpp,cxx,c,h,hpp} : Use the `GRPC_TRACE_FLAG_ENABLED(tracer)` macro to check if a trace flag is enabled in gRPC C++ code
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:32:26.248Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:26.248Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/trace_flags.yaml : Add new trace flags to the `trace_flags.yaml` file with the flag name and description before regenerating the trace flag header and implementation files
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:32:24.501Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:24.501Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/*.{cc,h} : Use the `GRPC_TRACE_FLAG_ENABLED(tracer)` macro to check if a trace flag is enabled in gRPC C++ code
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:32:27.692Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:27.692Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/trace_flags.yaml : Add new trace flags as entries in the `trace_flags.yaml` file with the flag name and a brief description of what it does
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:32:26.248Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:26.248Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/trace*.{h,cc} : Use `GRPC_TRACE_FLAG_ENABLED(tracer)` macro to check if a trace flag is enabled in gRPC core code
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:33:50.104Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/plugin_registry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:33:50.104Z
Learning: Applies to deps/grpc/src/core/plugin_registry/**/grpc_plugin_registry*.cc : Use the `GRPC_NO_XDS` macro to control the inclusion of extra plugins and enable lightweight builds of gRPC without XDS dependencies
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
🧬 Code graph analysis (1)
agents/grpc/src/grpc_client.h (1)
agents/grpc/src/grpc_client.cc (6)
MakeChannel(61-79)MakeChannel(62-63)MakeCredentials(33-56)MakeCredentials(34-35)MakeNSolidServiceStub(99-103)MakeNSolidServiceStub(100-101)
🪛 Cppcheck (2.19.0)
agents/grpc/src/grpc_client.cc
[error] 15-15: failed to evaluate #elif condition, undefined function-like macro invocation
(syntaxError)
[error] 16-16: #error "Protobuf C++ gencode is built with an incompatible version of"
(preprocessorErrorDirective)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: test-macOS
- GitHub Check: coverage-linux-without-intl
- GitHub Check: build-tarball
- GitHub Check: test-linux (ubuntu-24.04)
- GitHub Check: test-linux (ubuntu-24.04-arm)
- GitHub Check: coverage-linux
- GitHub Check: coverage-windows
- GitHub Check: lint-js-and-md
🔇 Additional comments (7)
agents/grpc/src/grpc_agent.h (1)
370-370: LGTM!The new
tls_keylog_file_member is appropriately placed alongside other TLS-related members and follows the existing naming conventions.agents/grpc/src/grpc_client.h (1)
62-84: LGTM!The API extensions are well-designed:
MakeChannelhas a default parameter maintaining backward compatibility- New
MakeCredentialsmethod cleanly encapsulates credential creation logicMakeNSolidServiceStubsignature update aligns with the new TLS keylog flowThe documentation comments are appropriate for the new method.
agents/grpc/src/grpc_agent.cc (1)
1066-1074: LGTM!The TLS keylog file path is correctly propagated to both the NSolid service stub and the OTLP client credentials, ensuring consistent keylog behavior across all gRPC connections.
agents/grpc/src/grpc_client.cc (4)
13-22: LGTM - Experimental API usage properly documented.The comment now clearly explains why experimental gRPC TLS APIs are required (for
set_tls_session_key_log_file_pathwhich isn't available in stable APIs) and references the pinned gRPC version (1.76.0).
30-56: LGTM!The
MakeCredentialsimplementation correctly handles all three credential scenarios:
- Insecure connections when SSL is disabled
- TLS with keylog support using experimental APIs when
tls_keylog_fileis provided- Standard TLS using stable
SslCredentialswhen keylog is not neededThe fallback to stable APIs when keylog isn't required is a good design choice that minimizes exposure to experimental APIs.
61-78: LGTM!The channel creation correctly delegates to
MakeCredentialsand maintains the keepalive configuration. The refactoring centralizes credential logic cleanly.
99-102: LGTM!
MakeNSolidServiceStubcorrectly propagates thetls_keylog_fileparameter through to channel creation.
By setting the `NSOLID_GRPC_KEYLOG` to a truthy value, a new keylog file will be generated that should allow us to decrypt the TLS v1.3 connections gRPC Agent uses. Very useful to debug issues on production. The file will be generated in the current working directory with the following format: `nsolid-tls-keylog-<process_pid>.log`.
santigimeno
force-pushed
the
santi/tls_keylog_file
branch
from
1e44b9d to
2afa7f1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
agents/grpc/src/grpc_client.h (1)
82-84: Consider adding a default parameter for consistency.
MakeChannelhas a default empty string fortls_keylog_file, butMakeNSolidServiceStubdoes not. If this is intentional to force callers to explicitly consider the keylog parameter, this is fine. Otherwise, consider adding a default for API consistency.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
agents/grpc/src/grpc_agent.ccagents/grpc/src/grpc_agent.hagents/grpc/src/grpc_client.ccagents/grpc/src/grpc_client.hdeps/opentelemetry-cpp/otlp-http-exporter.gyptest/agents/test-grpc-keylog-file.mjs
🚧 Files skipped from review as they are similar to previous changes (3)
- test/agents/test-grpc-keylog-file.mjs
- agents/grpc/src/grpc_agent.cc
- agents/grpc/src/grpc_agent.h
🧰 Additional context used
🧠 Learnings (34)
📓 Common learnings
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{tls_credentials.h,tls_credentials.cc} : Implement credentials for TLS transport security in `tls_credentials.h` / `tls_credentials.cc`
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:05.695Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/tls_credentials.{h,cc} : Use TLS (Transport Layer Security) credentials to configure TLS security for the Chttp2 transport
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/ext/filters/logging/GEMINI.md:0-0
Timestamp: 2025-12-03T14:30:21.671Z
Learning: Applies to deps/grpc/src/core/ext/filters/logging/**/{logging_filter.h,logging_filter.cc} : Implement gRPC logging filter to log call data including method name, peer address, and call status
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/ext/filters/logging/GEMINI.md:0-0
Timestamp: 2025-12-03T14:30:16.982Z
Learning: Applies to deps/grpc/src/core/ext/filters/logging/**/logging_filter.{h,cc} : Implement the logging filter in `logging_filter.h` and `logging_filter.cc` to log gRPC call data including method name, peer address, and call status
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/tsi/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:15.845Z
Learning: Create gRPC-specific implementations of the `tsi_transport_security` interface in dedicated files such as `transport_security_grpc.h` and `transport_security_grpc.cc`
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/tsi/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:16.494Z
Learning: TSI provides a unified interface for performing transport-level security handshakes and protecting data, allowing gRPC to support different security mechanisms without changing core transport code
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/security/authorization/GEMINI.md:0-0
Timestamp: 2025-12-03T14:33:17.207Z
Learning: Applies to deps/grpc/src/core/lib/security/authorization/**/audit_logging.{h,cc} : Implement audit logging interfaces to record information about authorization decisions for security auditing and debugging purposes
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{local_credentials.h,local_credentials.cc} : Implement credentials for local transport security (e.g., UDS) in `local_credentials.h` / `local_credentials.cc`
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/call/GEMINI.md:0-0
Timestamp: 2025-12-03T14:28:57.122Z
Learning: Applies to deps/grpc/src/core/credentials/call/**/*_credentials.{h,cc} : Implement call credentials for adding per-call security information to gRPC calls, such as OAuth 2.0 access tokens that need periodic refreshing
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{alts_credentials.h,alts_credentials.cc} : Implement credentials for ALTS transport security in `alts_credentials.h` / `alts_credentials.cc`
📚 Learning: 2025-12-03T14:29:08.569Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{alts_credentials.h,alts_credentials.cc} : Implement credentials for ALTS transport security in `alts_credentials.h` / `alts_credentials.cc`
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gypagents/grpc/src/grpc_client.ccagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:29:08.569Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{xds_credentials.h,xds_credentials.cc} : Implement credentials for XDS transport security in `xds_credentials.h` / `xds_credentials.cc`
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gypagents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:29:08.569Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{tls_credentials.h,tls_credentials.cc} : Implement credentials for TLS transport security in `tls_credentials.h` / `tls_credentials.cc`
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gypagents/grpc/src/grpc_client.ccagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:33:50.104Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/plugin_registry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:33:50.104Z
Learning: Applies to deps/grpc/src/core/plugin_registry/**/grpc_plugin_registry*.cc : Use the `GRPC_NO_XDS` macro to control the inclusion of extra plugins and enable lightweight builds of gRPC without XDS dependencies
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:32:27.692Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:27.692Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/*.{cc,cpp,cxx,c,h,hpp} : Use the `GRPC_TRACE_FLAG_ENABLED(tracer)` macro to check if a trace flag is enabled in gRPC C++ code
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:28:57.122Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/call/GEMINI.md:0-0
Timestamp: 2025-12-03T14:28:57.122Z
Learning: Applies to deps/grpc/src/core/credentials/call/**/*_credentials.{h,cc} : Implement call credentials for adding per-call security information to gRPC calls, such as OAuth 2.0 access tokens that need periodic refreshing
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gypagents/grpc/src/grpc_client.ccagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:29:05.695Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:05.695Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/*_credentials.{h,cc} : Implement transport-specific credentials in dedicated header/implementation file pairs (e.g., `alts_credentials.h` / `alts_credentials.cc`, `tls_credentials.h` / `tls_credentials.cc`)
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gypagents/grpc/src/grpc_client.ccagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:32:24.501Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:24.501Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/*.{cc,h} : Use the `GRPC_TRACE_FLAG_ENABLED(tracer)` macro to check if a trace flag is enabled in gRPC C++ code
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:29:08.569Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{google_iam_credentials.h,google_iam_credentials.cc} : Implement credentials for Google IAM transport security in `google_iam_credentials.h` / `google_iam_credentials.cc`
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gypagents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:32:26.248Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:26.248Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/trace*.{h,cc} : Use `GRPC_TRACE_FLAG_ENABLED(tracer)` macro to check if a trace flag is enabled in gRPC core code
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:29:08.569Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{insecure_credentials.h,insecure_credentials.cc} : Implement credentials for insecure (no security) transport in `insecure_credentials.h` / `insecure_credentials.cc`
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gypagents/grpc/src/grpc_client.ccagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:32:41.659Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/experiments/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:41.659Z
Learning: Applies to deps/grpc/src/core/lib/experiments/**/*.{cc,h} : Use `grpc_core::IsExperimentEnabled` function to check if an experiment is enabled
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gypagents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:32:41.659Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/experiments/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:41.659Z
Learning: Applies to deps/grpc/src/core/lib/experiments/**/*_test.{cc,h} : Use `grpc_core::ForceEnableExperiment` function only for testing purposes to force enable or disable experiments
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:35:34.369Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:34.369Z
Learning: Applies to deps/grpc/src/core/util/src/**/env.h : Environment variable utilities should be provided in `env.h`
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:32:26.248Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:26.248Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/trace*.{h,cc} : Use `GRPC_TRACE_LOG(tracer, level)` macro to log trace messages when a trace flag is enabled in gRPC core code
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:32:27.692Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:27.692Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/*.{cc,cpp,cxx,c,h,hpp} : Use `GRPC_TRACE_LOG(tracer, level)` macro to log trace messages when a trace flag is enabled in gRPC C++ code
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:32:26.248Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:26.248Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/trace*.{h,cc} : Use `GRPC_TRACE_DLOG(tracer, level)` macro to log trace messages in debug builds when a trace flag is enabled in gRPC core code
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:32:26.248Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:26.248Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/trace*.{h,cc} : Use `GRPC_TRACE_VLOG(tracer, level)` macro to log verbose trace messages when a trace flag is enabled in gRPC core code
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:32:24.501Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:24.501Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/*.{cc,h} : Use `GRPC_TRACE_LOG(tracer, level)` macro to log trace messages when a trace flag is enabled in gRPC C++ code
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:32:27.692Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:27.692Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/*.{cc,cpp,cxx,c,h,hpp} : Use `GRPC_TRACE_DLOG(tracer, level)` macro to log trace messages in debug builds when a trace flag is enabled in gRPC C++ code
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:32:27.692Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/debug/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:27.692Z
Learning: Applies to deps/grpc/src/core/lib/debug/**/*.{cc,cpp,cxx,c,h,hpp} : Use `GRPC_TRACE_VLOG(tracer, level)` macro to log verbose trace messages when a trace flag is enabled in gRPC C++ code
Applied to files:
deps/opentelemetry-cpp/otlp-http-exporter.gyp
📚 Learning: 2025-12-03T14:29:05.695Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:05.695Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/tls_credentials.{h,cc} : Use TLS (Transport Layer Security) credentials to configure TLS security for the Chttp2 transport
Applied to files:
agents/grpc/src/grpc_client.ccagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:29:08.569Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:08.569Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/{local_credentials.h,local_credentials.cc} : Implement credentials for local transport security (e.g., UDS) in `local_credentials.h` / `local_credentials.cc`
Applied to files:
agents/grpc/src/grpc_client.ccagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:29:05.695Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:05.695Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/alts_credentials.{h,cc} : Use ALTS (Application Layer Transport Security) credentials to configure ALTS security for the transport
Applied to files:
agents/grpc/src/grpc_client.ccagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:28:49.058Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/GEMINI.md:0-0
Timestamp: 2025-12-03T14:28:49.058Z
Learning: Applies to deps/grpc/src/core/credentials/src/**/credentials/call/**/*.{c,h,cc,cpp,hpp} : Implement the `grpc_call_credentials` interface in call credentials modules to add per-call security information to gRPC calls
Applied to files:
agents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:28:57.122Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/call/GEMINI.md:0-0
Timestamp: 2025-12-03T14:28:57.122Z
Learning: Use call credentials in conjunction with channel credentials to provide a complete security solution for gRPC
Applied to files:
agents/grpc/src/grpc_client.ccagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:35:15.845Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/tsi/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:15.845Z
Learning: Create gRPC-specific implementations of the `tsi_transport_security` interface in dedicated files such as `transport_security_grpc.h` and `transport_security_grpc.cc`
Applied to files:
agents/grpc/src/grpc_client.ccagents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:31:43.163Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/handshaker/security/GEMINI.md:0-0
Timestamp: 2025-12-03T14:31:43.163Z
Learning: Applies to deps/grpc/src/core/handshaker/security/**/*secure_endpoint*.{h,cc} : Use `SecureEndpoint` class as a wrapper around `grpc_endpoint` to provide a secure channel
Applied to files:
agents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:29:05.695Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:05.695Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/insecure_credentials.{h,cc} : Use insecure credentials implementation when no security is required for the transport
Applied to files:
agents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:29:05.695Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/credentials/transport/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:05.695Z
Learning: Applies to deps/grpc/src/core/credentials/transport/**/local_credentials.{h,cc} : Use local credentials to configure local credentials security (e.g., Unix Domain Sockets) for the transport
Applied to files:
agents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:28:32.158Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/client_channel/GEMINI.md:0-0
Timestamp: 2025-12-03T14:28:32.158Z
Learning: Applies to deps/grpc/src/core/client_channel/**/client_channel/connector.h : Implement the `Connector` interface to establish transport-level connections to backend addresses
Applied to files:
agents/grpc/src/grpc_client.cc
📚 Learning: 2025-12-03T14:32:10.553Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/channel/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:10.553Z
Learning: Applies to deps/grpc/src/core/lib/channel/**/{channel_args,channel_stack,channel_stack_builder,promise_based_filter,connected_channel}.{h,cc} : Use the `ChannelArgs` class to represent an immutable set of key-value pairs for configuring a channel.
Applied to files:
agents/grpc/src/grpc_client.h
📚 Learning: 2025-12-03T14:29:25.191Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/ext/filters/channel_idle/GEMINI.md:0-0
Timestamp: 2025-12-03T14:29:25.191Z
Learning: Applies to deps/grpc/src/core/ext/filters/channel_idle/**/*channel*filter*.{h,cc} : Use the 'legacy' prefix in class names to indicate that filters are based on the older channel filter API and will likely be replaced by a more modern implementation in the future
Applied to files:
agents/grpc/src/grpc_client.h
🧬 Code graph analysis (1)
agents/grpc/src/grpc_client.h (1)
agents/grpc/src/grpc_client.cc (6)
MakeChannel(61-79)MakeChannel(62-63)MakeCredentials(33-56)MakeCredentials(34-35)MakeNSolidServiceStub(99-103)MakeNSolidServiceStub(100-101)
🪛 Cppcheck (2.19.0)
agents/grpc/src/grpc_client.cc
[error] 15-15: failed to evaluate #elif condition, undefined function-like macro invocation
(syntaxError)
[error] 16-16: #error "Protobuf C++ gencode is built with an incompatible version of"
(preprocessorErrorDirective)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: test-macOS
- GitHub Check: test-linux (ubuntu-24.04-arm)
- GitHub Check: test-linux (ubuntu-24.04)
- GitHub Check: build-tarball
- GitHub Check: coverage-linux-without-intl
- GitHub Check: coverage-linux
- GitHub Check: coverage-windows
- GitHub Check: lint-js-and-md
🔇 Additional comments (6)
deps/opentelemetry-cpp/otlp-http-exporter.gyp (1)
65-65: LGTM! Credential preview flag correctly added.The
ENABLE_OTLP_GRPC_CREDENTIAL_PREVIEWdefine is properly added in both the maindefinesarray anddirect_dependent_settings, consistent with the pattern used for other defines likeENABLE_ASYNC_EXPORT. This enables the OpenTelemetry C++ preview API for custom gRPC channel credentials, which is necessary for the TLS keylog feature being implemented in this PR.Also applies to: 78-78
agents/grpc/src/grpc_client.h (1)
62-71: Well-structured API additions for TLS keylog support.The new
MakeCredentialsmethod cleanly separates credential creation logic, and the default empty string fortls_keylog_fileinMakeChannelmaintains backward compatibility. The API design follows good separation of concerns.agents/grpc/src/grpc_client.cc (4)
13-22: Good documentation of experimental API usage.The comment properly justifies using experimental gRPC TLS APIs (needed for
set_tls_session_key_log_file_pathwhich isn't available in stableSslCredentials), and documents the pinned gRPC version. This addresses the previous review concern.
61-78: Clean refactoring of channel creation.The
MakeChannelimplementation properly delegates credential creation toMakeCredentials, maintaining the keepalive configuration and adding keylog support. The code is well-documented.
99-102: LGTM!The stub creation correctly forwards the
tls_keylog_fileparameter toMakeChannel.
40-51: The code correctly uses system root certificates when keylog is enabled without custom CA certs. Whenssl_credentials_cacert_as_stringis empty,TlsChannelCredentialsOptionsdefaults to the system root certificate store—this is the expected behavior in gRPC's experimental TLS API.
RafaelGSS
approved these changes
Jan 6, 2026
santigimeno
added a commit
that referenced
this pull request
Jan 7, 2026
Only grpc insecure connections were being used in the tests. Also, these new tests allow to cover for the untested `NSOLID_GRPC_CERTS` env variable. PR-URL: #406 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
santigimeno
added a commit
that referenced
this pull request
Jan 7, 2026
By setting the `NSOLID_GRPC_KEYLOG` to a truthy value, a new keylog file will be generated that should allow us to decrypt the TLS v1.3 connections gRPC Agent uses. Very useful to debug issues on production. The file will be generated in the current working directory with the following format: `nsolid-tls-keylog-<process_pid>.log`. PR-URL: #406 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Member
Author
|
Landed in ba6c1d0...7d4cbca |
santigimeno
added a commit
that referenced
this pull request
Jan 14, 2026
Only grpc insecure connections were being used in the tests. Also, these new tests allow to cover for the untested `NSOLID_GRPC_CERTS` env variable. PR-URL: #406 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
santigimeno
added a commit
that referenced
this pull request
Jan 14, 2026
By setting the `NSOLID_GRPC_KEYLOG` to a truthy value, a new keylog file will be generated that should allow us to decrypt the TLS v1.3 connections gRPC Agent uses. Very useful to debug issues on production. The file will be generated in the current working directory with the following format: `nsolid-tls-keylog-<process_pid>.log`. PR-URL: #406 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
santigimeno
added a commit
that referenced
this pull request
Jan 14, 2026
Only grpc insecure connections were being used in the tests. Also, these new tests allow to cover for the untested `NSOLID_GRPC_CERTS` env variable. PR-URL: #406 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
santigimeno
added a commit
that referenced
this pull request
Jan 14, 2026
By setting the `NSOLID_GRPC_KEYLOG` to a truthy value, a new keylog file will be generated that should allow us to decrypt the TLS v1.3 connections gRPC Agent uses. Very useful to debug issues on production. The file will be generated in the current working directory with the following format: `nsolid-tls-keylog-<process_pid>.log`. PR-URL: #406 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
santigimeno
added a commit
that referenced
this pull request
Jan 14, 2026
Only grpc insecure connections were being used in the tests. Also, these new tests allow to cover for the untested `NSOLID_GRPC_CERTS` env variable. PR-URL: #406 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
santigimeno
added a commit
that referenced
this pull request
Jan 14, 2026
By setting the `NSOLID_GRPC_KEYLOG` to a truthy value, a new keylog file will be generated that should allow us to decrypt the TLS v1.3 connections gRPC Agent uses. Very useful to debug issues on production. The file will be generated in the current working directory with the following format: `nsolid-tls-keylog-<process_pid>.log`. PR-URL: #406 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
santigimeno
added a commit
that referenced
this pull request
Jan 14, 2026
By setting the `NSOLID_GRPC_KEYLOG` to a truthy value, a new keylog file will be generated that should allow us to decrypt the TLS v1.3 connections gRPC Agent uses. Very useful to debug issues on production. The file will be generated in the current working directory with the following format: `nsolid-tls-keylog-<process_pid>.log`. PR-URL: #406 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
santigimeno
added a commit
that referenced
this pull request
Jan 14, 2026
By setting the `NSOLID_GRPC_KEYLOG` to a truthy value, a new keylog file will be generated that should allow us to decrypt the TLS v1.3 connections gRPC Agent uses. Very useful to debug issues on production. The file will be generated in the current working directory with the following format: `nsolid-tls-keylog-<process_pid>.log`. PR-URL: #406 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
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.
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.