Skip to content

Commit

Permalink
Clean up span.kind tag handling
Browse files Browse the repository at this point in the history
Signed-off-by: Yuri Shkuro <[email protected]>
  • Loading branch information
yurishkuro committed Dec 20, 2024
1 parent 1e5a8ff commit 03b768f
Show file tree
Hide file tree
Showing 15 changed files with 81 additions and 72 deletions.
2 changes: 1 addition & 1 deletion cmd/agent/app/reporter/grpc/reporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func TestReporter_EmitZipkinBatch(t *testing.T) {
expected: model.Batch{
Spans: []*model.Span{{
TraceID: model.NewTraceID(0, 1), SpanID: model.NewSpanID(2), OperationName: "jonatan", Duration: time.Microsecond * 1,
Tags: model.KeyValues{{Key: "span.kind", VStr: "client", VType: model.StringType}},
Tags: model.KeyValues{model.SpanKindTag(model.SpanKindClient)},
Process: &model.Process{ServiceName: "spring"}, StartTime: tm.UTC(),
}},
},
Expand Down
12 changes: 6 additions & 6 deletions cmd/anonymizer/app/anonymizer/anonymizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ import (
)

var allowedTags = map[string]bool{
"error": true,
"span.kind": true,
"http.method": true,
"http.status_code": true,
"sampler.type": true,
"sampler.param": true,
"error": true,
"http.method": true,
"http.status_code": true,
model.SpanKindKey: true,
model.SamplerTypeKey: true,
model.SamplerParamKey: true,
}

const PermUserRW = 0o600 // Read-write for owner only
Expand Down
2 changes: 1 addition & 1 deletion cmd/anonymizer/app/uiconv/extractor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestExtractorTraceSuccess(t *testing.T) {

for i := range trace.Data {
for j := range trace.Data[i].Spans {
assert.Equal(t, "span.kind", trace.Data[i].Spans[j].Tags[0].Key)
assert.Equal(t, model.SpanKindKey, trace.Data[i].Spans[j].Tags[0].Key)
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion cmd/anonymizer/app/uiconv/module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"os"
"testing"

"github.com/jaegertracing/jaeger/model"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap"
Expand All @@ -30,7 +31,7 @@ func TestModule_TraceSuccess(t *testing.T) {

for i := range trace.Data {
for j := range trace.Data[i].Spans {
assert.Equal(t, "span.kind", trace.Data[i].Spans[j].Tags[0].Key)
assert.Equal(t, model.SpanKindKey, trace.Data[i].Spans[j].Tags[0].Key)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/query/app/apiv3/gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func makeTestTrace() (*model.Trace, spanstore.GetTraceParameters) {
SpanID: model.NewSpanID(180),
OperationName: "foobar",
Tags: []model.KeyValue{
model.String("span.kind", "server"),
model.SpanKindTag(model.SpanKindServer),
model.Bool("error", true),
},
},
Expand Down
7 changes: 3 additions & 4 deletions model/adjuster/span_hash_deduper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/otel/trace"

"github.com/jaegertracing/jaeger/model"
)
Expand All @@ -22,15 +21,15 @@ func newDuplicatedSpansTrace() *model.Trace {
SpanID: clientSpanID,
Tags: model.KeyValues{
// span.kind = server
model.String(keySpanKind, trace.SpanKindServer.String()),
model.SpanKindTag(model.SpanKindServer),
},
},
{
TraceID: traceID,
SpanID: clientSpanID, // shared span ID
Tags: model.KeyValues{
// span.kind = server
model.String(keySpanKind, trace.SpanKindServer.String()),
model.SpanKindTag(model.SpanKindServer),
},
},
{
Expand All @@ -52,7 +51,7 @@ func newUniqueSpansTrace() *model.Trace {
SpanID: anotherSpanID,
Tags: model.KeyValues{
// span.kind = server
model.String(keySpanKind, trace.SpanKindServer.String()),
model.SpanKindTag(model.SpanKindServer),
},
},
{
Expand Down
6 changes: 2 additions & 4 deletions model/adjuster/zipkin_span_id_uniquify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,13 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/otel/trace"

"github.com/jaegertracing/jaeger/model"
)

var (
clientSpanID = model.NewSpanID(1)
anotherSpanID = model.NewSpanID(11)
keySpanKind = "span.kind"
)

func newZipkinTrace() *model.Trace {
Expand All @@ -30,7 +28,7 @@ func newZipkinTrace() *model.Trace {
SpanID: clientSpanID,
Tags: model.KeyValues{
// span.kind = client
model.String(keySpanKind, trace.SpanKindClient.String()),
model.SpanKindTag(model.SpanKindClient),
},
},
{
Expand All @@ -39,7 +37,7 @@ func newZipkinTrace() *model.Trace {
SpanID: clientSpanID, // shared span ID
Tags: model.KeyValues{
// span.kind = server
model.String(keySpanKind, trace.SpanKindServer.String()),
model.SpanKindTag(model.SpanKindServer),
},
},
{
Expand Down
7 changes: 3 additions & 4 deletions model/converter/thrift/zipkin/to_domain.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
const (
// UnknownServiceName is serviceName we give to model.Process if we cannot find it anywhere in a Zipkin span
UnknownServiceName = "unknown-service-name"
keySpanKind = "span.kind"
component = "component"
peerservice = "peer.service"
peerHostIPv4 = "peer.ipv4"
Expand Down Expand Up @@ -166,15 +165,15 @@ func (td toDomain) transformSpan(zSpan *zipkincore.Span) []*model.Span {
}
// if the first span is a client span we create server span and vice-versa.
if result[0].IsRPCClient() {
s.Tags = []model.KeyValue{model.String(keySpanKind, trace.SpanKindServer.String())}
s.Tags = []model.KeyValue{model.SpanKindTag(model.SpanKindServer)}
//nolint: gosec // G115
s.StartTime = model.EpochMicrosecondsAsTime(uint64(sr.Timestamp))
if ss := td.findAnnotation(zSpan, zipkincore.SERVER_SEND); ss != nil {
//nolint: gosec // G115
s.Duration = model.MicrosecondsAsDuration(uint64(ss.Timestamp - sr.Timestamp))
}
} else {
s.Tags = []model.KeyValue{model.String(keySpanKind, trace.SpanKindClient.String())}
s.Tags = []model.KeyValue{model.SpanKindTag(model.SpanKindClient)}
//nolint: gosec // G115
s.StartTime = model.EpochMicrosecondsAsTime(uint64(cs.Timestamp))
if cr := td.findAnnotation(zSpan, zipkincore.CLIENT_RECV); cr != nil {
Expand Down Expand Up @@ -393,7 +392,7 @@ func (toDomain) getLogFields(annotation *zipkincore.Annotation) []model.KeyValue
func (toDomain) getSpanKindTag(annotations []*zipkincore.Annotation) (model.KeyValue, bool) {
for _, a := range annotations {
if spanKind, ok := coreAnnotations[a.Value]; ok {
return model.String(keySpanKind, spanKind), true
return model.SpanKindTag(model.SpanKind(spanKind)), true
}
}
return model.KeyValue{}, false
Expand Down
25 changes: 12 additions & 13 deletions model/converter/thrift/zipkin/to_domain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"github.com/kr/pretty"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/otel/trace"

"github.com/jaegertracing/jaeger/model"
z "github.com/jaegertracing/jaeger/thrift-gen/zipkincore"
Expand Down Expand Up @@ -107,30 +106,30 @@ func TestToDomainMultipleSpanKinds(t *testing.T) {
json string
tagFirstKey string
tagSecondKey string
tagFirstVal trace.SpanKind
tagSecondVal trace.SpanKind
tagFirstVal model.SpanKind
tagSecondVal model.SpanKind
}{
{
json: `[{ "trace_id": -1, "id": 31, "annotations": [
{"value": "cs", "host": {"service_name": "bar", "ipv4": 23456}},
{"value": "sr", "timestamp": 1, "host": {"service_name": "bar", "ipv4": 23456}},
{"value": "ss", "timestamp": 2, "host": {"service_name": "bar", "ipv4": 23456}}
]}]`,
tagFirstKey: keySpanKind,
tagSecondKey: keySpanKind,
tagFirstVal: trace.SpanKindClient,
tagSecondVal: trace.SpanKindServer,
tagFirstKey: model.SpanKindKey,
tagSecondKey: model.SpanKindKey,
tagFirstVal: model.SpanKindClient,
tagSecondVal: model.SpanKindServer,
},
{
json: `[{ "trace_id": -1, "id": 31, "annotations": [
{"value": "sr", "host": {"service_name": "bar", "ipv4": 23456}},
{"value": "cs", "timestamp": 1, "host": {"service_name": "bar", "ipv4": 23456}},
{"value": "cr", "timestamp": 2, "host": {"service_name": "bar", "ipv4": 23456}}
]}]`,
tagFirstKey: keySpanKind,
tagSecondKey: keySpanKind,
tagFirstVal: trace.SpanKindServer,
tagSecondVal: trace.SpanKindClient,
tagFirstKey: model.SpanKindKey,
tagSecondKey: model.SpanKindKey,
tagFirstVal: model.SpanKindServer,
tagSecondVal: model.SpanKindClient,
},
}

Expand All @@ -141,12 +140,12 @@ func TestToDomainMultipleSpanKinds(t *testing.T) {
assert.Len(t, trc.Spans, 2)
assert.Len(t, trc.Spans[0].Tags, 1)
assert.Equal(t, test.tagFirstKey, trc.Spans[0].Tags[0].Key)
assert.Equal(t, test.tagFirstVal.String(), trc.Spans[0].Tags[0].VStr)
assert.EqualValues(t, test.tagFirstVal, trc.Spans[0].Tags[0].VStr)

assert.Len(t, trc.Spans[1].Tags, 1)
assert.Equal(t, test.tagSecondKey, trc.Spans[1].Tags[0].Key)
assert.Equal(t, time.Duration(1000), trc.Spans[1].Duration)
assert.Equal(t, test.tagSecondVal.String(), trc.Spans[1].Tags[0].VStr)
assert.EqualValues(t, test.tagSecondVal, trc.Spans[1].Tags[0].VStr)
}
}

Expand Down
24 changes: 24 additions & 0 deletions model/keyvalue.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,32 @@ const (
Float64Type = ValueType_FLOAT64
// BinaryType indicates the value is binary blob stored as a byte array
BinaryType = ValueType_BINARY

SpanKindKey = "span.kind"
SamplerTypeKey = "sampler.type"
SamplerParamKey = "sampler.param"
)

type SpanKind string

const (
SpanKindClient SpanKind = "client"
SpanKindServer SpanKind = "server"
SpanKindProducer SpanKind = "producer"
SpanKindConsumer SpanKind = "consumer"
SpanKindInternal SpanKind = "internal"
SpanKindUnspecified SpanKind = ""
)

func SpanKindFromString(kind string) (SpanKind, error) {
switch SpanKind(kind) {
case SpanKindClient, SpanKindServer, SpanKindProducer, SpanKindConsumer, SpanKindInternal, SpanKindUnspecified:
return SpanKind(kind), nil
default:
return SpanKindUnspecified, fmt.Errorf("unknown span kind %q", kind)
}
}

// KeyValues is a type alias that exposes convenience functions like Sort, FindByKey.
type KeyValues []KeyValue

Expand Down
39 changes: 15 additions & 24 deletions model/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"io"
"strconv"

"go.opentelemetry.io/otel/trace"
"go.uber.org/zap"
)

Expand All @@ -28,23 +27,11 @@ const (
DebugFlag = Flags(2)
// FirehoseFlag is the bit in Flags in order to define a span as a firehose span
FirehoseFlag = Flags(8)

keySamplerType = "sampler.type"
keySpanKind = "span.kind"
keySamplerParam = "sampler.param"
)

// Flags is a bit map of flags for a span
type Flags uint32

var toSpanKind = map[string]trace.SpanKind{
"client": trace.SpanKindClient,
"server": trace.SpanKindServer,
"producer": trace.SpanKindProducer,
"consumer": trace.SpanKindConsumer,
"internal": trace.SpanKindInternal,
}

var toSamplerType = map[string]SamplerType{
"unrecognized": SamplerTypeUnrecognized,
"probabilistic": SamplerTypeProbabilistic,
Expand All @@ -70,6 +57,10 @@ func (s SamplerType) String() string {
}
}

func SpanKindTag(kind SpanKind) KeyValue {
return String(SpanKindKey, string(kind))
}

// Hash implements Hash from Hashable.
func (s *Span) Hash(w io.Writer) (err error) {
// gob is not the most efficient way, but it ensures we don't miss any fields.
Expand All @@ -79,27 +70,27 @@ func (s *Span) Hash(w io.Writer) (err error) {
}

// HasSpanKind returns true if the span has a `span.kind` tag set to `kind`.
func (s *Span) HasSpanKind(kind trace.SpanKind) bool {
if tag, ok := KeyValues(s.Tags).FindByKey(keySpanKind); ok {
return tag.AsString() == kind.String()
func (s *Span) HasSpanKind(kind SpanKind) bool {
if tag, ok := KeyValues(s.Tags).FindByKey(SpanKindKey); ok {
return tag.AsString() == string(kind)
}
return false
}

// GetSpanKind returns value of `span.kind` tag and whether the tag can be found
func (s *Span) GetSpanKind() (spanKind trace.SpanKind, found bool) {
if tag, ok := KeyValues(s.Tags).FindByKey(keySpanKind); ok {
if kind, ok := toSpanKind[tag.AsString()]; ok {
func (s *Span) GetSpanKind() (spanKind SpanKind, found bool) {
if tag, ok := KeyValues(s.Tags).FindByKey(SpanKindKey); ok {
if kind, err := SpanKindFromString(tag.AsString()); err == nil {
return kind, true
}
}
return trace.SpanKindUnspecified, false
return SpanKindUnspecified, false
}

// GetSamplerType returns the sampler type for span
func (s *Span) GetSamplerType() SamplerType {
// There's no corresponding opentelemetry tag label corresponding to sampler.type
if tag, ok := KeyValues(s.Tags).FindByKey(keySamplerType); ok {
if tag, ok := KeyValues(s.Tags).FindByKey(SamplerTypeKey); ok {
if s, ok := toSamplerType[tag.VStr]; ok {
return s
}
Expand All @@ -110,13 +101,13 @@ func (s *Span) GetSamplerType() SamplerType {
// IsRPCClient returns true if the span represents a client side of an RPC,
// as indicated by the `span.kind` tag set to `client`.
func (s *Span) IsRPCClient() bool {
return s.HasSpanKind(trace.SpanKindClient)
return s.HasSpanKind(SpanKindClient)
}

// IsRPCServer returns true if the span represents a server side of an RPC,
// as indicated by the `span.kind` tag set to `server`.
func (s *Span) IsRPCServer() bool {
return s.HasSpanKind(trace.SpanKindServer)
return s.HasSpanKind(SpanKindServer)
}

// NormalizeTimestamps changes all timestamps in this span to UTC.
Expand Down Expand Up @@ -168,7 +159,7 @@ func (s *Span) GetSamplerParams(logger *zap.Logger) (SamplerType, float64) {
if samplerType == SamplerTypeUnrecognized {
return SamplerTypeUnrecognized, 0
}
tag, ok := KeyValues(s.Tags).FindByKey(keySamplerParam)
tag, ok := KeyValues(s.Tags).FindByKey(SamplerParamKey)
if !ok {
return SamplerTypeUnrecognized, 0
}
Expand Down
Loading

0 comments on commit 03b768f

Please sign in to comment.