Skip to content

Commit

Permalink
Support error and status.code searches (#1093)
Browse files Browse the repository at this point in the history
* Support error and status.code searches

* changelog

* Review feedback

Co-authored-by: Koenraad Verheyden <[email protected]>

* Rename/simplification based on review feedback.

* lint

* Simplify error=true logic

Co-authored-by: Koenraad Verheyden <[email protected]>
  • Loading branch information
mdisibio and yvrhdn authored Nov 2, 2021
1 parent e3a71ec commit 79748ab
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
* [ENHANCEMENT] Jsonnet: add `$._config.search_enabled`, correctly set `http_api_prefix` in config [#1072](https://github.com/grafana/tempo/pull/1072) (@kvrhdn)
* [ENHANCEMENT] Performance: Remove WAL contention between ingest and searches [#1076](https://github.com/grafana/tempo/pull/1076) (@mdisibio)
* [ENHANCEMENT] Include tempo-cli in the release [#1086](https://github.com/grafana/tempo/pull/1086) (@zalegrala)
* [ENHANCEMENT] Add search on span status [#1093](https://github.com/grafana/tempo/pull/1093) (@mdisibio)
* [BUGFIX] Update port spec for GCS docker-compose example [#869](https://github.com/grafana/tempo/pull/869) (@zalegrala)
* [BUGFIX] Fix "magic number" errors and other block mishandling when an ingester forcefully shuts down [#937](https://github.com/grafana/tempo/issues/937) (@mdisibio)
* [BUGFIX] Fix compactor memory leak [#806](https://github.com/grafana/tempo/pull/806) (@mdisibio)
Expand Down
3 changes: 3 additions & 0 deletions modules/distributor/search_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ func extractSearchData(trace *tempopb.Trace, id []byte, extractTag extractTagFun

// Collect for any spans
data.AddTag(search.SpanNameTag, s.Name)
if s.Status != nil {
data.AddTag(search.StatusCodeTag, strconv.Itoa(int(s.Status.Code)))
}
data.SetStartTimeUnixNano(s.StartTimeUnixNano)
data.SetEndTimeUnixNano(s.EndTimeUnixNano)

Expand Down
11 changes: 11 additions & 0 deletions modules/querier/querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/grafana/tempo/pkg/model"
"github.com/grafana/tempo/pkg/tempopb"
"github.com/grafana/tempo/pkg/validation"
"github.com/grafana/tempo/tempodb/search"
"github.com/opentracing/opentracing-go"
ot_log "github.com/opentracing/opentracing-go/log"
"github.com/pkg/errors"
Expand Down Expand Up @@ -309,6 +310,11 @@ func (q *Querier) SearchTags(ctx context.Context, req *tempopb.SearchTagsRequest
}
}

// Extra tags
for _, k := range search.GetVirtualTags() {
uniqueMap[k] = struct{}{}
}

// Final response (sorted)
resp := &tempopb.SearchTagsResponse{
TagNames: make([]string, 0, len(uniqueMap)),
Expand Down Expand Up @@ -348,6 +354,11 @@ func (q *Querier) SearchTagValues(ctx context.Context, req *tempopb.SearchTagVal
}
}

// Extra values
for _, v := range search.GetVirtualTagValues(req.TagName) {
uniqueMap[v] = struct{}{}
}

// Final response (sorted)
resp := &tempopb.SearchTagValuesResponse{
TagValues: make([]string, 0, len(uniqueMap)),
Expand Down
51 changes: 43 additions & 8 deletions tempodb/search/pipeline.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package search

import (
"strconv"
"strings"
"time"

"github.com/grafana/tempo/pkg/tempofb"
"github.com/grafana/tempo/pkg/tempopb"
v1 "github.com/grafana/tempo/pkg/tempopb/trace/v1"
)

const SecretExhaustiveSearchTag = "x-dbg-exhaustive"
Expand Down Expand Up @@ -59,14 +61,8 @@ func NewSearchPipeline(req *tempopb.SearchRequest) Pipeline {
vb := make([][]byte, 0, len(req.Tags))

for k, v := range req.Tags {
if k == SecretExhaustiveSearchTag {
// Perform an exhaustive search by:
// * no block or page filters means all blocks and pages match
// * substitute this trace filter instead rejects everything. therefore it never
// quits early due to enough results
p.tracefilters = append(p.tracefilters, func(s tempofb.Trace) bool {
return false
})
skip, k, v := p.rewriteTagLookup(k, v)
if skip {
continue
}

Expand All @@ -91,6 +87,45 @@ func NewSearchPipeline(req *tempopb.SearchRequest) Pipeline {
return p
}

// rewriteTagLookup intercepts certain tag/value lookups and rewrites them. It returns
// true if the tag lookup should be excluded from the remaining tag/value lookups because
// the it was rewritten into a different filter altogether. Otherwise it returns false,
// and a new set of tag/value strings to use, which will either be the original inputs
// or rewritten lookups.
func (p *Pipeline) rewriteTagLookup(k, v string) (skip bool, newk, newv string) {
switch k {
case SecretExhaustiveSearchTag:
// Perform an exhaustive search by:
// * no block or page filters means all blocks and pages match
// * substitute this trace filter instead rejects everything. therefore it never
// quits early due to enough results
p.tracefilters = append(p.tracefilters, func(s tempofb.Trace) bool {
return false
})
// Skip
return true, "", ""

case ErrorTag:
if v == "true" {
// Error = true
return false, StatusCodeTag, strconv.Itoa(int(v1.Status_STATUS_CODE_ERROR))
}
// Else fall-through

case StatusCodeTag:
// Convert status.code=string into status.code=int
for statusStr, statusID := range statusCodeMapping {
if v == statusStr {
return false, StatusCodeTag, strconv.Itoa(statusID)
}
}
// Unknown mapping = fall-through
}

// No rewrite
return false, k, v
}

func (p *Pipeline) Matches(e tempofb.Trace) bool {

for _, f := range p.tracefilters {
Expand Down
17 changes: 16 additions & 1 deletion tempodb/search/pipeline_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package search

import (
"strconv"
"testing"
"time"

"github.com/grafana/tempo/pkg/tempofb"
"github.com/grafana/tempo/pkg/tempopb"
v1 "github.com/grafana/tempo/pkg/tempopb/trace/v1"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -46,7 +48,20 @@ func TestPipelineMatchesTags(t *testing.T) {
searchData: map[string][]string{"key1": {"value1"}, "key2": {"value2"}},
request: map[string]string{"key1": "value1", "key3": "value3"},
shouldMatch: false,
}}
},
{
name: "rewriteError",
searchData: map[string][]string{StatusCodeTag: {strconv.Itoa(int(v1.Status_STATUS_CODE_ERROR))}},
request: map[string]string{"error": "true"},
shouldMatch: true,
},
{
name: "rewriteStatusCode",
searchData: map[string][]string{StatusCodeTag: {strconv.Itoa(int(v1.Status_STATUS_CODE_ERROR))}},
request: map[string]string{StatusCodeTag: StatusCodeError},
shouldMatch: true,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
Expand Down
29 changes: 29 additions & 0 deletions tempodb/search/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package search
import (
"github.com/grafana/tempo/pkg/tempofb"
"github.com/grafana/tempo/pkg/tempopb"
v1 "github.com/grafana/tempo/pkg/tempopb/trace/v1"
"github.com/grafana/tempo/pkg/util"
)

Expand All @@ -11,8 +12,36 @@ const (
ServiceNameTag = "service.name"
RootSpanNameTag = "root.name"
SpanNameTag = "name"
ErrorTag = "error"
StatusCodeTag = "status.code"
StatusCodeUnset = "unset"
StatusCodeOK = "ok"
StatusCodeError = "error"
)

var statusCodeMapping = map[string]int{
StatusCodeUnset: int(v1.Status_STATUS_CODE_UNSET),
StatusCodeOK: int(v1.Status_STATUS_CODE_OK),
StatusCodeError: int(v1.Status_STATUS_CODE_ERROR),
}

func GetVirtualTags() []string {
return []string{ErrorTag}
}

func GetVirtualTagValues(tagName string) []string {
switch tagName {

case StatusCodeTag:
return []string{StatusCodeUnset, StatusCodeOK, StatusCodeError}

case ErrorTag:
return []string{"true"}
}

return nil
}

func GetSearchResultFromData(s *tempofb.SearchEntry) *tempopb.TraceSearchMetadata {
return &tempopb.TraceSearchMetadata{
TraceID: util.TraceIDToHexString(s.Id()),
Expand Down

0 comments on commit 79748ab

Please sign in to comment.