-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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.
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
Bump opentelemetry-collector to v0.14.0 #2617
Changes from all commits
64633e1
fc8f5d3
f6a0e29
a78e918
5b5e72e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,7 @@ | |
package esmodeltranslator | ||
|
||
import ( | ||
"encoding/hex" | ||
"errors" | ||
"fmt" | ||
"strconv" | ||
|
@@ -167,7 +168,7 @@ func toTime(nano pdata.TimestampUnixNano) time.Time { | |
} | ||
|
||
func references(links pdata.SpanLinkSlice, parentSpanID pdata.SpanID, traceID dbmodel.TraceID) ([]dbmodel.Reference, error) { | ||
parentSpanIDSet := len(parentSpanID.Bytes()) != 0 | ||
parentSpanIDSet := parentSpanID.IsValid() | ||
if !parentSpanIDSet && links.Len() == 0 { | ||
return emptyReferenceList, nil | ||
} | ||
|
@@ -222,24 +223,20 @@ func references(links pdata.SpanLinkSlice, parentSpanID pdata.SpanID, traceID db | |
} | ||
|
||
func convertSpanID(spanID pdata.SpanID) (dbmodel.SpanID, error) { | ||
spanIDInt, err := tracetranslator.BytesToUInt64SpanID(spanID.Bytes()) | ||
if err != nil { | ||
return "", err | ||
} | ||
if spanIDInt == 0 { | ||
if !spanID.IsValid() { | ||
return "", errZeroSpanID | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This feels misplaced to me. This kind of validation should've been done way earlier in the pipeline. The ES backend itself doesn't actually care if IDs are all zeroes, it would still work. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're definitely right, but practically having this error handled here can prevent some time of unnecessary debugging in case of zero IDs and missing validation earlier. I believe this error arises systematically, so anyway there will be collisions by ID and this will only implicitly indicate zero ID error. |
||
} | ||
return dbmodel.SpanID(fmt.Sprintf("%016x", spanIDInt)), nil | ||
src := spanID.Bytes() | ||
dst := make([]byte, hex.EncodedLen(len(src))) | ||
hex.Encode(dst, src[:]) | ||
return dbmodel.SpanID(dst), nil | ||
} | ||
|
||
func convertTraceID(traceID pdata.TraceID) (dbmodel.TraceID, error) { | ||
high, low, err := tracetranslator.BytesToUInt64TraceID(traceID.Bytes()) | ||
if err != nil { | ||
return "", err | ||
} | ||
if low == 0 && high == 0 { | ||
if !traceID.IsValid() { | ||
return "", errZeroTraceID | ||
} | ||
high, low := tracetranslator.BytesToUInt64TraceID(traceID.Bytes()) | ||
return dbmodel.TraceID(traceIDToString(high, low)), nil | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,6 @@ | |
package esmodeltranslator | ||
|
||
import ( | ||
"encoding/binary" | ||
"fmt" | ||
"testing" | ||
"time" | ||
|
@@ -30,8 +29,9 @@ import ( | |
) | ||
|
||
var ( | ||
traceID = pdata.NewTraceID([]byte("0123456789abcdef")) | ||
spanID = pdata.NewSpanID([]byte("01234567")) | ||
traceID = pdata.NewTraceID([16]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, | ||
0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F}) | ||
spanID = pdata.NewSpanID([8]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07}) | ||
) | ||
|
||
func TestAttributeToKeyValue(t *testing.T) { | ||
|
@@ -129,7 +129,9 @@ func TestConvertSpan(t *testing.T) { | |
span.Links().Resize(1) | ||
span.Links().At(0).InitEmpty() | ||
span.Links().At(0).SetSpanID(spanID) | ||
span.Links().At(0).SetTraceID(traceID) | ||
traceIDZeroHigh := pdata.NewTraceID([16]byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, | ||
0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07}) | ||
span.Links().At(0).SetTraceID(traceIDZeroHigh) | ||
|
||
c := &Translator{ | ||
tagKeysAsFields: map[string]bool{"toTagMap": true}, | ||
|
@@ -143,15 +145,15 @@ func TestConvertSpan(t *testing.T) { | |
Resource: resource, | ||
InstrumentationLibrary: traces.ResourceSpans().At(0).InstrumentationLibrarySpans().At(0).InstrumentationLibrary(), | ||
DBSpan: &dbmodel.Span{ | ||
TraceID: "30313233343536373839616263646566", | ||
SpanID: "3031323334353637", | ||
TraceID: "000102030405060708090a0b0c0d0e0f", | ||
SpanID: "0001020304050607", | ||
StartTime: 1000, | ||
Duration: 1000, | ||
OperationName: "root", | ||
StartTimeMillis: 1, | ||
Tags: []dbmodel.KeyValue{ | ||
{Key: "span.kind", Type: dbmodel.StringType, Value: "client"}, | ||
{Key: "status.code", Type: dbmodel.StringType, Value: "STATUS_CODE_CANCELLED"}, | ||
{Key: "status.code", Type: dbmodel.StringType, Value: "STATUS_CODE_OK"}, | ||
{Key: "error", Type: dbmodel.BoolType, Value: "true"}, | ||
{Key: "status.message", Type: dbmodel.StringType, Value: "messagetext"}, | ||
{Key: "foo", Type: dbmodel.BoolType, Value: "true"}, | ||
|
@@ -163,8 +165,8 @@ func TestConvertSpan(t *testing.T) { | |
{Key: "event", Value: "eventName", Type: dbmodel.StringType}, | ||
{Key: "foo", Value: "bar", Type: dbmodel.StringType}}, Timestamp: 500}}, | ||
References: []dbmodel.Reference{ | ||
{SpanID: "3031323334353637", TraceID: "30313233343536373839616263646566", RefType: dbmodel.ChildOf}, | ||
{SpanID: "3031323334353637", TraceID: "30313233343536373839616263646566", RefType: dbmodel.FollowsFrom}}, | ||
{SpanID: "0001020304050607", TraceID: "000102030405060708090a0b0c0d0e0f", RefType: dbmodel.ChildOf}, | ||
{SpanID: "0001020304050607", TraceID: "0001020304050607", RefType: dbmodel.FollowsFrom}}, | ||
Process: dbmodel.Process{ | ||
ServiceName: "myservice", | ||
Tags: []dbmodel.KeyValue{{Key: "num", Value: "16.66", Type: dbmodel.Float64Type}}, | ||
|
@@ -173,6 +175,12 @@ func TestConvertSpan(t *testing.T) { | |
}, spansData[0]) | ||
} | ||
|
||
func BenchmarkConvertSpanID(b *testing.B) { | ||
for i := 0; i < b.N; i++ { | ||
_, _ = convertSpanID(spanID) | ||
} | ||
} | ||
|
||
func TestSpanEmptyRef(t *testing.T) { | ||
traces := traces("myservice") | ||
span := addSpan(traces, "root", traceID, spanID) | ||
|
@@ -190,8 +198,8 @@ func TestSpanEmptyRef(t *testing.T) { | |
Resource: traces.ResourceSpans().At(0).Resource(), | ||
InstrumentationLibrary: traces.ResourceSpans().At(0).InstrumentationLibrarySpans().At(0).InstrumentationLibrary(), | ||
DBSpan: &dbmodel.Span{ | ||
TraceID: "30313233343536373839616263646566", | ||
SpanID: "3031323334353637", | ||
TraceID: "000102030405060708090a0b0c0d0e0f", | ||
SpanID: "0001020304050607", | ||
StartTime: 1000, | ||
Duration: 1000, | ||
OperationName: "root", | ||
|
@@ -215,26 +223,16 @@ func TestEmpty(t *testing.T) { | |
} | ||
|
||
func TestErrorIDs(t *testing.T) { | ||
zero64Bytes := make([]byte, 16) | ||
binary.LittleEndian.PutUint64(zero64Bytes, 0) | ||
binary.LittleEndian.PutUint64(zero64Bytes, 0) | ||
var zero64Bytes [16]byte | ||
var zero32Bytes [8]byte | ||
tests := []struct { | ||
spanID pdata.SpanID | ||
traceID pdata.TraceID | ||
err string | ||
}{ | ||
{ | ||
traceID: pdata.NewTraceID([]byte("invalid-%")), | ||
err: "TraceID does not have 16 bytes", | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure why these tests are removed - give the current code there would still be some kind of error returned from converters. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But TraceID is now exactly There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They are silently truncated according to this PR conversations open-telemetry/opentelemetry-collector#2001 |
||
{ | ||
traceID: traceID, | ||
spanID: pdata.NewSpanID([]byte("invalid-%")), | ||
err: "SpanID does not have 8 bytes", | ||
}, | ||
{ | ||
traceID: traceID, | ||
spanID: pdata.NewSpanID(zero64Bytes[:8]), | ||
spanID: pdata.NewSpanID(zero32Bytes), | ||
err: errZeroSpanID.Error(), | ||
}, | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hehe, what happens when so many people contributing are not native speakers. "trace exporter" was grammatically correct name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a jab at you, @Vemmy124 , but on the upstream change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I confess it did sound strange to my ears, but I'm not a native speaker myself, so, thought it was a problem with my ears :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that was bogdandrutu's idea open-telemetry/opentelemetry-collector#1974
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And maybe it's really the problem of me being not native speaker, but I agree with him, because the component consumes the structure named pdata.Traces
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't matter what exact data type is passed, a component that exports traces (plural) is "trace exporter" (noun used as adjective in singular form).