Skip to content

Commit

Permalink
[refactor] Change TraceQueryParams to accept typed attributes (#6780)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
- Towards #6765

## Description of the changes
- Updates `TraceQueryParams` to change the `Attributes` field from a
`map[string]string` to `pcommon.Map` to accept typed attributes to match
the Remote Storage API v2 being developed as part of
#6629

## How was this change tested?
- CI

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [x] 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`: `npm run lint` and `npm run test`

---------

Signed-off-by: Mahad Zaryab <[email protected]>
  • Loading branch information
mahadzaryab1 authored Feb 27, 2025
1 parent 1c99aa0 commit 043b826
Show file tree
Hide file tree
Showing 13 changed files with 221 additions and 57 deletions.
3 changes: 2 additions & 1 deletion cmd/jaeger/internal/integration/trace_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"google.golang.org/grpc/credentials/insecure"
"google.golang.org/grpc/status"

"github.com/jaegertracing/jaeger/internal/jptrace"
"github.com/jaegertracing/jaeger/internal/proto/api_v3"
"github.com/jaegertracing/jaeger/internal/storage/v1/api/spanstore"
"github.com/jaegertracing/jaeger/internal/storage/v2/api/tracestore"
Expand Down Expand Up @@ -120,7 +121,7 @@ func (r *traceReader) FindTraces(
Query: &api_v3.TraceQueryParameters{
ServiceName: query.ServiceName,
OperationName: query.OperationName,
Attributes: query.Tags,
Attributes: jptrace.AttributesToMap(query.Attributes),
StartTimeMin: query.StartTimeMin,
StartTimeMax: query.StartTimeMax,
DurationMin: query.DurationMin,
Expand Down
3 changes: 2 additions & 1 deletion cmd/query/app/apiv3/grpc_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (

"github.com/jaegertracing/jaeger-idl/model/v1"
"github.com/jaegertracing/jaeger/cmd/query/app/querysvc/v2/querysvc"
"github.com/jaegertracing/jaeger/internal/jptrace"
"github.com/jaegertracing/jaeger/internal/proto/api_v3"
"github.com/jaegertracing/jaeger/internal/storage/v2/api/tracestore"
"github.com/jaegertracing/jaeger/internal/storage/v2/v1adapter"
Expand Down Expand Up @@ -73,7 +74,7 @@ func (h *Handler) internalFindTraces(
TraceQueryParams: tracestore.TraceQueryParams{
ServiceName: query.GetServiceName(),
OperationName: query.GetOperationName(),
Tags: query.GetAttributes(),
Attributes: jptrace.MapToAttributes(query.GetAttributes()),
NumTraces: int(query.GetSearchDepth()),
},
RawTraces: query.GetRawTraces(),
Expand Down
3 changes: 2 additions & 1 deletion cmd/query/app/apiv3/http_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/gogo/protobuf/jsonpb"
"github.com/gogo/protobuf/proto"
"github.com/gorilla/mux"
"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/pdata/ptrace"
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
"go.opentelemetry.io/otel/trace"
Expand Down Expand Up @@ -211,7 +212,7 @@ func (h *HTTPGateway) parseFindTracesQuery(q url.Values, w http.ResponseWriter)
TraceQueryParams: tracestore.TraceQueryParams{
ServiceName: q.Get(paramServiceName),
OperationName: q.Get(paramOperationName),
Tags: nil, // most curiously not supported by grpc-gateway
Attributes: pcommon.NewMap(), // most curiously not supported by grpc-gateway
},
}

Expand Down
2 changes: 2 additions & 0 deletions cmd/query/app/apiv3/http_gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/pdata/ptrace"
"go.uber.org/zap"

Expand Down Expand Up @@ -260,6 +261,7 @@ func mockFindQueries() (url.Values, tracestore.TraceQueryParams) {
return q, tracestore.TraceQueryParams{
ServiceName: "foo",
OperationName: "bar",
Attributes: pcommon.NewMap(),
StartTimeMin: time1,
StartTimeMax: time2,
DurationMin: 1 * time.Second,
Expand Down
21 changes: 21 additions & 0 deletions internal/jptrace/attributes.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
// SPDX-License-Identifier: Apache-2.0
package jptrace

import (
"go.opentelemetry.io/collector/pdata/pcommon"
)

const (
// WarningsAttribute is the name of the span attribute where we can
// store various warnings produced from transformations,
Expand All @@ -13,3 +17,20 @@ const (
// e.g. proto, thrift, json.
FormatAttribute = "@jaeger@format"
)

func AttributesToMap(attributes pcommon.Map) map[string]string {
tags := make(map[string]string)
attributes.Range(func(k string, v pcommon.Value) bool {
tags[k] = v.AsString()
return true
})
return tags
}

func MapToAttributes(tags map[string]string) pcommon.Map {
attributes := pcommon.NewMap()
for k, v := range tags {
attributes.PutStr(k, v)
}
return attributes
}
101 changes: 101 additions & 0 deletions internal/jptrace/attributes_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
// Copyright (c) 2025 The Jaeger Authors.
// SPDX-License-Identifier: Apache-2.0

package jptrace

import (
"testing"

"github.com/stretchr/testify/assert"
"go.opentelemetry.io/collector/pdata/pcommon"
)

func TestAttributesToMap(t *testing.T) {
tests := []struct {
name string
attributes pcommon.Map
expected map[string]string
}{
{
name: "empty attributes",
attributes: pcommon.NewMap(),
expected: map[string]string{},
},
{
name: "single attribute",
attributes: func() pcommon.Map {
m := pcommon.NewMap()
m.PutStr("key1", "value1")
return m
}(),
expected: map[string]string{"key1": "value1"},
},
{
name: "multiple attributes",
attributes: func() pcommon.Map {
m := pcommon.NewMap()
m.PutStr("key1", "value1")
m.PutStr("key2", "value2")
return m
}(),
expected: map[string]string{"key1": "value1", "key2": "value2"},
},
{
name: "non-string attributes",
attributes: func() pcommon.Map {
m := pcommon.NewMap()
m.PutInt("key1", 1)
m.PutDouble("key2", 3.14)
return m
}(),
expected: map[string]string{"key1": "1", "key2": "3.14"},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
result := AttributesToMap(test.attributes)
assert.Equal(t, test.expected, result)
})
}
}

func TestMapToAttributes(t *testing.T) {
tests := []struct {
name string
tags map[string]string
expected pcommon.Map
}{
{
name: "empty map",
tags: map[string]string{},
expected: pcommon.NewMap(),
},
{
name: "single tag",
tags: map[string]string{"key1": "value1"},
expected: func() pcommon.Map {
m := pcommon.NewMap()
m.PutStr("key1", "value1")
return m
}(),
},
{
name: "multiple tags",
tags: map[string]string{"key1": "value1", "key2": "value2"},
expected: func() pcommon.Map {
m := pcommon.NewMap()
m.PutStr("key1", "value1")
m.PutStr("key2", "value2")
return m
}(),
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
result := MapToAttributes(test.tags)
assert.Equal(t, test.expected, result)
})
}
}
84 changes: 42 additions & 42 deletions internal/storage/integration/fixtures/queries.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
"OperationName": "",
"Tags": {
"sameplacetag1":"sameplacevalue",
"sameplacetag2":"123",
"sameplacetag3":"72.5",
"sameplacetag4":"true"
"sameplacetag2":123,
"sameplacetag3":72.5,
"sameplacetag4":true
},
"StartTimeMin": "2017-01-26T15:46:31.639875Z",
"StartTimeMax": "2017-01-26T17:46:31.639875Z",
Expand All @@ -25,9 +25,9 @@
"OperationName": "",
"Tags": {
"sameplacetag1":"sameplacevalue",
"sameplacetag2":"123",
"sameplacetag3":"72.5",
"sameplacetag4":"true"
"sameplacetag2":123,
"sameplacetag3":72.5,
"sameplacetag4":true
},
"StartTimeMin": "2017-01-26T15:46:31.639875Z",
"StartTimeMax": "2017-01-26T17:46:31.639875Z",
Expand All @@ -44,9 +44,9 @@
"OperationName": "",
"Tags": {
"sameplacetag1":"sameplacevalue",
"sameplacetag2":"123",
"sameplacetag3":"72.5",
"sameplacetag4":"true"
"sameplacetag2":123,
"sameplacetag3":72.5,
"sameplacetag4":true
},
"StartTimeMin": "2017-01-26T15:46:31.639875Z",
"StartTimeMax": "2017-01-26T17:46:31.639875Z",
Expand All @@ -63,9 +63,9 @@
"OperationName": "",
"Tags": {
"sameplacetag1":"sameplacevalue",
"sameplacetag2":"123",
"sameplacetag3":"72.5",
"sameplacetag4":"true"
"sameplacetag2":123,
"sameplacetag3":72.5,
"sameplacetag4":true
},
"StartTimeMin": "2017-01-26T15:46:31.639875Z",
"StartTimeMax": "2017-01-26T17:46:31.639875Z",
Expand Down Expand Up @@ -180,9 +180,9 @@
"OperationName": "query12-operation",
"Tags": {
"sameplacetag1":"sameplacevalue",
"sameplacetag2":"123",
"sameplacetag3":"72.5",
"sameplacetag4":"true"
"sameplacetag2":123,
"sameplacetag3":72.5,
"sameplacetag4":true
},
"StartTimeMin": "2017-01-26T15:46:31.639875Z",
"StartTimeMax": "2017-01-26T17:46:31.639875Z",
Expand All @@ -199,9 +199,9 @@
"OperationName": "query13-operation",
"Tags": {
"sameplacetag1":"sameplacevalue",
"sameplacetag2":"123",
"sameplacetag3":"72.5",
"sameplacetag4":"true"
"sameplacetag2":123,
"sameplacetag3":72.5,
"sameplacetag4":true
},
"StartTimeMin": "2017-01-26T15:46:31.639875Z",
"StartTimeMax": "2017-01-26T17:46:31.639875Z",
Expand All @@ -218,9 +218,9 @@
"OperationName": "query14-operation",
"Tags": {
"sameplacetag1":"sameplacevalue",
"sameplacetag2":"123",
"sameplacetag3":"72.5",
"sameplacetag4":"true"
"sameplacetag2":123,
"sameplacetag3":72.5,
"sameplacetag4":true
},
"StartTimeMin": "2017-01-26T15:46:31.639875Z",
"StartTimeMax": "2017-01-26T17:46:31.639875Z",
Expand All @@ -237,9 +237,9 @@
"OperationName": "",
"Tags": {
"sameplacetag1":"sameplacevalue",
"sameplacetag2":"123",
"sameplacetag3":"72.5",
"sameplacetag4":"true"
"sameplacetag2":123,
"sameplacetag3":72.5,
"sameplacetag4":true
},
"StartTimeMin": "2017-01-26T15:46:31.639875Z",
"StartTimeMax": "2017-01-26T17:46:31.639875Z",
Expand All @@ -256,9 +256,9 @@
"OperationName": "",
"Tags": {
"sameplacetag1":"sameplacevalue",
"sameplacetag2":"123",
"sameplacetag3":"72.5",
"sameplacetag4":"true"
"sameplacetag2":123,
"sameplacetag3":72.5,
"sameplacetag4":true
},
"StartTimeMin": "2017-01-26T15:46:31.639875Z",
"StartTimeMax": "2017-01-26T17:46:31.639875Z",
Expand All @@ -275,9 +275,9 @@
"OperationName": "query17-operation",
"Tags": {
"sameplacetag1":"sameplacevalue",
"sameplacetag2":"123",
"sameplacetag3":"72.5",
"sameplacetag4":"true"
"sameplacetag2":123,
"sameplacetag3":72.5,
"sameplacetag4":true
},
"StartTimeMin": "2017-01-26T15:46:31.639875Z",
"StartTimeMax": "2017-01-26T17:46:31.639875Z",
Expand All @@ -294,9 +294,9 @@
"OperationName": "query18-operation",
"Tags": {
"sameplacetag1":"sameplacevalue",
"sameplacetag2":"123",
"sameplacetag3":"72.5",
"sameplacetag4":"true"
"sameplacetag2":123,
"sameplacetag3":72.5,
"sameplacetag4":true
},
"StartTimeMin": "2017-01-26T15:46:31.639875Z",
"StartTimeMax": "2017-01-26T17:46:31.639875Z",
Expand All @@ -313,9 +313,9 @@
"OperationName": "query19-operation",
"Tags": {
"sameplacetag1":"sameplacevalue",
"sameplacetag2":"123",
"sameplacetag3":"72.5",
"sameplacetag4":"true"
"sameplacetag2":123,
"sameplacetag3":72.5,
"sameplacetag4":true
},
"StartTimeMin": "2017-01-26T15:46:31.639875Z",
"StartTimeMax": "2017-01-26T17:46:31.639875Z",
Expand All @@ -332,9 +332,9 @@
"OperationName": "",
"Tags": {
"sameplacetag1":"sameplacevalue",
"sameplacetag2":"123",
"sameplacetag3":"72.5",
"sameplacetag4":"true"
"sameplacetag2":123,
"sameplacetag3":72.5,
"sameplacetag4":true
},
"StartTimeMin": "2017-01-26T15:46:31.639875Z",
"StartTimeMax": "2017-01-26T17:46:31.639875Z",
Expand All @@ -351,9 +351,9 @@
"OperationName": "",
"Tags": {
"sameplacetag1":"sameplacevalue",
"sameplacetag2":"123",
"sameplacetag3":"72.5",
"sameplacetag4":"true"
"sameplacetag2":123,
"sameplacetag3":72.5,
"sameplacetag4":true
},
"StartTimeMin": "2017-01-26T15:46:31.639875Z",
"StartTimeMax": "2017-01-26T17:46:31.639875Z",
Expand Down
Loading

0 comments on commit 043b826

Please sign in to comment.