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.

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

Support pganalyze tracestate to set start time of the span #475

Merged
merged 4 commits into from
Nov 21, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 38 additions & 3 deletions logs/querysample/tracing.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"encoding/binary"
"encoding/hex"
"fmt"
"strconv"
"strings"
"time"

"github.com/pganalyze/collector/state"
Expand Down Expand Up @@ -32,6 +34,41 @@ func urlToSample(server *state.Server, grant state.GrantLogs, sample state.Postg
)
}

func startAndEndTime(traceState trace.TraceState, sample state.PostgresQuerySample) (startTime time.Time, endTime time.Time) {
if pganalyzeState := traceState.Get("pganalyze"); pganalyzeState != "" {
// A pganalyze traceState allows the client to pass the query start time (sent time)
// on the client side, in nano second precision, like pganalyze=t:1697666938.6297212
// If there are multiple values in a pganalzye traceState, they are separated by semicolon
// like pganalyze=t:1697666938.6297212;x=123
for _, part := range strings.Split(strings.TrimSpace(pganalyzeState), ";") {
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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

+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).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

@keiko713 Refactoring looks good!

startSec := int64(startInSec)
startNanoSec := int64(startInSec*1e9) - (startSec * 1e9)
startTime = time.Unix(startSec, startNanoSec).UTC()
// With this, we're adding the query duration to the start time.
// This could result creating inaccurate spans, as the start time passed
// from the client side using tracestate is the time of the query is sent
// from the client to the server.
// This means, we will ignore the network time between the client and the
// server, as well as the machine clock different between them.
keiko713 marked this conversation as resolved.
Show resolved Hide resolved
endTime = startTime.Add(time.Duration(sample.RuntimeMs) * time.Millisecond)
return
}
}
}
}
}
// Calculate start and end time based on sample data
keiko713 marked this conversation as resolved.
Show resolved Hide resolved
duration := time.Duration(sample.RuntimeMs) * time.Millisecond
startTime = sample.OccurredAt.Add(-1 * duration)
keiko713 marked this conversation as resolved.
Show resolved Hide resolved
endTime = sample.OccurredAt

return
}

func ExportQuerySamplesAsTraceSpans(ctx context.Context, server *state.Server, logger *util.Logger, grant state.GrantLogs, samples []state.PostgresQuerySample) {
exportCount := 0
for _, sample := range samples {
Expand All @@ -49,9 +86,7 @@ func ExportQuerySamplesAsTraceSpans(ctx context.Context, server *state.Server, l
trace.WithInstrumentationVersion(util.CollectorVersion),
trace.WithSchemaURL(semconv.SchemaURL),
)
duration := -1 * time.Duration(sample.RuntimeMs) * time.Millisecond
startTime := sample.OccurredAt.Add(duration)
endTime := sample.OccurredAt
startTime, endTime := startAndEndTime(trace.SpanContextFromContext(ctx).TraceState(), sample)
_, span := tracer.Start(ctx, otelSpanName, trace.WithTimestamp(startTime))
// See https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/database/
// however note that "db.postgresql.plan" is non-standard.
Expand Down
86 changes: 86 additions & 0 deletions logs/querysample/tracing_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package querysample

import (
"testing"
"time"

"github.com/pganalyze/collector/state"
"go.opentelemetry.io/otel/trace"
)

type startAndEndTimeTestPair struct {
testName string
traceState trace.TraceState
sample state.PostgresQuerySample
startTime time.Time
endTime time.Time
}

func TestStartAndEndTime(t *testing.T) {
currentTime, err := time.Parse("2006-01-02", "2023-01-01")
if err != nil {
t.Fatalf("Failed to initialize object: %v", err)
}
traceState := trace.TraceState{}
otelTraceState, err := traceState.Insert("ot", "p:8;r:62")
if err != nil {
t.Fatalf("Failed to initialize object: %v", err)
}
pganalyzeTraceStateWithoutT, err := traceState.Insert("pganalyze", "x:foo;y:bar")
if err != nil {
t.Fatalf("Failed to initialize object: %v", err)
}
// inserting the same key will update the value
pganalyzeTraceState, err := traceState.Insert("pganalyze", "t:1697666938.6297212")
keiko713 marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
t.Fatalf("Failed to initialize object: %v", err)
}
// 1697666938.6297212 = 2023-10-18 22:08:58.6297212
pganalyzeTime, err := time.Parse("2006-01-02T15:04:05.999999999", "2023-10-18T22:08:58.6297212")
if err != nil {
t.Fatalf("Failed to initialize object: %v", err)
}
// due to the limitation of the floating point, the result won't exactly like above, so tweaking to pass the test
pganalyzeTime = pganalyzeTime.Add(-1 * 112)

var startAndEndTimeTests = []startAndEndTimeTestPair{
{
"No trace state",
trace.TraceState{},
state.PostgresQuerySample{RuntimeMs: 1000, OccurredAt: currentTime},
currentTime.Add(-1 * 1000 * time.Millisecond),
currentTime,
},
{
"No pganalyze trace state",
otelTraceState,
state.PostgresQuerySample{RuntimeMs: 1000, OccurredAt: currentTime},
currentTime.Add(-1 * 1000 * time.Millisecond),
currentTime,
},
{
"pganalyze trace state without t",
pganalyzeTraceStateWithoutT,
state.PostgresQuerySample{RuntimeMs: 1000, OccurredAt: currentTime},
currentTime.Add(-1 * 1000 * time.Millisecond),
currentTime,
},
{
"pganalyze trace state",
pganalyzeTraceState,
state.PostgresQuerySample{RuntimeMs: 1000, OccurredAt: currentTime},
pganalyzeTime,
pganalyzeTime.Add(1000 * time.Millisecond),
},
}

for _, pair := range startAndEndTimeTests {
startTime, endTime := startAndEndTime(pair.traceState, pair.sample)
if pair.startTime != startTime {
t.Errorf("For %s: expected startTime to be %v, but was %v\n", pair.testName, pair.startTime, startTime)
}
if pair.endTime != endTime {
t.Errorf("For %s: expected endTime to be %v, but was %v\n", pair.testName, pair.endTime, endTime)
}
}
}
Loading