agents: export extra_attrs when using OTLP#293
agents: export extra_attrs when using OTLP#293santigimeno wants to merge 1 commit intonode-v22.x-nsolid-v5.xfrom
Conversation
Which was overlooked in the initial implementation. Add tests covering the case. Fixes: nodesource/nsolid-roadmap#29
santigimeno
force-pushed
the
santi/otlp_extra_attrs
branch
from
777b00f to
1544c4d
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThe changes introduce enhanced support for OpenTelemetry attribute propagation in gRPC/OTLP tracing. The core logic in the agent now merges additional JSON attributes and supports arrays of primitive types (boolean, integer, unsigned integer, float, string) as span attributes, skipping arrays that are empty or contain mixed types. A new test suite verifies correct propagation of various attribute types, including arrays, for both HTTP and custom tracing scenarios. The test client for custom traces now sets an array of strings as an attribute and executes tracing logic asynchronously after a delay. Changes
Assessment against linked issues
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (1)
agents/otlp/src/otlp_common.cc (1)
422-503: Avoid usinggotofor skipping non-homogeneous arrays.
Although it works as intended, structured approaches (e.g., extracting the validation logic into a function that returns a boolean) can improve readability and maintainability.if (!item.is_boolean()) { - goto skip_array; + // Skip non-homogeneous arrays with early return or a helper function + is_valid_array = false; + break; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
agents/otlp/src/otlp_common.cc(2 hunks)test/agents/test-grpc-tracing.mjs(1 hunks)test/common/nsolid-grpc-agent/client.js(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
test/common/nsolid-grpc-agent/client.js (6)
test/common/nsolid-zmq-agent/client.js (7)
api(60-61)require(4-4)require(5-5)require(7-7)nsolid(6-6)tracer(63-63)span(64-65)test/common/nsolid-grpc-agent/index.js (4)
require(5-5)require(6-6)require(7-7)require(10-14)test/parallel/test-nsolid-grpc-heap-sampling.js (1)
nsolid(5-5)test/parallel/test-nsolid-grpc-profile.js (1)
nsolid(5-5)test/parallel/test-nsolid-grpc-snapshot.js (1)
nsolid(5-5)test/parallel/test-nsolid-grpc-heap-profile.js (1)
nsolid(5-5)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test-tarball-linux
- GitHub Check: coverage-windows
- GitHub Check: coverage-linux-without-intl
- GitHub Check: coverage-linux
🔇 Additional comments (2)
test/common/nsolid-grpc-agent/client.js (1)
59-74: Ensure the 1-second delay is sufficient for trace capture.
Switching to a delayedsetTimeout()block allows asynchronous trace recording. However, if slower environments exceed one second before the agent processes or exports the trace, the test may yield inconsistent results. Consider verifying or making the delay configurable if necessary.test/agents/test-grpc-tracing.mjs (1)
1-280: Comprehensive test suite appears solid.
This file thoroughly tests both HTTP and custom traces, including array-typed attributes. The structure is clear, and checks for resource attributes and various span types are well-organized.
Which was overlooked in the initial implementation. Add tests covering the case. Fixes: nodesource/nsolid-roadmap#29 PR-URL: #293 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
|
Landed in 354e74b |
Which was overlooked in the initial implementation. Add tests covering the case. Fixes: nodesource/nsolid-roadmap#29 PR-URL: #293 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Which was overlooked in the initial implementation. Add tests covering the case. Fixes: nodesource/nsolid-roadmap#29 PR-URL: #293 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Which was overlooked in the initial implementation. Add tests covering the case. Fixes: nodesource/nsolid-roadmap#29 PR-URL: #293 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Which was overlooked in the initial implementation. Add tests covering the case. Fixes: nodesource/nsolid-roadmap#29 PR-URL: #293 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
3 participants
Which was overlooked in the initial implementation. Add tests covering the case.
Fixes: https://github.com/nodesource/nsolid-roadmap/issues/29
Summary by CodeRabbit