Skip to content

Commit

Permalink
review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
MasslessParticle committed Apr 3, 2024
1 parent cab8da2 commit d302464
Show file tree
Hide file tree
Showing 8 changed files with 123 additions and 19 deletions.
17 changes: 16 additions & 1 deletion pkg/loghttp/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ type QueryStatus string
const (
QueryStatusSuccess = "success"
QueryStatusFail = "fail"
// How much stack space to allocate for unescaping JSON strings; if a string longer
// than this needs to be escaped, it will result in a heap allocation
unescapeStackBufSize = 64
)

// QueryResponse represents the http json response to a Loki range and instant query
Expand All @@ -55,7 +58,9 @@ func (q *QueryResponse) UnmarshalJSON(data []byte) error {
case "warnings":
var warnings []string
if _, err := jsonparser.ArrayEach(value, func(value []byte, dataType jsonparser.ValueType, offset int, err error) {
warnings = append(warnings, string(value))
if dataType == jsonparser.String {
warnings = append(warnings, unescapeJSONString(value))
}
}); err != nil {
return err
}
Expand All @@ -72,6 +77,16 @@ func (q *QueryResponse) UnmarshalJSON(data []byte) error {
})
}

func unescapeJSONString(b []byte) string {
var stackbuf [unescapeStackBufSize]byte // stack-allocated array for allocation-free unescaping of small strings
bU, err := jsonparser.Unescape(b, stackbuf[:])
if err != nil {
return ""
}

return string(bU)
}

// PushRequest models a log stream push but is unmarshalled to proto push format.
type PushRequest struct {
Streams []LogProtoStream `json:"streams"`
Expand Down
6 changes: 2 additions & 4 deletions pkg/logql/accumulator.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"container/heap"
"context"
"fmt"
"golang.org/x/exp/maps"
"sort"
"time"

Expand Down Expand Up @@ -355,10 +356,7 @@ func (acc *AccumulatedStreams) Result() []logqlmodel.Result {
)
}

warnings := make([]string, 0, len(acc.warnings))
for w := range acc.warnings {
warnings = append(warnings, w)
}
warnings := maps.Keys(acc.warnings)
sort.Strings(warnings)

res.Warnings = warnings
Expand Down
11 changes: 6 additions & 5 deletions pkg/logqlmodel/metadata/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package metadata
import (
"context"
"errors"
"golang.org/x/exp/maps"
"sort"
"sync"

Expand Down Expand Up @@ -79,11 +80,7 @@ func (c *Context) Warnings() []string {
c.mtx.Lock()
defer c.mtx.Unlock()

warnings := make([]string, 0, len(c.warnings))
for warning := range c.warnings {
warnings = append(warnings, warning)
}

warnings := maps.Keys(c.warnings)
sort.Strings(warnings)

return warnings
Expand Down Expand Up @@ -121,6 +118,10 @@ func ExtendHeaders(dst map[string][]string, src []*definitions.PrometheusRespons
}

func AddWarnings(ctx context.Context, warnings ...string) error {
if len(warnings) == 0 {
return nil
}

context, ok := ctx.Value(metadataKey).(*Context)
if !ok {
return ErrNoCtxData
Expand Down
6 changes: 2 additions & 4 deletions pkg/querier/queryrange/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"context"
"errors"
"fmt"
"golang.org/x/exp/maps"
"io"
"net/http"
"net/url"
Expand Down Expand Up @@ -1992,10 +1993,7 @@ func mergeLokiResponse(responses ...queryrangebase.Response) *LokiResponse {
}
}

warnings := make([]string, 0, len(uniqueWarnings))
for w := range uniqueWarnings {
warnings = append(warnings, w)
}
warnings := maps.Keys(uniqueWarnings)
sort.Strings(warnings)

if len(warnings) == 0 {
Expand Down
6 changes: 2 additions & 4 deletions pkg/querier/queryrange/queryrangebase/query_range.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"fmt"
"golang.org/x/exp/maps"
"io"
"math"
"net/http"
Expand Down Expand Up @@ -171,10 +172,7 @@ func (p prometheusCodec) MergeResponse(responses ...Response) (Response, error)
}
}

warnings := make([]string, 0, len(uniqueWarnings))
for w := range uniqueWarnings {
warnings = append(warnings, w)
}
warnings := maps.Keys(uniqueWarnings)
sort.Strings(warnings)

if len(warnings) == 0 {
Expand Down
1 change: 0 additions & 1 deletion pkg/querier/queryrange/querysharding.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,6 @@ func (ast *astMapperware) Do(ctx context.Context, r queryrangebase.Request) (que
return nil, err
}

// TODO(tpatterson): add warnings to response
switch res.Data.Type() {
case parser.ValueTypeMatrix:
return &LokiPromResponse{
Expand Down
94 changes: 94 additions & 0 deletions vendor/golang.org/x/exp/maps/maps.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1633,6 +1633,7 @@ golang.org/x/crypto/sha3
# golang.org/x/exp v0.0.0-20240325151524-a685a6edb6d8
## explicit; go 1.20
golang.org/x/exp/constraints
golang.org/x/exp/maps
golang.org/x/exp/slices
# golang.org/x/mod v0.16.0
## explicit; go 1.18
Expand Down

0 comments on commit d302464

Please sign in to comment.