Support pganalyze tracestate to set start time of the span #475
Conversation
logs/querysample/tracing.go
Outdated
| if strings.Contains(part, ":") { | ||
| keyAndValue := strings.SplitN(part, ":", 2) | ||
| if strings.TrimSpace(keyAndValue[0]) == "t" { | ||
| if startInSec, err := strconv.ParseFloat(keyAndValue[1], 64); err == nil { |
There was a problem hiding this comment.
I really feel like there should be some easy way to do this, I might be simply missing something.
Also, I could try some another way, like splitting keyAndValue[1] with . then parsing both of them to int. In that way, I don't need to fight with float in test.
There was a problem hiding this comment.
+1 for parsing this as two separate values. It looks like Go's time.Time has an internal representation that allows it more nanosecond precision than can be stored in a 64-bit float. I don't think it will really matter in practice, but we might as well get this right. And simplifying the test is a nice bonus.
Since that's going to complicate this branch, I think we should extract that to a separate function like func parseTraceStateStartTime(value string) (time.Time, error) (no strong feelings on the name, just throwing something out there).
There was a problem hiding this comment.
Ah @lfittl sorry looks like you just approved (forgot to push) but I did some refactoring around here and put that functionality to util. Well hope you're okay with that? Or feel free to 👀 again.
There was a problem hiding this comment.
@keiko713 Refactoring looks good!
logs/querysample/tracing.go
Outdated
| if strings.Contains(part, ":") { | ||
| keyAndValue := strings.SplitN(part, ":", 2) | ||
| if strings.TrimSpace(keyAndValue[0]) == "t" { | ||
| if startInSec, err := strconv.ParseFloat(keyAndValue[1], 64); err == nil { |
There was a problem hiding this comment.
+1 for parsing this as two separate values. It looks like Go's time.Time has an internal representation that allows it more nanosecond precision than can be stored in a 64-bit float. I don't think it will really matter in practice, but we might as well get this right. And simplifying the test is a nice bonus.
Since that's going to complicate this branch, I think we should extract that to a separate function like func parseTraceStateStartTime(value string) (time.Time, error) (no strong feelings on the name, just throwing something out there).
Problems
Currently, we generate an OpenTelemetry tracing span using
sample.OccurredAtandsample.RuntimeMsderived from logs. This approach generally functions well, however, it can encounter precision issues due to the log timestamp format.Specifically, timestamps with lower precision (e.g., hundredths of a second like
12:34:56.78, seen with RDS) result in spans ending at the 100th millisecond mark. This leads to misalignment, causing the span to appear earlier or later than it should.Solution
To solve this, we would like to introduce a new tracestate for pganalyze. With tracestate, we can provide a vendor-specific trace identification information. We will use the key
pganalyzefor this, while the value is a list, containing a member with semicolon (;) separated format. Each member should also have key and value, separated with colon (:).We use a member key
tto specify the start time of the query, the format should be the Unix time in second, allowing decimals to present up to nano seconds.Here is an example of tracestate to be passed:
See OpenTelemetry document for how they use this tracestate. When this is used together with OpenTelemetry tracestate, this will look like:
PR summary
With this PR, we will add the capability for the collector to read a tracestate for pganalyze and use the time specified with
tas the start time of the tracing span.