Skip to content

Commit

Permalink
feat: rename batches property of Trace to ResourceSpans to be OTEL co…
Browse files Browse the repository at this point in the history
…mpatible (#3895)

* rename batches property of Trace to ResourceSpans to be OTEL compatible

* added test to assert that the response hasn't change

* update test data

* fix unmarshall test

* changelog

* fix e2e

* try to fix flaky test

* create helper functions to marshal and unmarshal traces to OTEL compatible schema

* added better documentation
  • Loading branch information
javiermolinar authored Jul 25, 2024
1 parent ae31bbb commit 5cae77c
Show file tree
Hide file tree
Showing 59 changed files with 583 additions and 440 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@
/tempodb/encoding/benchmark_block
private-key.key
integration/e2e/e2e_integration_test[0-9]*
.tempo.yaml
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
* [ENHANCEMENT] Add vParquet4 support to the tempo-cli analyse blocks command [#3868](https://github.com/grafana/tempo/pull/3868) (@stoewer)
* [ENHANCEMENT] Improve trace id lookup from Tempo Vulture by selecting a date range [#3874](https://github.com/grafana/tempo/pull/3874) (@javiermolinar)
* [ENHANCEMENT] Add native histograms for internal metrics[#3870](https://github.com/grafana/tempo/pull/3870) (@zalegrala)
* [ENHANCEMENT] Expose availability-zone as a cli flag in ingester [#3881](https://github.com/grafana/tempo/pull/3881)
* [ENHANCEMENT] Rename batches property of Trace to ResourceSpans to be OTEL compatible [#3895](https://github.com/grafana/tempo/pull/3895)
* [ENHANCEMENT] Reduce memory consumption of query-frontend[#3888](https://github.com/grafana/tempo/pull/3888) (@joe-elliott)
* [ENHANCEMENT] Reduce log level verbosity for e2e tests[#3900](https://github.com/grafana/tempo/pull/3900) (@javiermolinar)
* [BUGFIX] Fix panic in certain metrics queries using `rate()` with `by` [#3847](https://github.com/grafana/tempo/pull/3847) (@stoewer)
Expand All @@ -48,7 +50,7 @@
* [BUGFIX] Improved handling of complete blocks in localblocks processor after enabling flusing [#3805](https://github.com/grafana/tempo/pull/3805) (@mapno)
* [BUGFIX] Handle out of boundaries spans kinds [#3861](https://github.com/grafana/tempo/pull/3861) (@javiermolinar)
* [BUGFIX] Maintain previous tenant blocklist on tenant errors [#3860](https://github.com/grafana/tempo/pull/3860) (@zalegrala)
* [ENHANCEMENT] expose availability-zone as a cli flag in ingester [#3881](https://github.com/grafana/tempo/pull/3881)



## v2.5.0
Expand Down
2 changes: 1 addition & 1 deletion cmd/tempo-cli/cmd-list-block.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func relativeValue(v values) float64 {

func extractKVPairs(t *tempopb.Trace) kvPairs {
kvp := kvPairs{}
for _, b := range t.Batches {
for _, b := range t.ResourceSpans {
spanCount := 0
for _, ils := range b.ScopeSpans {
for _, s := range ils.Spans {
Expand Down
2 changes: 1 addition & 1 deletion cmd/tempo-cli/cmd-query-trace-summary.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func queryBucketForSummary(ctx context.Context, percentage float32, r backend.Re

for q := range resultsCh {
numBlock++
for _, b := range q.trace.Batches {
for _, b := range q.trace.ResourceSpans {
size += b.Size()
for _, attr := range b.Resource.Attributes {
if "service.name" == attr.Key {
Expand Down
6 changes: 3 additions & 3 deletions cmd/tempo-vulture/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ func queryTrace(client httpclient.TempoHTTPClient, info *util.TraceInfo, l *zap.
return tm, err
}

if len(trace.Batches) == 0 {
if len(trace.ResourceSpans) == 0 {
logger.Error("trace contains 0 batches")
tm.notFoundByID++
return tm, nil
Expand Down Expand Up @@ -565,7 +565,7 @@ func hasMissingSpans(t *tempopb.Trace) bool {
// collect all parent span IDs
linkedSpanIDs := make([][]byte, 0)

for _, b := range t.Batches {
for _, b := range t.ResourceSpans {
for _, ss := range b.ScopeSpans {
for _, s := range ss.Spans {
if len(s.ParentSpanId) > 0 {
Expand All @@ -579,7 +579,7 @@ func hasMissingSpans(t *tempopb.Trace) bool {
found := false

B:
for _, b := range t.Batches {
for _, b := range t.ResourceSpans {
for _, ss := range b.ScopeSpans {
for _, s := range ss.Spans {
if bytes.Equal(s.SpanId, id) {
Expand Down
10 changes: 5 additions & 5 deletions cmd/tempo-vulture/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func TestHasMissingSpans(t *testing.T) {
}{
{
&tempopb.Trace{
Batches: []*v1.ResourceSpans{
ResourceSpans: []*v1.ResourceSpans{
{
ScopeSpans: []*v1.ScopeSpans{
{
Expand All @@ -45,7 +45,7 @@ func TestHasMissingSpans(t *testing.T) {
},
{
&tempopb.Trace{
Batches: []*v1.ResourceSpans{
ResourceSpans: []*v1.ResourceSpans{
{
ScopeSpans: []*v1.ScopeSpans{
{
Expand Down Expand Up @@ -262,13 +262,13 @@ type traceOps func(*tempopb.Trace)
func TestQueryTrace(t *testing.T) {
noOp := func(_ *tempopb.Trace) {}
setMissingSpan := func(trace *tempopb.Trace) {
trace.Batches[0].ScopeSpans[0].Spans[0].ParentSpanId = []byte{'t', 'e', 's', 't'}
trace.ResourceSpans[0].ScopeSpans[0].Spans[0].ParentSpanId = []byte{'t', 'e', 's', 't'}
}
setNoBatchesSpan := func(trace *tempopb.Trace) {
trace.Batches = make([]*v1.ResourceSpans, 0)
trace.ResourceSpans = make([]*v1.ResourceSpans, 0)
}
setAlteredSpan := func(trace *tempopb.Trace) {
trace.Batches[0].ScopeSpans[0].Spans[0].Name = "Different spam"
trace.ResourceSpans[0].ScopeSpans[0].Spans[0].Name = "Different spam"
}
tests := []struct {
name string
Expand Down
2 changes: 1 addition & 1 deletion cmd/tempo-vulture/testdata/trace.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"batches": [
"resourceSpans": [
{
"resource": {
"attributes": [
Expand Down
8 changes: 5 additions & 3 deletions integration/e2e/compression_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ package e2e

import (
"compress/gzip"
"io"
"net/http"
"testing"
"time"

"github.com/gogo/protobuf/jsonpb"
"github.com/grafana/e2e"
"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -74,8 +74,10 @@ func queryAndAssertTraceCompression(t *testing.T, client *httpclient.Client, inf
defer gzipReader.Close()

m := &tempopb.Trace{}
unmarshaller := &jsonpb.Unmarshaler{}
err = unmarshaller.Unmarshal(gzipReader, m)

bodyBytes, _ := io.ReadAll(gzipReader)
err = tempopb.UnmarshalFromJSONV1(bodyBytes, m)

require.NoError(t, err)
assertEqualTrace(t, expected, m)
}
2 changes: 1 addition & 1 deletion integration/e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ func assertEqualTrace(t *testing.T, a, b *tempopb.Trace) {

func spanCount(a *tempopb.Trace) float64 {
count := 0
for _, batch := range a.Batches {
for _, batch := range a.ResourceSpans {
for _, spans := range batch.ScopeSpans {
count += len(spans.Spans)
}
Expand Down
2 changes: 1 addition & 1 deletion integration/e2e/limits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ func TestLimitsPartialSuccess(t *testing.T) {
if count == 1 {
result, err := client.QueryTrace(tempoUtil.TraceIDToHexString(traceIDs[i]))
require.NoError(t, err)
assert.Equal(t, 1, len(result.Batches))
assert.Equal(t, 1, len(result.ResourceSpans))
}
}

Expand Down
Loading

0 comments on commit 5cae77c

Please sign in to comment.