Revert "Allow users to sort traces (#8497)" (#8547)

highlight · May 13, 2024 · 7a6bc43 · 7a6bc43
1 parent 73d46ef
commit 7a6bc43
Show file tree

Hide file tree

Showing 17 changed files with 115 additions and 440 deletions.
diff --git a/backend/clickhouse/logs.go b/backend/clickhouse/logs.go
@@ -3,12 +3,11 @@ package clickhouse
 import (
 	"context"
 	"fmt"
+	"github.com/openlyinc/pointy"
 	"math"
 	"strings"
 	"time"
 
-	"github.com/openlyinc/pointy"
-
 	"github.com/ClickHouse/clickhouse-go/v2/lib/driver"
 	e "github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -37,7 +36,6 @@ var logKeysToColumns = map[modelInputs.ReservedLogKey]string{
 	modelInputs.ReservedLogKeyServiceVersion:  "ServiceVersion",
 	modelInputs.ReservedLogKeyEnvironment:     "Environment",
 	modelInputs.ReservedLogKeyMessage:         "Body",
-	modelInputs.ReservedLogKeyTimestamp:       "Timestamp",
 }
 
 // These keys show up as recommendations, but with no recommended values due to high cardinality
@@ -200,8 +198,9 @@ func (client *Client) ReadSessionLogs(ctx context.Context, projectID int, params
 		selectCols,
 		[]int{projectID},
 		params,
-		Pagination{Direction: modelInputs.SortDirectionAsc},
-	)
+		Pagination{},
+		OrderBackwardInverted,
+		OrderForwardInverted)
 	if err != nil {
 		return nil, err
 	}
@@ -252,7 +251,9 @@ func (client *Client) ReadLogsTotalCount(ctx context.Context, projectID int, par
 		[]string{"COUNT(*)"},
 		[]int{projectID},
 		params,
-		Pagination{CountOnly: true})
+		Pagination{CountOnly: true},
+		OrderBackwardNatural,
+		OrderForwardNatural)
 	if err != nil {
 		return 0, err
 	}
@@ -339,6 +340,8 @@ func (client *Client) ReadLogsHistogram(ctx context.Context, projectID int, para
 			[]int{projectID},
 			params,
 			Pagination{CountOnly: true},
+			OrderBackwardNatural,
+			OrderForwardNatural,
 		)
 	} else {
 		fromSb, err = makeSelectBuilder(
@@ -347,6 +350,8 @@ func (client *Client) ReadLogsHistogram(ctx context.Context, projectID int, para
 			[]int{projectID},
 			params,
 			Pagination{CountOnly: true},
+			OrderBackwardNatural,
+			OrderForwardNatural,
 		)
 	}
 	if err != nil {

diff --git a/backend/clickhouse/logs_test.go b/backend/clickhouse/logs_test.go
@@ -43,9 +43,6 @@ func setupTest(tb testing.TB) (*Client, func(tb testing.TB)) {
 
 		err = client.conn.Exec(context.Background(), fmt.Sprintf("TRUNCATE TABLE %s", TracesTable))
 		assert.NoError(tb, err)
-
-		err = client.conn.Exec(context.Background(), fmt.Sprintf("TRUNCATE TABLE %s", TracesSamplingTable))
-		assert.NoError(tb, err)
 	}
 }
 

diff --git a/backend/clickhouse/query.go b/backend/clickhouse/query.go
@@ -39,7 +39,12 @@ func readObjects[TObj interface{}, TReservedKey ~string](ctx context.Context, cl
 	var err error
 	var args []interface{}
 
-	orderForward, _ := getSortOrders[TReservedKey](sb, pagination.Direction, config, params)
+	orderForward := OrderForwardNatural
+	orderBackward := OrderBackwardNatural
+	if pagination.Direction == modelInputs.SortDirectionAsc {
+		orderForward = OrderForwardInverted
+		orderBackward = OrderBackwardInverted
+	}
 
 	outerSelect := strings.Join(config.SelectColumns, ", ")
 	innerSelect := []string{"Timestamp", "UUID"}
@@ -53,7 +58,9 @@ func readObjects[TObj interface{}, TReservedKey ~string](ctx context.Context, cl
 			params,
 			Pagination{
 				Before: pagination.At,
-			})
+			},
+			orderBackward,
+			orderForward)
 		if err != nil {
 			return nil, err
 		}
@@ -66,7 +73,9 @@ func readObjects[TObj interface{}, TReservedKey ~string](ctx context.Context, cl
 			params,
 			Pagination{
 				At: pagination.At,
-			})
+			},
+			orderBackward,
+			orderForward)
 		if err != nil {
 			return nil, err
 		}
@@ -79,7 +88,9 @@ func readObjects[TObj interface{}, TReservedKey ~string](ctx context.Context, cl
 			params,
 			Pagination{
 				After: pagination.At,
-			})
+			},
+			orderBackward,
+			orderForward)
 		if err != nil {
 			return nil, err
 		}
@@ -98,7 +109,9 @@ func readObjects[TObj interface{}, TReservedKey ~string](ctx context.Context, cl
 			innerSelect,
 			[]int{projectID},
 			params,
-			pagination)
+			pagination,
+			orderBackward,
+			orderForward)
 		if err != nil {
 			return nil, err
 		}
@@ -150,11 +163,11 @@ func makeSelectBuilder[T ~string](
 	projectIDs []int,
 	params modelInputs.QueryInput,
 	pagination Pagination,
+	orderBackward string,
+	orderForward string,
 ) (*sqlbuilder.SelectBuilder, error) {
 	sb := sqlbuilder.NewSelectBuilder()
 
-	orderForward, orderBackward := getSortOrders[T](sb, pagination.Direction, config, params)
-
 	sb.Select(selectCols...)
 	sb.From(config.TableName)
 	sb.Where(sb.In("ProjectId", projectIDs))
@@ -599,6 +612,8 @@ func readWorkspaceMetrics[T ~string](ctx context.Context, client *Client, sample
 		projectIDs,
 		params,
 		Pagination{CountOnly: true},
+		OrderBackwardNatural,
+		OrderForwardNatural,
 	)
 
 	var col string
@@ -880,6 +895,8 @@ func logLines[T ~string](ctx context.Context, client *Client, tableConfig model.
 		[]int{projectID},
 		params,
 		Pagination{CountOnly: true},
+		OrderBackwardNatural,
+		OrderForwardNatural,
 	)
 	if err != nil {
 		return nil, err
@@ -942,36 +959,3 @@ func repr(val reflect.Value) string {
 		return val.String()
 	}
 }
-
-func getSortOrders[TReservedKey ~string](
-	sb *sqlbuilder.SelectBuilder,
-	paginationDirection modelInputs.SortDirection,
-	config model.TableConfig[TReservedKey],
-	params modelInputs.QueryInput,
-) (string, string) {
-	sortColumn := "timestamp"
-	sortDirection := modelInputs.SortDirectionDesc
-	if params.Sort != nil {
-		sortColumn = params.Sort.Column
-		sortDirection = params.Sort.Direction
-	}
-
-	if col, found := config.KeysToColumns[TReservedKey(sortColumn)]; found {
-		sortColumn = col
-	} else {
-		sortColumn = fmt.Sprintf("%s[%s]", config.AttributesColumn, sb.Var(sortColumn))
-	}
-
-	forwardDirection := "DESC"
-	backwardDirection := "ASC"
-	if paginationDirection == modelInputs.SortDirectionAsc || sortDirection == modelInputs.SortDirectionAsc {
-		forwardDirection = "ASC"
-	} else {
-		backwardDirection = "DESC"
-	}
-
-	orderForward := fmt.Sprintf("%s %s, UUID %s", sortColumn, forwardDirection, forwardDirection)
-	orderBackward := fmt.Sprintf("%s %s, UUID %s", sortColumn, backwardDirection, backwardDirection)
-
-	return orderForward, orderBackward
-}
diff --git a/backend/clickhouse/traces.go b/backend/clickhouse/traces.go
@@ -46,7 +46,6 @@ var traceKeysToColumns = map[modelInputs.ReservedTraceKey]string{
 	modelInputs.ReservedTraceKeyMetricValue:     "MetricValue",
 	modelInputs.ReservedTraceKeyEnvironment:     "Environment",
 	modelInputs.ReservedTraceKeyHasErrors:       "HasErrors",
-	modelInputs.ReservedTraceKeyTimestamp:       "Timestamp",
 }
 
 var traceColumns = []string{
@@ -269,12 +268,7 @@ func (client *Client) ReadTraces(ctx context.Context, projectID int, params mode
 		}, nil
 	}
 
-	tableConfig := TracesTableConfig
-	if params.Sort != nil {
-		tableConfig = tracesSamplingTableConfig
-	}
-
-	conn, err := readObjects(ctx, client, tableConfig, projectID, params, pagination, scanTrace)
+	conn, err := readObjects(ctx, client, TracesTableConfig, projectID, params, pagination, scanTrace)
 	if err != nil {
 		return nil, err
 	}

diff --git a/backend/clickhouse/traces_test.go b/backend/clickhouse/traces_test.go
@@ -2,7 +2,6 @@ package clickhouse
 
 import (
 	"context"
-	"fmt"
 	"testing"
 	"time"
 
@@ -192,51 +191,3 @@ func TestReadTracesWithEnvironmentFilter(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Len(t, payload.Edges, 2)
 }
-
-func TestReadTracesWithSorting(t *testing.T) {
-	ctx := context.Background()
-	client, teardown := setupTest(t)
-	defer teardown(t)
-
-	now := time.Now()
-	rows := []*ClickhouseTraceRow{
-		NewTraceRow(now, 1).WithSpanName("Span A").WithDuration(now, now.Add(100*time.Nanosecond)).WithTraceAttributes(map[string]string{"host.name": "b"}).AsClickhouseTraceRow(),
-		NewTraceRow(now, 1).WithSpanName("Span B").WithDuration(now, now.Add(300*time.Nanosecond)).WithTraceAttributes(map[string]string{"host.name": "c"}).AsClickhouseTraceRow(),
-		NewTraceRow(now, 1).WithSpanName("Span C").WithDuration(now, now.Add(200*time.Nanosecond)).WithTraceAttributes(map[string]string{"host.name": "a"}).AsClickhouseTraceRow(),
-	}
-
-	assert.NoError(t, client.BatchWriteTraceRows(ctx, rows))
-
-	payload, err := client.ReadTraces(ctx, 1, modelInputs.QueryInput{
-		DateRange: makeDateWithinRange(now),
-		Query:     "",
-		Sort: &modelInputs.SortInput{
-			Column:    "duration",
-			Direction: "DESC",
-		},
-	}, Pagination{})
-	assert.NoError(t, err)
-	assert.Len(t, payload.Edges, 3)
-
-	assert.Equal(t, "Span B", payload.Edges[0].Node.SpanName)
-	assert.Equal(t, "Span C", payload.Edges[1].Node.SpanName)
-	assert.Equal(t, "Span A", payload.Edges[2].Node.SpanName)
-
-	payload, err = client.ReadTraces(ctx, 1, modelInputs.QueryInput{
-		DateRange: makeDateWithinRange(now),
-		Query:     "",
-		Sort: &modelInputs.SortInput{
-			Column:    "host.name",
-			Direction: "DESC",
-		},
-	}, Pagination{})
-	assert.NoError(t, err)
-	assert.Len(t, payload.Edges, 3)
-
-	for i, edge := range payload.Edges {
-		fmt.Printf("Edge %d: %v\n", i, edge.Node)
-	}
-	assert.Equal(t, "Span B", payload.Edges[0].Node.SpanName)
-	assert.Equal(t, "Span A", payload.Edges[1].Node.SpanName)
-	assert.Equal(t, "Span C", payload.Edges[2].Node.SpanName)
-}