src: use monotonic clock for timestamps #399
Conversation
WalkthroughReplaced ms_since_epoch() with new current_timestamp_ns()/current_timestamp_ms(), made ZMQ and gRPC agents' create_recorded() parameterless (they now obtain timestamps internally), and updated multiple metric/trace/log call sites to use the new utils-based timestamp functions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
🧹 Nitpick comments (4)
src/nsolid/nsolid_api.cc (1)
280-298: Thread and metrics-stream timestamps now share the same monotonic ms sourceUsing
utils::current_timestamp_ns() / 1e6for bothThreadMetrics::MetricsStor::timestampand datapointtsunifies them on a single monotonic, nanosecond-based clock converted to milliseconds. This keeps ordering consistent across per-thread metrics, process metrics, and streamed datapoints while avoiding wall-clock jumps.If more call sites start doing this, consider a small helper like
current_timestamp_ms()to avoid repeating the/ 1e6literal and make intent clearer.Also applies to: 1430-1438
src/nsolid.cc (1)
250-276: Process metrics timestamp correctly migrated to current_timestamp_nsUpdating
stor_.timestamptoutils::current_timestamp_ns() / 1000000keeps it as milliseconds since epoch while sourcing it from the shared monotonic clock. This is consistent with the rest of the PR’s timestamp changes and avoids per-callsystem_clockoverhead.For readability, you might mirror other call sites and use a floating literal (
1e6) or a named constant to make the ns→ms conversion intent more obvious.agents/zmq/src/zmq_agent.h (1)
537-538: create_recorded() API simplification is coherent with internal implementationMaking
ZmqAgent::create_recorded()parameterless and letting it internally callutils::current_timestamp_ns()matches the new gRPC helper and centralizes timestamp sourcing. As long as external code doesn’t depend on the old signature (it appears to be used only internally), this is a clean and compatible refactor.For symmetry with the gRPC helper, you could add a short comment in the definition documenting that the pair is
{seconds, nanoseconds}since epoch derived fromcurrent_timestamp_ns().agents/grpc/src/grpc_agent.cc (1)
81-85: Shared create_recorded() helper correctly wraps current_timestamp_ns for gRPC eventsThe new
create_recorded()helper cleanly convertsutils::current_timestamp_ns()into{seconds, nanoseconds}andPopulateCommon()now uses it directly, removing scatteredsystem_clock::now()logic and aligning gRPC event timestamps with the rest of the monotonic timestamp infrastructure. This matches the ZMQ agent’s behavior and looks correct.Minor nit: inside
namespace node::nsolid::grpc, you could shorten the call toutils::current_timestamp_ns()for readability, mirroring the ZMQ implementation.Also applies to: 118-125
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
agents/grpc/src/grpc_agent.cc(2 hunks)agents/zmq/src/zmq_agent.cc(4 hunks)agents/zmq/src/zmq_agent.h(1 hunks)src/nsolid.cc(1 hunks)src/nsolid/nsolid_api.cc(3 hunks)src/nsolid/nsolid_trace.cc(2 hunks)src/nsolid/nsolid_util.h(2 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:32.707Z
Learning: Applies to deps/grpc/src/core/util/**/*.{cc,h,cpp} : Use time-related functions from `time.h` and `time.cc`
📚 Learning: 2025-12-03T14:35:32.707Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:32.707Z
Learning: Applies to deps/grpc/src/core/util/**/*.{cc,h,cpp} : Use time-related functions from `time.h` and `time.cc`
Applied to files:
src/nsolid/nsolid_trace.ccsrc/nsolid.ccsrc/nsolid/nsolid_api.ccsrc/nsolid/nsolid_util.hagents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:35:35.065Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:35.065Z
Learning: Applies to deps/grpc/src/core/util/src/core/util/time.{h,cc} : Time-related functions should be implemented in dedicated header and implementation files (time.h / time.cc)
Applied to files:
src/nsolid/nsolid_util.hagents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:35:38.574Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/json/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:38.574Z
Learning: Maintain C++-native JSON library implementation with no external dependencies
Applied to files:
src/nsolid/nsolid_util.h
📚 Learning: 2025-12-03T14:40:17.411Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/json/GEMINI.md:0-0
Timestamp: 2025-12-03T14:40:17.411Z
Learning: Applies to deps/grpc/src/core/util/json/**/json*.{cc,cpp,h,hpp} : Design JSON utilities to be efficient and avoid unnecessary memory allocations
Applied to files:
src/nsolid/nsolid_util.h
📚 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/**/time.{h,cc} : Time-related functions should be implemented in `time.h` and `time.cc`
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:34:55.521Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/telemetry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:55.521Z
Learning: Applies to deps/grpc/src/core/telemetry/**/telemetry/call_tracer.{h,cc} : Use the `CallTracer` class to trace the lifecycle of a gRPC call in call_tracer.h / call_tracer.cc
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:35:32.707Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:32.707Z
Learning: Applies to deps/grpc/src/core/util/**/*.{cc,h,cpp} : Use `RefCounted` class from `ref_counted.h` as a base class for reference-counted objects
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:35:32.707Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:32.707Z
Learning: Applies to deps/grpc/src/core/util/**/*.{cc,h,cpp} : Use `ref_counted_ptr` smart pointer from `ref_counted_ptr.h` for managing `RefCounted` objects
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:34:55.521Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/telemetry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:55.521Z
Learning: Applies to deps/grpc/src/core/telemetry/**/telemetry/metrics.{h,cc} : Use the `Metrics` class to collect and report metrics in metrics.h / metrics.cc
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:34:52.246Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/telemetry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:52.246Z
Learning: Applies to deps/grpc/src/core/telemetry/**/call_tracer.{h,cc} : Implement CallTracer class to trace the lifecycle of gRPC calls in call_tracer.h/call_tracer.cc
Applied to files:
agents/grpc/src/grpc_agent.cc
🧬 Code graph analysis (5)
src/nsolid/nsolid_trace.cc (1)
src/nsolid/nsolid_util.h (1)
current_timestamp_ns(132-140)
agents/zmq/src/zmq_agent.h (2)
agents/grpc/src/grpc_agent.cc (2)
create_recorded(81-85)create_recorded(81-81)agents/zmq/src/zmq_agent.cc (2)
create_recorded(1520-1524)create_recorded(1520-1520)
src/nsolid.cc (1)
src/nsolid/nsolid_util.h (1)
current_timestamp_ns(132-140)
src/nsolid/nsolid_api.cc (2)
src/nsolid/nsolid_util.h (1)
current_timestamp_ns(132-140)lib/internal/perf/usertiming.js (1)
ts(63-63)
agents/grpc/src/grpc_agent.cc (2)
src/nsolid/nsolid_util.h (1)
current_timestamp_ns(132-140)agents/zmq/src/zmq_agent.cc (2)
create_recorded(1520-1524)create_recorded(1520-1520)
🪛 Cppcheck (2.18.0)
src/nsolid/nsolid_trace.cc
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
agents/zmq/src/zmq_agent.cc
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
src/nsolid.cc
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
src/nsolid/nsolid_api.cc
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
agents/grpc/src/grpc_agent.cc
[information] 82-82: Include file
(missingIncludeSystem)
[information] 83-83: Include file
(missingIncludeSystem)
[information] 84-84: Include file
(missingIncludeSystem)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
⏰ 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: build-tarball
- GitHub Check: test-linux (ubuntu-24.04)
- GitHub Check: test-linux (ubuntu-24.04-arm)
- GitHub Check: lint-js-and-md
- GitHub Check: coverage-linux
- GitHub Check: coverage-windows
- GitHub Check: coverage-linux-without-intl
🔇 Additional comments (8)
src/nsolid/nsolid_trace.cc (1)
202-219: Monotonic end timestamps remain in the same epoch-ms domain as span startsSwitching both
endPendingSpans()andexpired_span_cb_()toutils::current_timestamp_ns() / 1e6keepsstor_.endas milliseconds since the same epoch asstor_.start(which usesperformance_process_start_timestamp / 1e3 + offset_ms), while gaining the monotonicity and precision ofuv_hrtime(). Looks consistent and safe.Also applies to: 281-291
src/nsolid/nsolid_api.cc (1)
2228-2239: Log timestamps switched to current_timestamp_ns for consistency with other telemetry
WriteLognow usesutils::current_timestamp_ns()directly, which preserves the nanosecond-since-epoch representation previously derived fromsystem_clockwhile aligning logs with the same monotonic-backed time source used for metrics and traces. This should be a drop-in replacement for downstream log processing.agents/zmq/src/zmq_agent.cc (4)
1080-1080: LGTM: Call site updated correctly.The call to
create_recorded()has been updated to the parameterless version, consistent with the new internal timestamp generation approach.
1520-1524: LGTM: Correct implementation of nanosecond timestamp splitting.The implementation correctly converts the nanosecond timestamp from
current_timestamp_ns()into separate seconds and nanoseconds components using integer division and modulo operations. The const qualifier is appropriate.
1571-1571: LGTM: Call site updated correctly.Consistent with the parameterless
create_recorded()API change.
1612-1612: LGTM: Call site updated correctly.Consistent with the parameterless
create_recorded()API change.src/nsolid/nsolid_util.h (2)
19-19: LGTM: Required include for performance metrics.The
node_perf_common.hinclude is necessary to access the performance timestamp globals used incurrent_timestamp_ns().
132-140: The offset calculation is correct and safe.The implementation properly converts
performance_process_start_timestamp(in microseconds) to nanoseconds by multiplying by 1000, matching the units ofperformance_process_start(returned byuv_hrtime()in nanoseconds). The resulting offset—capturing the difference between epoch time and monotonic time at process start—is mathematically sound and avoids overflow, as epoch time in nanoseconds (~1.7×10¹⁸) is well withinuint64_trange and much larger than the small hrtime value at process start. Precision loss from the double-to-uint64_t cast is negligible for timestamp values and consistent with Node.js practices.
santigimeno
force-pushed
the
santi/add_current_timestamp_ns
branch
2 times, most recently
from
f9b9495 to
3505afa
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/nsolid/nsolid_util.h (1)
19-20: Central monotonic timestamp helpers look correct; consider documenting assumptionsThe new
current_timestamp_ns()/current_timestamp_ms()helpers correctly derive an epoch-based monotonic clock fromperformance::performance_process_start_timestampandperformance::performance_process_start, with a one‑time computed offset and cheapuv_hrtime()on each call. Numeric ranges and precision are safe for realistic runtimes.Given these are now the canonical time source, it would be helpful to document (in a brief comment here or in
node_perf_common.h) that:
performance_process_start_timestampis “epoch microseconds at process start”.performance_process_startis “uv_hrtime()at the same instant”.- Both are immutable after process start, so the cached offset remains valid.
This makes the invariants explicit for future maintainers touching perf/time code.
Also applies to: 132-143
agents/grpc/src/grpc_agent.cc (3)
81-85: create_recorded() correctly switches to monotonic ns timestamp, mirrors ZMQ implementationThe new helper cleanly wraps
utils::current_timestamp_ns()and produces(seconds, nanoseconds)via integer division/modulo, matching the ZMQ agent’screate_recorded()semantics and keeping timestamp math consistent and monotonic across agents.If you want to DRY this up a bit, you could:
- Introduce a shared
constexpr uint64_t kNanosPerSecond = 1000000000ULL;, and/or- Factor the
ns → (sec, nsec)split into a tiny common utility used by both gRPC and ZMQ agents.These are purely stylistic/maintainability tweaks; the current logic is correct.
118-131: PopulateCommon now derives timestamps internally; semantics look goodSwitching
PopulateCommonto call the parameterlesscreate_recorded()aligns this path with the new monotonic timestamp source and removes duplicatedsystem_clockusage. This keeps all gRPC “recorded” times on the same monotonic, ns-resolution clock as the rest of the observability pipeline.As a small cleanup,
using std::chrono::system_clock;andusing std::chrono::time_point;near the top of the file appear unused after this change and could be dropped to avoid confusion about wall-clock dependencies here.
1490-1563: Consider reusing current_timestamp_ns() math in got_continuous_profile for consistency
got_continuous_profile()still computesstart_ts,end_ts, anddurationby directly combininguv_hrtime()withperformance::performance_process_start_timestamp/performance::performance_process_start. With the introduction ofutils::current_timestamp_ns(), this function now has its own parallel epoch/monotonic conversion logic.To keep timestamp behavior uniform and reduce the chance of subtle drift if the process-start timing math ever changes, consider:
- Centralizing the “epoch-from-hrtime” calculations in
nsolid_util.h(e.g., helpers to get start/end as seconds or doubles fromcurrent_timestamp_ns()), and- Refactoring
got_continuous_profile()to use those helpers instead of re-implementing the offset math locally.Functionally this is fine today; this is about maintainability and keeping all observability timestamps on a single, shared code path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
agents/grpc/src/grpc_agent.cc(2 hunks)agents/zmq/src/zmq_agent.cc(4 hunks)agents/zmq/src/zmq_agent.h(1 hunks)src/nsolid.cc(1 hunks)src/nsolid/nsolid_api.cc(3 hunks)src/nsolid/nsolid_trace.cc(2 hunks)src/nsolid/nsolid_util.h(2 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:32.707Z
Learning: Applies to deps/grpc/src/core/util/**/*.{cc,h,cpp} : Use time-related functions from `time.h` and `time.cc`
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/telemetry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:55.521Z
Learning: Applies to deps/grpc/src/core/telemetry/**/telemetry/metrics.{h,cc} : Use the `Metrics` class to collect and report metrics in metrics.h / metrics.cc
📚 Learning: 2025-12-03T14:35:32.707Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:32.707Z
Learning: Applies to deps/grpc/src/core/util/**/*.{cc,h,cpp} : Use time-related functions from `time.h` and `time.cc`
Applied to files:
src/nsolid/nsolid_trace.ccsrc/nsolid/nsolid_util.hsrc/nsolid/nsolid_api.ccagents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:35:35.065Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:35.065Z
Learning: Applies to deps/grpc/src/core/util/src/core/util/time.{h,cc} : Time-related functions should be implemented in dedicated header and implementation files (time.h / time.cc)
Applied to files:
src/nsolid/nsolid_util.hagents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:35:38.574Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/json/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:38.574Z
Learning: Maintain C++-native JSON library implementation with no external dependencies
Applied to files:
src/nsolid/nsolid_util.h
📚 Learning: 2025-12-03T14:40:17.411Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/json/GEMINI.md:0-0
Timestamp: 2025-12-03T14:40:17.411Z
Learning: Applies to deps/grpc/src/core/util/json/**/json*.{cc,cpp,h,hpp} : Design JSON utilities to be efficient and avoid unnecessary memory allocations
Applied to files:
src/nsolid/nsolid_util.h
📚 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/**/time.{h,cc} : Time-related functions should be implemented in `time.h` and `time.cc`
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:35:32.707Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:32.707Z
Learning: Applies to deps/grpc/src/core/util/**/*.{cc,h,cpp} : Use `ref_counted_ptr` smart pointer from `ref_counted_ptr.h` for managing `RefCounted` objects
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:35:32.707Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:32.707Z
Learning: Applies to deps/grpc/src/core/util/**/*.{cc,h,cpp} : Use `RefCounted` class from `ref_counted.h` as a base class for reference-counted objects
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:34:55.521Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/telemetry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:55.521Z
Learning: Applies to deps/grpc/src/core/telemetry/**/telemetry/call_tracer.{h,cc} : Use the `CallTracer` class to trace the lifecycle of a gRPC call in call_tracer.h / call_tracer.cc
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:31:12.056Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/ext/transport/inproc/GEMINI.md:0-0
Timestamp: 2025-12-03T14:31:12.056Z
Learning: Applies to deps/grpc/src/core/ext/transport/inproc/**/inproc_transport.{h,cc} : Use `grpc_core::MakeInProcessTransportPair` as the main entry point for creating connected in-process transport pairs in the modern implementation
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:34:55.521Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/telemetry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:55.521Z
Learning: Applies to deps/grpc/src/core/telemetry/**/telemetry/metrics.{h,cc} : Use the `Metrics` class to collect and report metrics in metrics.h / metrics.cc
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:35:32.707Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:32.707Z
Learning: Applies to deps/grpc/src/core/util/**/*.{cc,h,cpp} : Use synchronization primitives from `sync.h` and `sync.cc`
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:34:52.246Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/telemetry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:52.246Z
Learning: Applies to deps/grpc/src/core/telemetry/**/call_tracer.{h,cc} : Implement CallTracer class to trace the lifecycle of gRPC calls in call_tracer.h/call_tracer.cc
Applied to files:
agents/grpc/src/grpc_agent.cc
🧬 Code graph analysis (6)
src/nsolid/nsolid_trace.cc (1)
src/nsolid/nsolid_util.h (1)
current_timestamp_ms(141-143)
agents/zmq/src/zmq_agent.cc (2)
agents/grpc/src/grpc_agent.cc (2)
create_recorded(81-85)create_recorded(81-81)src/nsolid/nsolid_util.h (1)
current_timestamp_ns(132-139)
src/nsolid/nsolid_api.cc (1)
src/nsolid/nsolid_util.h (2)
current_timestamp_ms(141-143)current_timestamp_ns(132-139)
agents/zmq/src/zmq_agent.h (2)
agents/grpc/src/grpc_agent.cc (2)
create_recorded(81-85)create_recorded(81-81)agents/zmq/src/zmq_agent.cc (2)
create_recorded(1520-1524)create_recorded(1520-1520)
agents/grpc/src/grpc_agent.cc (2)
src/nsolid/nsolid_util.h (1)
current_timestamp_ns(132-139)agents/zmq/src/zmq_agent.cc (2)
create_recorded(1520-1524)create_recorded(1520-1520)
src/nsolid.cc (1)
src/nsolid/nsolid_util.h (1)
current_timestamp_ms(141-143)
🪛 Cppcheck (2.18.0)
src/nsolid/nsolid_trace.cc
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
agents/zmq/src/zmq_agent.cc
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
src/nsolid/nsolid_api.cc
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
agents/grpc/src/grpc_agent.cc
[information] 82-82: Include file
(missingIncludeSystem)
[information] 83-83: Include file
(missingIncludeSystem)
[information] 84-84: Include file
(missingIncludeSystem)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
src/nsolid.cc
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
⏰ 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: coverage-linux
- GitHub Check: build-tarball
- GitHub Check: coverage-linux-without-intl
- GitHub Check: lint-js-and-md
- GitHub Check: coverage-windows
🔇 Additional comments (10)
agents/zmq/src/zmq_agent.cc (3)
1520-1523: create_recorded() correctly centralizes ZMQ timestamps on the new monotonic clockImplementation matches the grpc helper and cleanly converts
current_timestamp_ns()into(seconds, nanoseconds)for therecordedJSON field. Using the shared monotonic epoch helper keeps ZMQ timestamps consistent with the rest of the system and avoids repeated time math at call sites.
1076-1111: Using create_recorded() here keeps command timestamps consistent and cheapSwitching
send_command_message()to computerecordedvia the internalcreate_recorded()aligns command messages with the new monotonic clock and reduces per-call work to a single helper call. The recursive retry on buffer resize still behaves correctly; the only effect is a negligible timestamp shift between attempts.
1569-1624: Error paths now share the same timestamp source as normal command messagesBoth
send_error_message()(Line 1571) andsend_error_command_message()(Line 1613) now callcreate_recorded(), so success and error envelopes share a consistent, monotonic epoch time representation. Reusing the samerecordedacross the initial and resizedsnprintfcalls also preserves a single logical timestamp per message.agents/zmq/src/zmq_agent.h (1)
535-538: Header declaration matches the new internal timestamp helperUpdating
create_recorded()to a zero-argument method keeps the public interface in sync with the implementation and simplifies callers by hiding timestamp acquisition details. No issues spotted.src/nsolid.cc (1)
250-257: Process metrics timestamp now uses centralized monotonic millisecondsAssigning
stor_.timestampfromutils::current_timestamp_ms()aligns process metrics with the new clock source and with thread metrics/datapoints, without changing the JSON schema (still “milliseconds since epoch” as a double). Looks good.src/nsolid/nsolid_trace.cc (2)
202-209: Span end timestamps switched to the shared monotonic ms helperUsing
utils::current_timestamp_ms()forendPendingSpans()(Line 204) makes synthesized end times consistent with spanstart(which is already epoch-ms based) and with other components using the same helper. This also removes direct dependence on older wall-clock helpers.
281-288: Expired span callback now uses the same epoch/ms source as other spansSetting
span.stor_.endviautils::current_timestamp_ms()keeps expired spans on the same time base as normal spans and metrics, improving ordering guarantees without extra per-call overhead.src/nsolid/nsolid_api.cc (3)
280-298: Thread metrics timestamp now shares the same monotonic ms source
EnvInst::GetThreadMetrics()populatesstor->timestampfromutils::current_timestamp_ms(), which matches the new process-level timestamping and avoids scattering system_clock math. This should improve consistency across thread and process metrics streams.
1430-1437: Datapoint timestamps now come from current_timestamp_ms()Using
utils::current_timestamp_ms()inEnvInst::send_datapoint()means HTTP/DNS datapoints carry the same epoch-ms representation as other metrics, while only incurring the timestamp cost when there are active metrics stream hooks. This fits the PR’s centralization goal.
2227-2239: LogWriteInfo now stores timestamps in nanoseconds from the shared helperSwitching the log timestamp field to
utils::current_timestamp_ns()gives nanosecond-resolution, monotonic, epoch-based timestamps for log lines, and reuses the same offset logic as other components. This should make correlating logs with traces/metrics easier.
santigimeno
force-pushed
the
santi/add_current_timestamp_ns
branch
from
3505afa to
466954b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
agents/zmq/src/zmq_agent.cc (1)
1078-1092: Zero‑argcreate_recorded()correctly switches ZMQ “recorded” times to the monotonic clockThe new
ZmqAgent::create_recorded()usesutils::current_timestamp_ns()and cleanly maps it to the{seconds, nanoseconds}pair expected by MSG_1/2/4/5, and all call sites now use the zero‑arg helper. This preserves the JSON protocol while movingrecordedto the same monotonic, epoch‑anchored source used elsewhere.If you touch this again, consider sharing this tiny helper between the ZMQ and gRPC agents to avoid duplicating the seconds/nanos split logic, but it’s not urgent.
Also applies to: 1518-1522, 1569-1579, 1610-1622
agents/grpc/src/grpc_agent.cc (1)
79-83: Monotoniccreate_recorded()and continuous‑profile start/end timestamps are consistentThe new free
create_recorded()helper correctly derives(seconds, nanoseconds)fromutils::current_timestamp_ns()andPopulateCommonnow uses it so that all gRPC events share the same monotonic, epoch‑anchored recorded time source as the rest of the system.In
got_continuous_profile, computingdurationfromuv_hrtime()(via the storedstart_timestamp) and then settingstart_ts = end_ts - durationwithend_tsfromutils::current_timestamp_ms()keeps the reported start/end times self‑consistent and robust against wall‑clock adjustments, while preserving millisecond granularity expected by the proto fields.Also applies to: 116-129, 1537-1550
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
agents/grpc/src/grpc_agent.cc(3 hunks)agents/zmq/src/zmq_agent.cc(4 hunks)agents/zmq/src/zmq_agent.h(1 hunks)src/nsolid.cc(1 hunks)src/nsolid/nsolid_api.cc(3 hunks)src/nsolid/nsolid_trace.cc(2 hunks)src/nsolid/nsolid_util.h(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/nsolid.cc
- agents/zmq/src/zmq_agent.h
- src/nsolid/nsolid_util.h
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:32.707Z
Learning: Applies to deps/grpc/src/core/util/**/*.{cc,h,cpp} : Use time-related functions from `time.h` and `time.cc`
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/telemetry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:55.521Z
Learning: Applies to deps/grpc/src/core/telemetry/**/telemetry/metrics.{h,cc} : Use the `Metrics` class to collect and report metrics in metrics.h / metrics.cc
📚 Learning: 2025-12-03T14:35:32.707Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:32.707Z
Learning: Applies to deps/grpc/src/core/util/**/*.{cc,h,cpp} : Use time-related functions from `time.h` and `time.cc`
Applied to files:
src/nsolid/nsolid_api.ccagents/grpc/src/grpc_agent.ccsrc/nsolid/nsolid_trace.cc
📚 Learning: 2025-12-03T14:35:35.065Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:35.065Z
Learning: Applies to deps/grpc/src/core/util/src/core/util/time.{h,cc} : Time-related functions should be implemented in dedicated header and implementation files (time.h / time.cc)
Applied to files:
src/nsolid/nsolid_api.ccagents/grpc/src/grpc_agent.ccsrc/nsolid/nsolid_trace.cc
📚 Learning: 2025-12-03T14:34:55.521Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/telemetry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:55.521Z
Learning: Applies to deps/grpc/src/core/telemetry/**/telemetry/metrics.{h,cc} : Use the `Metrics` class to collect and report metrics in metrics.h / metrics.cc
Applied to files:
src/nsolid/nsolid_api.ccagents/grpc/src/grpc_agent.cc
📚 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/**/time.{h,cc} : Time-related functions should be implemented in `time.h` and `time.cc`
Applied to files:
src/nsolid/nsolid_api.ccagents/grpc/src/grpc_agent.ccsrc/nsolid/nsolid_trace.cc
📚 Learning: 2025-12-03T14:35:38.574Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/json/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:38.574Z
Learning: Maintain C++-native JSON library implementation with no external dependencies
Applied to files:
agents/zmq/src/zmq_agent.cc
📚 Learning: 2025-12-03T14:35:32.707Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:32.707Z
Learning: Applies to deps/grpc/src/core/util/**/*.{cc,h,cpp} : Use synchronization primitives from `sync.h` and `sync.cc`
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:35:32.707Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:32.707Z
Learning: Applies to deps/grpc/src/core/util/**/*.{cc,h,cpp} : Use `ref_counted_ptr` smart pointer from `ref_counted_ptr.h` for managing `RefCounted` objects
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:34:55.521Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/telemetry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:55.521Z
Learning: Applies to deps/grpc/src/core/telemetry/**/telemetry/call_tracer.{h,cc} : Use the `CallTracer` class to trace the lifecycle of a gRPC call in call_tracer.h / call_tracer.cc
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:35:32.707Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:32.707Z
Learning: Applies to deps/grpc/src/core/util/**/*.{cc,h,cpp} : Use `RefCounted` class from `ref_counted.h` as a base class for reference-counted objects
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:32:58.550Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/lib/promise/GEMINI.md:0-0
Timestamp: 2025-12-03T14:32:58.550Z
Learning: Applies to deps/grpc/src/core/lib/promise/**/*.{cc,cpp} : Use `ArenaPromise<T>` instead of regular `Promise<T>` in performance-sensitive code to avoid heap allocations
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:34:52.246Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/telemetry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:52.246Z
Learning: Applies to deps/grpc/src/core/telemetry/**/metrics.{h,cc} : Implement Metrics class to collect and report metrics in metrics.h/metrics.cc
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:34:47.986Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/telemetry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:47.986Z
Learning: Applies to deps/grpc/src/core/telemetry/**/metrics.{h,cc} : The `Metrics` class should be used to collect and report metrics
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:40:17.411Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/json/GEMINI.md:0-0
Timestamp: 2025-12-03T14:40:17.411Z
Learning: Applies to deps/grpc/src/core/util/json/**/*.{cc,cpp,h,hpp} : Use JsonObjectLoader for converting JSON data into C++ objects in a safe and structured way
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:30:16.146Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/ext/filters/load_reporting/GEMINI.md:0-0
Timestamp: 2025-12-03T14:30:16.146Z
Learning: Applies to deps/grpc/src/core/ext/filters/load_reporting/**/registered_opencensus_objects.h : Define OpenCensus measures and tags used by the server load reporting filter in the registered_opencensus_objects.h file
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:40:17.411Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/json/GEMINI.md:0-0
Timestamp: 2025-12-03T14:40:17.411Z
Learning: Applies to deps/grpc/src/core/util/json/**/*.{cc,cpp,h,hpp} : Use the C++-native JSON library (json.h, json_reader, json_writer) for parsing and generating JSON data in gRPC core
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:40:17.411Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/json/GEMINI.md:0-0
Timestamp: 2025-12-03T14:40:17.411Z
Learning: Applies to deps/grpc/src/core/util/json/**/json*.{cc,cpp,h,hpp} : Design JSON utilities to be efficient and avoid unnecessary memory allocations
Applied to files:
agents/grpc/src/grpc_agent.cc
🧬 Code graph analysis (4)
src/nsolid/nsolid_api.cc (1)
src/nsolid/nsolid_util.h (2)
current_timestamp_ms(141-143)current_timestamp_ns(132-139)
agents/zmq/src/zmq_agent.cc (2)
agents/grpc/src/grpc_agent.cc (2)
create_recorded(79-83)create_recorded(79-79)src/nsolid/nsolid_util.h (1)
current_timestamp_ns(132-139)
agents/grpc/src/grpc_agent.cc (2)
src/nsolid/nsolid_util.h (2)
current_timestamp_ns(132-139)current_timestamp_ms(141-143)agents/zmq/src/zmq_agent.cc (2)
create_recorded(1518-1522)create_recorded(1518-1518)
src/nsolid/nsolid_trace.cc (1)
src/nsolid/nsolid_util.h (1)
current_timestamp_ms(141-143)
⏰ 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: build-tarball
- GitHub Check: test-linux (ubuntu-24.04-arm)
- GitHub Check: test-linux (ubuntu-24.04)
- GitHub Check: coverage-linux
- GitHub Check: coverage-linux-without-intl
- GitHub Check: coverage-windows
- GitHub Check: lint-js-and-md
🔇 Additional comments (2)
src/nsolid/nsolid_trace.cc (1)
204-219: Monotonic span end timestamps viautils::current_timestamp_ms()look correctUsing
utils::current_timestamp_ms()in bothendPendingSpans()andexpired_span_cb_()keeps span end times in the same milliseconds‑since‑epoch space as the JS‑driven start times while switching to a monotonic, uv_hrtime‑based source. Units and types align, and this matches the central timestamp utility introduced elsewhere in the PR.Also applies to: 281-291
src/nsolid/nsolid_api.cc (1)
280-288: Centralizing metrics/log timestamps onutils::current_timestamp_*is consistent and sound
EnvInst::GetThreadMetrics,EnvInst::send_datapoint, andWriteLognow all source their timestamps from the shared monotonic helpers (utils::current_timestamp_ms()/utils::current_timestamp_ns()), keeping units (ms for metrics, ns for logs) clear and making ordering robust against wall‑clock jumps. This aligns with the new timestamp utilities without introducing unit or type issues in these call sites.Also applies to: 1430-1438, 2227-2239
Replace scattered wall-clock timestamp calls (system_clock::now(), GetCurrentTimeInMicroseconds(), ms_since_epoch()) with a couple of current_timestamp_ns(), current_timestamp_ms() utilities that anchor to epoch time at process start and uses uv_hrtime() for elapsed time. Benefits: - Monotonicity: timestamps never jump backward due to NTP adjustments, VM migrations, or manual clock changes - Consistency: all metrics, traces, and agent protocol messages use the same time source - Precision: nanosecond resolution from uv_hrtime() - Performance: uv_hrtime() uses vDSO-accelerated clock_gettime() on Linux, avoiding syscall overhead on each timestamp The approach trusts the system clock once at startup, then relies on the monotonic high-resolution timer. This is preferable for observability data where ordering and rate calculations matter more than tracking real-time clock drift. Refactored current_timestamp_ns() to cache the offset between epoch and hrtime at startup, reducing per-call overhead to a single addition.
santigimeno
force-pushed
the
santi/add_current_timestamp_ns
branch
from
466954b to
1904f2a
Compare
santigimeno
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
agents/grpc/src/grpc_agent.cc(3 hunks)agents/zmq/src/zmq_agent.cc(4 hunks)agents/zmq/src/zmq_agent.h(1 hunks)src/nsolid.cc(1 hunks)src/nsolid/nsolid_api.cc(3 hunks)src/nsolid/nsolid_trace.cc(2 hunks)src/nsolid/nsolid_util.h(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/nsolid.cc
- src/nsolid/nsolid_api.cc
- agents/zmq/src/zmq_agent.cc
- agents/zmq/src/zmq_agent.h
- src/nsolid/nsolid_trace.cc
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:32.707Z
Learning: Applies to deps/grpc/src/core/util/**/*.{cc,h,cpp} : Use time-related functions from `time.h` and `time.cc`
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/telemetry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:55.521Z
Learning: Applies to deps/grpc/src/core/telemetry/**/telemetry/metrics.{h,cc} : Use the `Metrics` class to collect and report metrics in metrics.h / metrics.cc
📚 Learning: 2025-12-03T14:35:32.707Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:32.707Z
Learning: Applies to deps/grpc/src/core/util/**/*.{cc,h,cpp} : Use time-related functions from `time.h` and `time.cc`
Applied to files:
agents/grpc/src/grpc_agent.ccsrc/nsolid/nsolid_util.h
📚 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/**/time.{h,cc} : Time-related functions should be implemented in `time.h` and `time.cc`
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:35:35.065Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:35.065Z
Learning: Applies to deps/grpc/src/core/util/src/core/util/time.{h,cc} : Time-related functions should be implemented in dedicated header and implementation files (time.h / time.cc)
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:35:32.707Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:32.707Z
Learning: Applies to deps/grpc/src/core/util/**/*.{cc,h,cpp} : Use synchronization primitives from `sync.h` and `sync.cc`
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:34:55.521Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/telemetry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:55.521Z
Learning: Applies to deps/grpc/src/core/telemetry/**/telemetry/metrics.{h,cc} : Use the `Metrics` class to collect and report metrics in metrics.h / metrics.cc
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:35:32.707Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:32.707Z
Learning: Applies to deps/grpc/src/core/util/**/*.{cc,h,cpp} : Use `ref_counted_ptr` smart pointer from `ref_counted_ptr.h` for managing `RefCounted` objects
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:35:32.707Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:32.707Z
Learning: Applies to deps/grpc/src/core/util/**/*.{cc,h,cpp} : Use `RefCounted` class from `ref_counted.h` as a base class for reference-counted objects
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:34:55.521Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/telemetry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:55.521Z
Learning: Applies to deps/grpc/src/core/telemetry/**/telemetry/call_tracer.{h,cc} : Use the `CallTracer` class to trace the lifecycle of a gRPC call in call_tracer.h / call_tracer.cc
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:34:55.521Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/telemetry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:55.521Z
Learning: Applies to deps/grpc/src/core/telemetry/**/telemetry/stats.{h,cc} : Use the `Stats` class to collect and report statistics in stats.h / stats.cc
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:34:52.246Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/telemetry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:52.246Z
Learning: Applies to deps/grpc/src/core/telemetry/**/metrics.{h,cc} : Implement Metrics class to collect and report metrics in metrics.h/metrics.cc
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:34:47.986Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/telemetry/GEMINI.md:0-0
Timestamp: 2025-12-03T14:34:47.986Z
Learning: Applies to deps/grpc/src/core/telemetry/**/metrics.{h,cc} : The `Metrics` class should be used to collect and report metrics
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:40:17.411Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/json/GEMINI.md:0-0
Timestamp: 2025-12-03T14:40:17.411Z
Learning: Applies to deps/grpc/src/core/util/json/**/*.{cc,cpp,h,hpp} : Use JsonObjectLoader for converting JSON data into C++ objects in a safe and structured way
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:30:16.146Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/ext/filters/load_reporting/GEMINI.md:0-0
Timestamp: 2025-12-03T14:30:16.146Z
Learning: Applies to deps/grpc/src/core/ext/filters/load_reporting/**/registered_opencensus_objects.h : Define OpenCensus measures and tags used by the server load reporting filter in the registered_opencensus_objects.h file
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:40:17.411Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/json/GEMINI.md:0-0
Timestamp: 2025-12-03T14:40:17.411Z
Learning: Applies to deps/grpc/src/core/util/json/**/*.{cc,cpp,h,hpp} : Use the C++-native JSON library (json.h, json_reader, json_writer) for parsing and generating JSON data in gRPC core
Applied to files:
agents/grpc/src/grpc_agent.cc
📚 Learning: 2025-12-03T14:40:17.411Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/json/GEMINI.md:0-0
Timestamp: 2025-12-03T14:40:17.411Z
Learning: Applies to deps/grpc/src/core/util/json/**/json*.{cc,cpp,h,hpp} : Design JSON utilities to be efficient and avoid unnecessary memory allocations
Applied to files:
agents/grpc/src/grpc_agent.ccsrc/nsolid/nsolid_util.h
📚 Learning: 2025-12-03T14:35:38.574Z
Learnt from: CR
Repo: nodesource/nsolid PR: 0
File: deps/grpc/src/core/util/json/GEMINI.md:0-0
Timestamp: 2025-12-03T14:35:38.574Z
Learning: Maintain C++-native JSON library implementation with no external dependencies
Applied to files:
src/nsolid/nsolid_util.h
🧬 Code graph analysis (1)
agents/grpc/src/grpc_agent.cc (2)
src/nsolid/nsolid_util.h (2)
current_timestamp_ns(132-139)current_timestamp_ms(141-143)agents/zmq/src/zmq_agent.cc (2)
create_recorded(1518-1522)create_recorded(1518-1518)
⏰ 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: build-tarball
- GitHub Check: coverage-windows
- GitHub Check: coverage-linux-without-intl
- GitHub Check: test-macOS
- GitHub Check: test-linux (ubuntu-24.04-arm)
- GitHub Check: test-linux (ubuntu-24.04)
- GitHub Check: lint-js-and-md
- GitHub Check: coverage-linux
🔇 Additional comments (4)
src/nsolid/nsolid_util.h (2)
19-19: Include ofnode_perf_common.his appropriate and requiredThis header is the right place to source
performance::performance_process_start_timestampandperformance::performance_process_startused by the new timestamp helpers; no issues here.
132-143:current_timestamp_ns()/current_timestamp_ms()implementation is correct and ready to mergeThe offset calculation is mathematically sound:
performance_process_start_timestampis in microseconds since Unix epoch andperformance_process_startis in nanoseconds fromuv_hrtime(). The conversion to nanoseconds and subsequent addition withuv_hrtime()yields a properly anchored, monotonic timestamp as intended.agents/grpc/src/grpc_agent.cc (2)
116-123: Populatingrecordedvia internalcreate_recorded()is consistent and simplifies call sites
PopulateCommonnow always uses the internalcreate_recorded()to setrecorded.secondsandrecorded.nanoseconds, removing the need for callers to supply asystem_clock::now()and ensuring this path uses the same monotonic, process-anchored time source everywhere.
79-83:create_recorded()correctly reuses the shared monotonic ns timestamp helperUsing
utils::current_timestamp_ns()and splitting via/ 1000000000and% 1000000000gives a proper(seconds, nanoseconds)pair consistent with the ZMQ agent implementation and protobufTimestampconstraints. The function is used inPopulateCommon()where bothset_seconds()andset_nanoseconds()are properly called with the split values, centralizing the time source without changing semantics for callers.
Replace scattered wall-clock timestamp calls (system_clock::now(), GetCurrentTimeInMicroseconds(), ms_since_epoch()) with a single current_timestamp_ns() utility that anchors to epoch time at process start and uses uv_hrtime() for elapsed time.
Benefits:
The approach trusts the system clock once at startup, then relies on the monotonic high-resolution timer. This is preferable for observability data where ordering and rate calculations matter more than tracking real-time clock drift.
Refactored current_timestamp_ns() to cache the offset between epoch and hrtime at startup, reducing per-call overhead to a single addition.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.