diff --git a/CHANGELOG.md b/CHANGELOG.md index e47d5018733..83f6819cb98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ Additionally, default label `span_status` is renamed to `status_code`. * [BUGFIX] Fix nil pointer panic when the trace by id path errors. [#1441](https://github.com/grafana/tempo/pull/1441) (@joe-elliott) * [BUGFIX] Update tempo microservices Helm values example which missed the 'enabled' key for thriftHttp. [#1472](https://github.com/grafana/tempo/pull/1472) (@hajowieland) * [BUGFIX] Fix race condition in forwarder overrides loop. [1468](https://github.com/grafana/tempo/pull/1468) (@mapno) +* [BUGFIX] Fix v2 backend check on span name to be substring [#1538](https://github.com/grafana/tempo/pull/1538) (@mdisibio) * [ENHANCEMENT] Add a config to query single ingester instance based on trace id hash for Trace By ID API. (1484)[https://github.com/grafana/tempo/pull/1484] (@sagarwala, @bikashmishra100, @ashwinidulams) ## v1.4.1 / 2022-05-05 diff --git a/pkg/model/trace/matches.go b/pkg/model/trace/matches.go index c5285a86c1b..2aa769a78f8 100644 --- a/pkg/model/trace/matches.go +++ b/pkg/model/trace/matches.go @@ -88,8 +88,10 @@ func MatchesProto(id []byte, trace *tempopb.Trace, req *tempopb.SearchRequest) ( return nil, nil } - if !(req.Start <= uint32(traceEndMs/1000) && req.End >= uint32(traceStartMs/1000)) { - return nil, nil + if req.Start > 0 || req.End > 0 { + if !(req.Start <= uint32(traceEndMs/1000) && req.End >= uint32(traceStartMs/1000)) { + return nil, nil + } } // woohoo! @@ -124,7 +126,7 @@ func allTagsFound(tagsToFind map[string]string) bool { // the provided tags map func matchSpan(tags map[string]string, s *v1.Span) { if name, ok := tags[SpanNameTag]; ok { - if name == s.Name { + if strings.Contains(s.Name, name) { delete(tags, SpanNameTag) } } diff --git a/pkg/model/v2/object_decoder.go b/pkg/model/v2/object_decoder.go index 97539740d83..2e504a83408 100644 --- a/pkg/model/v2/object_decoder.go +++ b/pkg/model/v2/object_decoder.go @@ -61,8 +61,10 @@ func (d *ObjectDecoder) Matches(id []byte, obj []byte, req *tempopb.SearchReques return nil, err } - if !(req.Start <= end && req.End >= start) { - return nil, nil + if req.Start > 0 || req.End > 0 { + if !(req.Start <= end && req.End >= start) { + return nil, nil + } } // assert duration before we unmarshal diff --git a/tempodb/encoding/vparquet/block_search.go b/tempodb/encoding/vparquet/block_search.go index 408a6c3df7f..8b4b9e95ed2 100644 --- a/tempodb/encoding/vparquet/block_search.go +++ b/tempodb/encoding/vparquet/block_search.go @@ -23,8 +23,7 @@ import ( // These are reserved search parameters const ( - LabelName = "name" - LabelStatus = "status" + LabelName = "name" StatusCodeTag = "status.code" StatusCodeUnset = "unset" @@ -182,7 +181,7 @@ func makePipelineWithRowGroups(ctx context.Context, req *tempopb.SearchRequest, } // Non-numeric string field otherAttrConditions[k] = v - case LabelStatus: + case StatusCodeTag: code := StatusCodeMapping[v] resourceIters = append(resourceIters, makeIter("rs.ils.Spans.StatusCode", pq.NewIntBetweenPredicate(int64(code), int64(code)), "")) default: diff --git a/tempodb/encoding/vparquet/block_search_test.go b/tempodb/encoding/vparquet/block_search_test.go index cf69d635270..7dae6f1deb1 100644 --- a/tempodb/encoding/vparquet/block_search_test.go +++ b/tempodb/encoding/vparquet/block_search_test.go @@ -122,7 +122,7 @@ func TestBackendBlockSearch(t *testing.T) { makeReq(LabelHTTPMethod, "get"), makeReq(LabelHTTPUrl, "hello"), makeReq(LabelHTTPStatusCode, "500"), - makeReq(LabelStatus, StatusCodeError), + makeReq(StatusCodeTag, StatusCodeError), // Span attributes makeReq("foo", "bar"), @@ -176,7 +176,7 @@ func TestBackendBlockSearch(t *testing.T) { makeReq(LabelHTTPMethod, "post"), makeReq(LabelHTTPUrl, "asdf"), makeReq(LabelHTTPStatusCode, "200"), - makeReq(LabelStatus, StatusCodeOK), + makeReq(StatusCodeTag, StatusCodeOK), // Span attributes makeReq("foo", "baz"), diff --git a/tempodb/tempodb_search_test.go b/tempodb/tempodb_search_test.go new file mode 100644 index 00000000000..e1420062fdb --- /dev/null +++ b/tempodb/tempodb_search_test.go @@ -0,0 +1,286 @@ +package tempodb + +import ( + "context" + "path" + "testing" + "time" + + "github.com/go-kit/log" + "github.com/google/uuid" + "github.com/grafana/tempo/pkg/model" + "github.com/grafana/tempo/pkg/tempopb" + v1_common "github.com/grafana/tempo/pkg/tempopb/common/v1" + v1_resource "github.com/grafana/tempo/pkg/tempopb/resource/v1" + v1 "github.com/grafana/tempo/pkg/tempopb/trace/v1" + "github.com/grafana/tempo/pkg/util" + "github.com/grafana/tempo/pkg/util/test" + "github.com/grafana/tempo/tempodb/backend/local" + "github.com/grafana/tempo/tempodb/encoding/common" + v2 "github.com/grafana/tempo/tempodb/encoding/v2" + "github.com/grafana/tempo/tempodb/encoding/vparquet" + "github.com/grafana/tempo/tempodb/wal" + "github.com/stretchr/testify/require" +) + +func TestSearchCompleteBlock(t *testing.T) { + for _, v := range []string{v2.VersionString, vparquet.VersionString} { + t.Run(v, func(t *testing.T) { + testSearchCompleteBlock(t, v) + }) + } +} + +func testSearchCompleteBlock(t *testing.T, blockVersion string) { + tempDir := t.TempDir() + ctx := context.Background() + + r, w, c, err := New(&Config{ + Backend: "local", + Local: &local.Config{ + Path: path.Join(tempDir, "traces"), + }, + Block: &common.BlockConfig{ + IndexDownsampleBytes: 17, + BloomFP: .01, + BloomShardSizeBytes: 100_000, + Version: blockVersion, + IndexPageSizeBytes: 1000, + }, + WAL: &wal.Config{ + Filepath: path.Join(tempDir, "wal"), + IngestionSlack: time.Since(time.Time{}), + }, + BlocklistPoll: 0, + }, log.NewNopLogger()) + require.NoError(t, err) + + c.EnableCompaction(&CompactorConfig{ + ChunkSizeBytes: 10, + MaxCompactionRange: time.Hour, + BlockRetention: 0, + CompactedBlockRetention: 0, + }, &mockSharder{}, &mockOverrides{}) + + r.EnablePolling(&mockJobSharder{}) + rw := r.(*readerWriter) + + id, wantTr, wantMeta := fullyPopulatedSearchTrace() + + // Write to wal + wal := w.WAL() + head, err := wal.NewBlock(uuid.New(), testTenantID, model.CurrentEncoding) + require.NoError(t, err) + dec := model.MustNewSegmentDecoder(model.CurrentEncoding) + b1, err := dec.PrepareForWrite(wantTr, 1000, 1001) + require.NoError(t, err) + b2, err := dec.ToObject([][]byte{b1}) + require.NoError(t, err) + err = head.Append(id, b2, 1000, 1001) + require.NoError(t, err, "unexpected error writing req") + + // Complete block + block, err := w.CompleteBlock(head, &mockCombiner{}) + require.NoError(t, err) + meta := block.BlockMeta() + + // Helper function to make a tag search + makeReq := func(k, v string) *tempopb.SearchRequest { + return &tempopb.SearchRequest{ + Tags: map[string]string{ + k: v, + }, + } + } + + // Matches + searchesThatMatch := []*tempopb.SearchRequest{ + { + // Empty request + }, + { + MinDurationMs: 999, + MaxDurationMs: 1001, + }, + { + Start: 1000, + End: 2000, + }, + { + // Overlaps start + Start: 999, + End: 1001, + }, + { + // Overlaps end + Start: 1001, + End: 1002, + }, + + // Well-known resource attributes + makeReq("service.name", "service"), + makeReq("cluster", "cluster"), + makeReq("namespace", "namespace"), + makeReq("pod", "pod"), + makeReq("container", "container"), + makeReq("k8s.cluster.name", "k8scluster"), + makeReq("k8s.namespace.name", "k8snamespace"), + makeReq("k8s.pod.name", "k8spod"), + makeReq("k8s.container.name", "k8scontainer"), + + // Well-known span attributes + makeReq("name", "ell"), + makeReq("http.method", "get"), + makeReq("http.url", "hello"), + makeReq("http.status_code", "500"), + makeReq("status.code", "error"), + + // Span attributes + makeReq("foo", "bar"), + // Resource attributes + makeReq("bat", "baz"), + + // Multiple + { + Tags: map[string]string{ + "service.name": "service", + "http.method": "get", + "foo": "bar", + }, + }, + } + + for _, req := range searchesThatMatch { + res, err := r.Search(ctx, meta, req, common.DefaultSearchOptions()) + require.NoError(t, err) + require.Equal(t, 1, len(res.Traces), "search request: %+v", req) + require.Equal(t, wantMeta, res.Traces[0], "search request:", req) + } + + // Excludes + searchesThatDontMatch := []*tempopb.SearchRequest{ + { + MinDurationMs: 1001, + }, + { + MaxDurationMs: 999, + }, + { + Start: 100, + End: 200, + }, + + // Well-known resource attributes + makeReq("service.name", "foo"), + makeReq("cluster", "foo"), + makeReq("namespace", "foo"), + makeReq("pod", "foo"), + makeReq("container", "foo"), + + // Well-known span attributes + makeReq("http.method", "post"), + makeReq("http.url", "asdf"), + makeReq("http.status_code", "200"), + makeReq("status.code", "ok"), + + // Span attributes + makeReq("foo", "baz"), + } + for _, req := range searchesThatDontMatch { + res, err := rw.Search(ctx, meta, req, common.DefaultSearchOptions()) + require.NoError(t, err) + require.Empty(t, res.Traces, "search request:", req) + } + +} + +// This is a fully-populated trace that we search for every condition +func fullyPopulatedSearchTrace() (common.ID, *tempopb.Trace, *tempopb.TraceSearchMetadata) { + stringKV := func(k, v string) *v1_common.KeyValue { + return &v1_common.KeyValue{ + Key: k, + Value: &v1_common.AnyValue{Value: &v1_common.AnyValue_StringValue{StringValue: v}}, + } + } + + intKV := func(k string, v int) *v1_common.KeyValue { + return &v1_common.KeyValue{ + Key: k, + Value: &v1_common.AnyValue{Value: &v1_common.AnyValue_IntValue{IntValue: int64(v)}}, + } + } + id := test.ValidTraceID(nil) + tr := &tempopb.Trace{ + Batches: []*v1.ResourceSpans{ + { + Resource: &v1_resource.Resource{ + Attributes: []*v1_common.KeyValue{ + stringKV("service.name", "myservice"), + stringKV("cluster", "cluster"), + stringKV("namespace", "namespace"), + stringKV("pod", "pod"), + stringKV("container", "container"), + stringKV("k8s.cluster.name", "k8scluster"), + stringKV("k8s.namespace.name", "k8snamespace"), + stringKV("k8s.pod.name", "k8spod"), + stringKV("k8s.container.name", "k8scontainer"), + stringKV("bat", "baz"), + }, + }, + InstrumentationLibrarySpans: []*v1.InstrumentationLibrarySpans{ + { + Spans: []*v1.Span{ + { + TraceId: id, + Name: "hello", + SpanId: []byte{1, 2, 3}, + ParentSpanId: []byte{4, 5, 6}, + StartTimeUnixNano: uint64(1000 * time.Second), + EndTimeUnixNano: uint64(1001 * time.Second), + Status: &v1.Status{ + Code: v1.Status_STATUS_CODE_ERROR, + }, + Attributes: []*v1_common.KeyValue{ + stringKV("http.method", "get"), + stringKV("http.url", "url/hello/world"), + intKV("http.status_code", 500), + stringKV("foo", "bar"), + }, + }, + }, + }, + }, + }, + { + Resource: &v1_resource.Resource{ + Attributes: []*v1_common.KeyValue{ + stringKV("service.name", "RootService"), + }, + }, + InstrumentationLibrarySpans: []*v1.InstrumentationLibrarySpans{ + { + Spans: []*v1.Span{ + { + TraceId: id, + Name: "RootSpan", + StartTimeUnixNano: uint64(1000 * time.Second), + EndTimeUnixNano: uint64(1001 * time.Second), + Status: &v1.Status{}, + }, + }, + }, + }, + }, + }, + } + + expected := &tempopb.TraceSearchMetadata{ + TraceID: util.TraceIDToHexString(id), + StartTimeUnixNano: uint64(1000 * time.Second), + DurationMs: 1000, + RootServiceName: "RootService", + RootTraceName: "RootSpan", + } + + return id, tr, expected +}