Skip to content

[refactor] Change TraceQueryParams to accept typed attributes #6780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 27, 2025
Merged

[refactor] Change TraceQueryParams to accept typed attributes #6780

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
75fd131
Add Utility Functions For Converting Between Map And Attributes
mahadzaryab1 Feb 27, 2025
8d9a489
Use Typed Attributes In TraceQueryParams
mahadzaryab1 Feb 27, 2025
8a365ba
Run Formatter
mahadzaryab1 Feb 27, 2025
dea92a0
Use Typed Attributes In Test
mahadzaryab1 Feb 27, 2025
98a99eb
Fix Unit Tests
mahadzaryab1 Feb 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cmd/jaeger/internal/integration/trace_reader.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading