From ab442668c1a39977284d14f9f49edcfd6ae457ad Mon Sep 17 00:00:00 2001 From: Varkeychan Jacob Date: Sat, 11 Nov 2023 13:42:42 +0530 Subject: [PATCH 1/4] adding iterator interface --- instrumentation/opentelemetry/span.go | 32 ++++++++++++++---- instrumentation/opentelemetry/span_test.go | 13 +++++--- sdk/internal/mock/span.go | 39 +++++++++++++++++++--- sdk/span.go | 7 +++- 4 files changed, 74 insertions(+), 17 deletions(-) diff --git a/instrumentation/opentelemetry/span.go b/instrumentation/opentelemetry/span.go index 164e907..9efc668 100644 --- a/instrumentation/opentelemetry/span.go +++ b/instrumentation/opentelemetry/span.go @@ -15,6 +15,27 @@ import ( "go.opentelemetry.io/otel/trace" ) +var _ sdk.Iterator = (*Iterator)(nil) + +type Iterator struct { + size int + curr int + attrs []attribute.KeyValue +} + +func (i *Iterator) HasNext() bool { + return i.curr < i.size +} + +func (i *Iterator) Next() sdk.Attribute { + attr := sdk.Attribute{ + Key: string(i.attrs[i.curr].Key), + Value: i.attrs[i.curr].Value.AsInterface(), + } + i.curr++ + return attr +} + var _ sdk.AttributeList = (*AttributeList)(nil) type AttributeList struct { @@ -31,13 +52,12 @@ func (l *AttributeList) GetValue(key string) interface{} { return nil } -func (l *AttributeList) GetAll() []sdk.Attribute { - size := len(l.attrs) - attributes := make([]sdk.Attribute, size) - for i := 0; i < size; i++ { - attributes[i] = sdk.Attribute{Key: string(l.attrs[i].Key), Value: l.attrs[i].Value.AsInterface()} +func (l *AttributeList) GetIterator() sdk.Iterator { + return &Iterator{ + size: len(l.attrs), + curr: 0, + attrs: l.attrs, } - return attributes } var _ sdk.Span = (*Span)(nil) diff --git a/instrumentation/opentelemetry/span_test.go b/instrumentation/opentelemetry/span_test.go index c1c9764..898bc10 100644 --- a/instrumentation/opentelemetry/span_test.go +++ b/instrumentation/opentelemetry/span_test.go @@ -110,7 +110,7 @@ func TestGetAttributes(t *testing.T) { assert.Equal(t, nil, attrs.GetValue("non_existent")) } -func TestGetAllAttributes(t *testing.T) { +func TestGetIterator(t *testing.T) { sampler := sdktrace.AlwaysSample() tp := sdktrace.NewTracerProvider( sdktrace.WithSampler(sampler), @@ -119,15 +119,18 @@ func TestGetAllAttributes(t *testing.T) { _, s, _ := StartSpan(context.Background(), "test_span", &sdk.SpanOptions{}) s.SetAttribute("k1", "v1") s.SetAttribute("k2", 200) - attrs := s.GetAttributes().GetAll() + itr := s.GetAttributes().GetIterator() - // service.instance.id is added implicitly in StartSpan so 3 attributes will be present. - assert.Equal(t, 3, len(attrs)) - for _, attr := range attrs { + numAttrs := 0 + for itr.HasNext() { + numAttrs++ + attr := itr.Next() if attr.Key == "k1" { assert.Equal(t, "v1", fmt.Sprintf("%v", attr.Value)) } else if attr.Key == "k2" { assert.Equal(t, "200", fmt.Sprintf("%v", attr.Value)) } } + // service.instance.id is added implicitly in StartSpan so 3 attributes will be present. + assert.Equal(t, 3, numAttrs) } diff --git a/sdk/internal/mock/span.go b/sdk/internal/mock/span.go index 08f2047..9e6938a 100644 --- a/sdk/internal/mock/span.go +++ b/sdk/internal/mock/span.go @@ -19,6 +19,29 @@ type Status struct { Message string } +var _ sdk.Iterator = (*Iterator)(nil) + +type Iterator struct { + size int + curr int + keySet []string + attrs map[string]interface{} +} + +func (i *Iterator) HasNext() bool { + return i.curr < i.size +} + +func (i *Iterator) Next() sdk.Attribute { + key := i.keySet[i.curr] + attr := sdk.Attribute{ + Key: key, + Value: i.attrs[key], + } + i.curr++ + return attr +} + var _ sdk.AttributeList = (*AttributeList)(nil) type AttributeList struct { @@ -29,15 +52,21 @@ func (l *AttributeList) GetValue(key string) interface{} { return l.attrs[key] } -func (l *AttributeList) GetAll() []sdk.Attribute { +func (l *AttributeList) GetIterator() sdk.Iterator { - attributes := make([]sdk.Attribute, len(l.attrs)) + keySet := make([]string, len(l.attrs)) i := 0 - for key, value := range l.attrs { - attributes[i] = sdk.Attribute{Key: key, Value: value} + for k := range l.attrs { + keySet[i] = k i++ } - return attributes + + return &Iterator{ + size: len(l.attrs), + curr: 0, + keySet: keySet, + attrs: l.attrs, + } } var _ sdk.Span = &Span{} diff --git a/sdk/span.go b/sdk/span.go index 54dc8c7..bbac1ee 100644 --- a/sdk/span.go +++ b/sdk/span.go @@ -10,9 +10,14 @@ type Attribute struct { Value interface{} } +type Iterator interface { + HasNext() bool + Next() Attribute +} + type AttributeList interface { GetValue(key string) interface{} - GetAll() []Attribute + GetIterator() Iterator } // Span is an interface that accepts attributes and can be From 9d1f8aa6950c28c800b8f12d86d878c1245412c5 Mon Sep 17 00:00:00 2001 From: Varkeychan Jacob Date: Mon, 13 Nov 2023 01:49:20 +0530 Subject: [PATCH 2/4] adding new iterator --- instrumentation/opentelemetry/span.go | 8 ++++++++ instrumentation/opentelemetry/span_test.go | 24 ++++++++++++++++++++++ sdk/span.go | 1 + 3 files changed, 33 insertions(+) diff --git a/instrumentation/opentelemetry/span.go b/instrumentation/opentelemetry/span.go index 9efc668..c496b6a 100644 --- a/instrumentation/opentelemetry/span.go +++ b/instrumentation/opentelemetry/span.go @@ -60,6 +60,14 @@ func (l *AttributeList) GetIterator() sdk.Iterator { } } +func (l *AttributeList) IterateItems(yield func(attr sdk.Attribute) bool) { + for _, attr := range l.attrs { + if !yield(sdk.Attribute{Key: string(attr.Key), Value: attr.Value.AsInterface()}) { + return + } + } +} + var _ sdk.Span = (*Span)(nil) type Span struct { diff --git a/instrumentation/opentelemetry/span_test.go b/instrumentation/opentelemetry/span_test.go index 898bc10..b86b219 100644 --- a/instrumentation/opentelemetry/span_test.go +++ b/instrumentation/opentelemetry/span_test.go @@ -134,3 +134,27 @@ func TestGetIterator(t *testing.T) { // service.instance.id is added implicitly in StartSpan so 3 attributes will be present. assert.Equal(t, 3, numAttrs) } + +func TestIterateItems(t *testing.T) { + sampler := sdktrace.AlwaysSample() + tp := sdktrace.NewTracerProvider( + sdktrace.WithSampler(sampler), + ) + otel.SetTracerProvider(tp) + _, s, _ := StartSpan(context.Background(), "test_span", &sdk.SpanOptions{}) + s.SetAttribute("k1", "v1") + s.SetAttribute("k2", 200) + + numAttrs := 0 + s.GetAttributes().IterateItems(func(attr sdk.Attribute) bool { + if attr.Key == "k1" { + assert.Equal(t, "v1", fmt.Sprintf("%v", attr.Value)) + } else if attr.Key == "k2" { + assert.Equal(t, "200", fmt.Sprintf("%v", attr.Value)) + } + numAttrs++ + return true + }) + // service.instance.id is added implicitly in StartSpan so 3 attributes will be present. + assert.Equal(t, 3, numAttrs) +} diff --git a/sdk/span.go b/sdk/span.go index bbac1ee..abeb516 100644 --- a/sdk/span.go +++ b/sdk/span.go @@ -18,6 +18,7 @@ type Iterator interface { type AttributeList interface { GetValue(key string) interface{} GetIterator() Iterator + IterateItems(yield func(attr Attribute) bool) } // Span is an interface that accepts attributes and can be From d4bb0a54960f94478d5fb927dfdb45ad17bad263 Mon Sep 17 00:00:00 2001 From: Varkeychan Jacob Date: Mon, 13 Nov 2023 09:15:10 +0530 Subject: [PATCH 3/4] adding iterator --- instrumentation/opentelemetry/span.go | 33 ++--------------- instrumentation/opentelemetry/span_test.go | 37 ++++--------------- sdk/internal/mock/span.go | 42 +++------------------- sdk/span.go | 13 +------ 4 files changed, 14 insertions(+), 111 deletions(-) diff --git a/instrumentation/opentelemetry/span.go b/instrumentation/opentelemetry/span.go index c496b6a..79e7de0 100644 --- a/instrumentation/opentelemetry/span.go +++ b/instrumentation/opentelemetry/span.go @@ -15,27 +15,6 @@ import ( "go.opentelemetry.io/otel/trace" ) -var _ sdk.Iterator = (*Iterator)(nil) - -type Iterator struct { - size int - curr int - attrs []attribute.KeyValue -} - -func (i *Iterator) HasNext() bool { - return i.curr < i.size -} - -func (i *Iterator) Next() sdk.Attribute { - attr := sdk.Attribute{ - Key: string(i.attrs[i.curr].Key), - Value: i.attrs[i.curr].Value.AsInterface(), - } - i.curr++ - return attr -} - var _ sdk.AttributeList = (*AttributeList)(nil) type AttributeList struct { @@ -52,17 +31,9 @@ func (l *AttributeList) GetValue(key string) interface{} { return nil } -func (l *AttributeList) GetIterator() sdk.Iterator { - return &Iterator{ - size: len(l.attrs), - curr: 0, - attrs: l.attrs, - } -} - -func (l *AttributeList) IterateItems(yield func(attr sdk.Attribute) bool) { +func (l *AttributeList) Iterate(yield func(key string, value interface{}) bool) { for _, attr := range l.attrs { - if !yield(sdk.Attribute{Key: string(attr.Key), Value: attr.Value.AsInterface()}) { + if !yield(string(attr.Key), attr.Value.AsInterface()) { return } } diff --git a/instrumentation/opentelemetry/span_test.go b/instrumentation/opentelemetry/span_test.go index b86b219..2e8d01e 100644 --- a/instrumentation/opentelemetry/span_test.go +++ b/instrumentation/opentelemetry/span_test.go @@ -110,7 +110,7 @@ func TestGetAttributes(t *testing.T) { assert.Equal(t, nil, attrs.GetValue("non_existent")) } -func TestGetIterator(t *testing.T) { +func TestIterate(t *testing.T) { sampler := sdktrace.AlwaysSample() tp := sdktrace.NewTracerProvider( sdktrace.WithSampler(sampler), @@ -119,38 +119,13 @@ func TestGetIterator(t *testing.T) { _, s, _ := StartSpan(context.Background(), "test_span", &sdk.SpanOptions{}) s.SetAttribute("k1", "v1") s.SetAttribute("k2", 200) - itr := s.GetAttributes().GetIterator() numAttrs := 0 - for itr.HasNext() { - numAttrs++ - attr := itr.Next() - if attr.Key == "k1" { - assert.Equal(t, "v1", fmt.Sprintf("%v", attr.Value)) - } else if attr.Key == "k2" { - assert.Equal(t, "200", fmt.Sprintf("%v", attr.Value)) - } - } - // service.instance.id is added implicitly in StartSpan so 3 attributes will be present. - assert.Equal(t, 3, numAttrs) -} - -func TestIterateItems(t *testing.T) { - sampler := sdktrace.AlwaysSample() - tp := sdktrace.NewTracerProvider( - sdktrace.WithSampler(sampler), - ) - otel.SetTracerProvider(tp) - _, s, _ := StartSpan(context.Background(), "test_span", &sdk.SpanOptions{}) - s.SetAttribute("k1", "v1") - s.SetAttribute("k2", 200) - - numAttrs := 0 - s.GetAttributes().IterateItems(func(attr sdk.Attribute) bool { - if attr.Key == "k1" { - assert.Equal(t, "v1", fmt.Sprintf("%v", attr.Value)) - } else if attr.Key == "k2" { - assert.Equal(t, "200", fmt.Sprintf("%v", attr.Value)) + s.GetAttributes().Iterate(func(key string, value interface{}) bool { + if key == "k1" { + assert.Equal(t, "v1", fmt.Sprintf("%v", value)) + } else if key == "k2" { + assert.Equal(t, "200", fmt.Sprintf("%v", value)) } numAttrs++ return true diff --git a/sdk/internal/mock/span.go b/sdk/internal/mock/span.go index 9e6938a..e8b279f 100644 --- a/sdk/internal/mock/span.go +++ b/sdk/internal/mock/span.go @@ -19,29 +19,6 @@ type Status struct { Message string } -var _ sdk.Iterator = (*Iterator)(nil) - -type Iterator struct { - size int - curr int - keySet []string - attrs map[string]interface{} -} - -func (i *Iterator) HasNext() bool { - return i.curr < i.size -} - -func (i *Iterator) Next() sdk.Attribute { - key := i.keySet[i.curr] - attr := sdk.Attribute{ - Key: key, - Value: i.attrs[key], - } - i.curr++ - return attr -} - var _ sdk.AttributeList = (*AttributeList)(nil) type AttributeList struct { @@ -52,20 +29,11 @@ func (l *AttributeList) GetValue(key string) interface{} { return l.attrs[key] } -func (l *AttributeList) GetIterator() sdk.Iterator { - - keySet := make([]string, len(l.attrs)) - i := 0 - for k := range l.attrs { - keySet[i] = k - i++ - } - - return &Iterator{ - size: len(l.attrs), - curr: 0, - keySet: keySet, - attrs: l.attrs, +func (l *AttributeList) Iterate(yield func(key string, value interface{}) bool) { + for key, value := range l.attrs { + if !yield(key, value) { + return + } } } diff --git a/sdk/span.go b/sdk/span.go index abeb516..e0b7f3a 100644 --- a/sdk/span.go +++ b/sdk/span.go @@ -5,20 +5,9 @@ import ( "time" ) -type Attribute struct { - Key string - Value interface{} -} - -type Iterator interface { - HasNext() bool - Next() Attribute -} - type AttributeList interface { GetValue(key string) interface{} - GetIterator() Iterator - IterateItems(yield func(attr Attribute) bool) + Iterate(yield func(key string, value interface{}) bool) } // Span is an interface that accepts attributes and can be From 17f0db7c4d1179fe2e27d4f99112d5351d2eb476 Mon Sep 17 00:00:00 2001 From: Varkeychan Jacob Date: Mon, 13 Nov 2023 18:44:14 +0530 Subject: [PATCH 4/4] adding comment --- sdk/span.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sdk/span.go b/sdk/span.go index e0b7f3a..8f34577 100644 --- a/sdk/span.go +++ b/sdk/span.go @@ -7,6 +7,9 @@ import ( type AttributeList interface { GetValue(key string) interface{} + + // Iterate loops through the attributes list and applies the yield function on each attribute. + // If the yield function returns false, we exit the loop. Iterate(yield func(key string, value interface{}) bool) }