-
Notifications
You must be signed in to change notification settings - Fork 91
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.
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
Switch SpanEvent
times to use time.Time
#1101
Conversation
1c1a30d
to
3b14c98
Compare
Use `time.Time`, a timestamp, to identify the start and end time of the `SpanEvent` instead of representing them as nanoseconds since boot time in a built-in `int64`. This is motivate by the need to support things that report timestamps for start/end times directly. Things like the custom SDK (see open-telemetry#1045). To support the conversion for probes, the `utils.BootOffsetToTime` function is added. This converts between the measured nanoseconds since boot-time that an eBPF program measures, and a timestamp.
3b14c98
to
b001760
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, if there is a way to call this conversion in the generic probe so we can do it in one place it is better but I don't have a strong opinion about it.
Co-authored-by: Ron Federman <73110295+RonFed@users.noreply.github.com>
All of the |
My point was that before this change the conversion was done in one place (the controller) and now it is duplicated across all the probes. As I said I don't have a strong opinion about this. |
Understood. Keep in mind, with the addition of the SDK not all events are going to report time as an offset from boot-time. This change is made to support that addition. This also means there is not going to be a generic way to handle all event times given they would be timestamps or offsets from boot-time. I see it as preferable to unify on using timestamps, as proposed here, and convert the boot-time offsets so when we allow users to send us data1 they can rely on standard Go types and time measurements. Footnotes |
Use
time.Time
, a timestamp, to identify the start and end time of theSpanEvent
instead of representing them as nanoseconds since boot time in a built-inint64
.This is motivate by the need to support things that report timestamps for start/end times directly. Things like the custom SDK (see #1045).
To support the conversion for probes, the
utils.BootOffsetToTime
function is added. This converts between the measured nanoseconds since boot-time that an eBPF program measures, and a timestamp.The inverse function,
utils.TimeToBootOffset
, is also added to facilitate testing.