Skip to content

Commit

Permalink
fix(profiling) sample timestamps need to be in seconds (#12563)
Browse files Browse the repository at this point in the history
  • Loading branch information
JonasBa authored Jun 19, 2024
1 parent 847c05a commit 424937f
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 3 deletions.
8 changes: 5 additions & 3 deletions packages/profiling-node/bindings/cpu_profiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,8 @@ CreateFrameNode(const napi_env &env, const v8::CpuProfileNode &node,
};

napi_value CreateSample(const napi_env &env, const enum ProfileFormat format,
const uint32_t stack_id, const int64_t sample_timestamp,
const uint32_t stack_id,
const int64_t sample_timestamp_ns,
const double chunk_timestamp,
const uint32_t thread_id) {
napi_value js_node;
Expand All @@ -564,7 +565,7 @@ napi_value CreateSample(const napi_env &env, const enum ProfileFormat format,
switch (format) {
case ProfileFormat::kFormatThread: {
napi_value timestamp;
napi_create_int64(env, sample_timestamp, &timestamp);
napi_create_int64(env, sample_timestamp_ns, &timestamp);
napi_set_named_property(env, js_node, "elapsed_since_start_ns", timestamp);
} break;
case ProfileFormat::kFormatChunk: {
Expand Down Expand Up @@ -643,7 +644,8 @@ static void GetSamples(const napi_env &env, const v8::CpuProfile *profile,
uint64_t sample_offset_from_profile_start_ms =
(sample_timestamp_us - profile_start_time_us) * 1e-3;
double seconds_since_start =
profile_start_timestamp_ms + sample_offset_from_profile_start_ms;
(profile_start_timestamp_ms + sample_offset_from_profile_start_ms) *
1e-3;

napi_value sample = nullptr;
sample = CreateSample(env, format, stack_index, sample_timestamp_ns,
Expand Down
4 changes: 4 additions & 0 deletions packages/profiling-node/test/cpu_profiler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,10 @@ describe('Profiler bindings', () => {
throw new Error(`Sample ${JSON.stringify(sample)} has no timestamp`);
}
expect(sample.timestamp).toBeDefined();
// No older than a minute and not in the future. Timestamp is in seconds so convert to ms
// as the constructor expectes ms.
expect(new Date((sample.timestamp as number) * 1e3).getTime()).toBeGreaterThan(Date.now() - 60 * 1e3);
expect(new Date((sample.timestamp as number) * 1e3).getTime()).toBeLessThanOrEqual(Date.now());
}
});

Expand Down

0 comments on commit 424937f

Please sign in to comment.