Skip to content

Commit

Permalink
[v2] Remove temporary SkipBinaryAttrs flag from e2e tests (#5436)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
- PR #5322 temporarily added a SkipBinaryAttrs flag to avoid checking
span's tags with a binary type since the OTEL Jaeger translator has a
bug that converts binary tags into string tags. Since it has been fixed,
we will delete this flag.

## Description of the changes
- Delete the SkipBinaryAttrs flag from StorageIntegration.

## How was this change tested?
- Tested locally by running all the e2e storage tests. e.g.
`STORAGE=grpc SPAN_STORAGE_TYPE=memory make
jaeger-v2-storage-integration-test`

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

Signed-off-by: James Ryans <[email protected]>
  • Loading branch information
james-ryans authored May 11, 2024
1 parent 5ddcaa9 commit fb5c804
Show file tree
Hide file tree
Showing 6 changed files with 1 addition and 44 deletions.
1 change: 0 additions & 1 deletion cmd/jaeger/internal/integration/badger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ func TestBadgerStorage(t *testing.T) {
s := &E2EStorageIntegration{
ConfigFile: "../../config-badger.yaml",
StorageIntegration: integration.StorageIntegration{
SkipBinaryAttrs: true,
SkipArchiveTest: true,
CleanUp: purge,

Expand Down
1 change: 0 additions & 1 deletion cmd/jaeger/internal/integration/cassandra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ func TestCassandraStorage(t *testing.T) {
CleanUp: purge,
GetDependenciesReturnsSource: true,
SkipArchiveTest: true,
SkipBinaryAttrs: true,

SkipList: integration.CassandraSkippedTests,
},
Expand Down
1 change: 0 additions & 1 deletion cmd/jaeger/internal/integration/es_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ func TestESStorage(t *testing.T) {
StorageIntegration: integration.StorageIntegration{
CleanUp: purge,
Fixtures: integration.LoadAndParseQueryTestCases(t, "fixtures/queries_es.json"),
SkipBinaryAttrs: true,
GetOperationsMissingSpanKind: true,
},
}
Expand Down
3 changes: 0 additions & 3 deletions cmd/jaeger/internal/integration/grpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ func TestGRPCStorage(t *testing.T) {
s := &GRPCStorageIntegration{
E2EStorageIntegration: E2EStorageIntegration{
ConfigFile: "../../config-remote-storage.yaml",
StorageIntegration: integration.StorageIntegration{
SkipBinaryAttrs: true,
},
},
}
s.CleanUp = s.cleanUp
Expand Down
1 change: 0 additions & 1 deletion cmd/jaeger/internal/integration/os_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ func TestOSStorage(t *testing.T) {
StorageIntegration: integration.StorageIntegration{
CleanUp: purge,
Fixtures: integration.LoadAndParseQueryTestCases(t, "fixtures/queries_es.json"),
SkipBinaryAttrs: true,
GetOperationsMissingSpanKind: true,
},
}
Expand Down
38 changes: 1 addition & 37 deletions plugin/storage/integration/integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,6 @@ type StorageIntegration struct {
// Skip Archive Test if not supported by the storage backend
SkipArchiveTest bool

// TODO: remove this after upstream issue in OTEL jaeger translator is fixed
// Skip testing trace binary tags, logs, and process
SkipBinaryAttrs bool

// List of tests which has to be skipped, it can be regex too.
SkipList []string

Expand Down Expand Up @@ -369,39 +365,7 @@ func (s *StorageIntegration) loadParseAndWriteLargeTrace(t *testing.T) *model.Tr

func (s *StorageIntegration) getTraceFixture(t *testing.T, fixture string) *model.Trace {
fileName := fmt.Sprintf("fixtures/traces/%s.json", fixture)
trace := getTraceFixtureExact(t, fileName)

if s.SkipBinaryAttrs {
t.Logf("Dropped binary type attributes from trace ID: %s", trace.Spans[0].TraceID.String())
trace = s.dropBinaryAttrs(t, trace)
}

return trace
}

func (s *StorageIntegration) dropBinaryAttrs(t *testing.T, trace *model.Trace) *model.Trace {
for _, span := range trace.Spans {
span.Tags = s.dropBinaryTags(t, span.Tags)
span.Process.Tags = s.dropBinaryTags(t, span.Process.Tags)

for i := range span.Logs {
span.Logs[i].Fields = s.dropBinaryTags(t, span.Logs[i].Fields)
}
}

return trace
}

func (s *StorageIntegration) dropBinaryTags(_ *testing.T, tags []model.KeyValue) []model.KeyValue {
newTags := make([]model.KeyValue, 0)
for _, tag := range tags {
if tag.VType == model.ValueType_BINARY {
continue
}
newTags = append(newTags, tag)
}

return newTags
return getTraceFixtureExact(t, fileName)
}

func getTraceFixtureExact(t *testing.T, fileName string) *model.Trace {
Expand Down

0 comments on commit fb5c804

Please sign in to comment.