Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

uftrace: add support for UTC offset information #26

Merged
merged 1 commit into from
May 28, 2024
Merged

uftrace: add support for UTC offset information #26

merged 1 commit into from
May 28, 2024

Conversation

MatthewKhouzam
Copy link
Contributor

@MatthewKhouzam MatthewKhouzam commented May 10, 2024 •
edited

This is a soon to be released feature in uftrace, allows correlation with lttng kernel traces.

This fixes all traces being from boot time and helps with correlation.
image

Fixes namhyung/uftrace#1923

@honggyukim
Copy link

Hi @MatthewKhouzam, namhyung/uftrace#1923 is now on master so you can use it.

bhufmann
bhufmann requested changes May 27, 2024
View reviewed changes
Copy link
Contributor

@bhufmann bhufmann left a comment

Choose a reason for hiding this comment

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

Code looks good. Just some formatting issues. I'm not able to test it though.

...uftrace.core/src/org/eclipse/tracecompass/incubator/internal/uftrace/core/trace/Uftrace.java Outdated Show resolved Hide resolved
...uftrace.core/src/org/eclipse/tracecompass/incubator/internal/uftrace/core/trace/Uftrace.java Outdated Show resolved Hide resolved
...uftrace.core/src/org/eclipse/tracecompass/incubator/internal/uftrace/core/trace/Uftrace.java Outdated Show resolved Hide resolved
@MatthewKhouzam MatthewKhouzam force-pushed the uftrace-utc branch 2 times, most recently from 5163224 to fe7bc9a Compare May 27, 2024 19:45
@honggyukim
Copy link

I didn't read the code carefully but please make sure to handle backward compatibiliy since the previous uftrace data doesn't have utc_offset field in its info.

bhufmann
bhufmann approved these changes May 28, 2024
View reviewed changes
Copy link
Contributor

@bhufmann bhufmann left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@bhufmann
Copy link
Contributor

Incubator build is still broken. It needs the PR #28 to be merged first and then a rebase.

@MatthewKhouzam
Copy link
Contributor Author

I didn't read the code carefully but please make sure to handle backward compatibiliy since the previous uftrace data doesn't have utc_offset field in its info.

I should have replied earlier.

It assumes UTC Offset is 0. If there is a field named UTC Offset and it is NOT "0" it tries to parse the number. If it succeeds in parsing, it assigns the offset to the non-zero value.

@MatthewKhouzam
uftrace: add support for UTC offset information
672e263 
This is a soon to be released feature in uftrace, allows correlation with
lttng kernel traces.

Signed-off-by: Matthew Khouzam <matthew.khouzam@ericsson.com>
Change-Id: I76ee112ba5c407d7e4810df77882c9e11bfaa43d
@MatthewKhouzam
Copy link
Contributor Author

Simple rebase

@MatthewKhouzam MatthewKhouzam merged commit f230c18 into eclipse-tracecompass-incubator:master May 28, 2024
2 checks passed
@MatthewKhouzam MatthewKhouzam deleted the uftrace-utc branch May 28, 2024 15:29
@honggyukim
Copy link

I didn't read the code carefully but please make sure to handle backward compatibiliy since the previous uftrace data doesn't have utc_offset field in its info.

I should have replied earlier.

It assumes UTC Offset is 0. If there is a field named UTC Offset and it is NOT "0" it tries to parse the number. If it succeeds in parsing, it assigns the offset to the non-zero value.

Sounds good. Thanks your work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bhufmann bhufmann bhufmann approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@MatthewKhouzam @honggyukim @bhufmann