Skip to content

offcpu: Fix incorrect OffTimes values in offcpu profiling.#559

Merged
christos68k merged 1 commit intoopen-telemetry:mainfrom
tsint:offcpu
Jul 3, 2025
Merged

offcpu: Fix incorrect OffTimes values in offcpu profiling.#559
christos68k merged 1 commit intoopen-telemetry:mainfrom
tsint:offcpu

Conversation

@tsint
Copy link
Copy Markdown
Contributor

@tsint tsint commented Jun 27, 2025

In the original code, while sorting the samples by timestamp, the corresponding offcpu profiling values were not sorted accordingly, leading to incorrect samples mappings and overlapping offcpu time intervals.

@tsint tsint requested review from a team as code owners June 27, 2025 11:33
Comment thread reporter/internal/pdata/generate.go Outdated
Comment thread reporter/internal/pdata/generate.go Outdated
@tsint tsint force-pushed the offcpu branch 2 times, most recently from 7f89426 to 30aa4f9 Compare June 30, 2025 08:42
@florianl florianl self-requested a review June 30, 2025 08:44
@rockdaboot
Copy link
Copy Markdown
Contributor

The changes LGTM!

Can you possibly create a test function?
Something like

func TestFunctionTableOrder(t *testing.T) {

but stripped down, as it only needs to check for correct profile.Duration and profile.StartTime. Ideally have a test case with two samples and unsorted timestamp values.

@tsint
Copy link
Copy Markdown
Contributor Author

tsint commented Jun 30, 2025

The changes LGTM!

Can you possibly create a test function? Something like

func TestFunctionTableOrder(t *testing.T) {

but stripped down, as it only needs to check for correct profile.Duration and profile.StartTime. Ideally have a test case with two samples and unsorted timestamp values.

Okay, I'll look into it.

@tsint tsint force-pushed the offcpu branch 4 times, most recently from db18b92 to fb0482e Compare June 30, 2025 09:56
Comment thread reporter/internal/pdata/generate_test.go
Copy link
Copy Markdown
Contributor

@rockdaboot rockdaboot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for finding the issue and fixing it!

@christos68k christos68k merged commit dcd30e2 into open-telemetry:main Jul 3, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants