From 3aef60aac182aab9b2b4f1779b2916b0a31ff751 Mon Sep 17 00:00:00 2001 From: Wilbert Guo Date: Mon, 23 Nov 2020 11:20:35 -0800 Subject: [PATCH] Make changes based on PR (https://github.com/open-telemetry/opentelemetry-go-contrib/pull/459) --- idgenerator/aws/go.mod | 5 +- idgenerator/aws/go.sum | 6 ++ idgenerator/aws/xray/aws_xray_idgenerator.go | 46 +++++++------ .../aws/xray/aws_xray_idgenerator_test.go | 69 +++++++++---------- 4 files changed, 69 insertions(+), 57 deletions(-) diff --git a/idgenerator/aws/go.mod b/idgenerator/aws/go.mod index 4734aa345de..2130eebe098 100644 --- a/idgenerator/aws/go.mod +++ b/idgenerator/aws/go.mod @@ -2,4 +2,7 @@ module go.opentelemetry.io/contrib/idgenerator/aws/xray go 1.15 -require github.com/stretchr/testify v1.6.1 +require ( + github.com/stretchr/testify v1.6.1 + go.opentelemetry.io/otel v0.14.0 +) diff --git a/idgenerator/aws/go.sum b/idgenerator/aws/go.sum index afe7890c9a1..3f628996f99 100644 --- a/idgenerator/aws/go.sum +++ b/idgenerator/aws/go.sum @@ -1,10 +1,16 @@ github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/google/go-cmp v0.5.3 h1:x95R7cp+rSeeqAMI2knLtQ0DKlaBhv2NrtrOvafPHRo= +github.com/google/go-cmp v0.5.3/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +go.opentelemetry.io/otel v0.14.0 h1:YFBEfjCk9MTjaytCNSUkp9Q8lF7QJezA06T71FbQxLQ= +go.opentelemetry.io/otel v0.14.0/go.mod h1:vH5xEuwy7Rts0GNtsCW3HYQoZDY+OmBJ6t1bFGGlxgw= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= diff --git a/idgenerator/aws/xray/aws_xray_idgenerator.go b/idgenerator/aws/xray/aws_xray_idgenerator.go index 5e5113edc9f..cdeb8559967 100644 --- a/idgenerator/aws/xray/aws_xray_idgenerator.go +++ b/idgenerator/aws/xray/aws_xray_idgenerator.go @@ -18,30 +18,32 @@ import ( crand "crypto/rand" "encoding/binary" "encoding/hex" - "log" "math/rand" "strconv" "sync" "time" + + "go.opentelemetry.io/otel/trace" +) + +const ( + errConvertTimeToHex errorConst = "cannot convert current timestamp to hex" ) -type spanID [8]byte -type traceID [16]byte +type errorConst string -// IDGenerator is an interface for generating new TraceIDs and SpanIDs -type IDGenerator interface { - NewTraceID() traceID - NewSpanID() spanID +func (e errorConst) Error() string { + return string(e) } -type xRayIDGenerator struct { +type IDGenerator struct { sync.Mutex randSource *rand.Rand } -// XRayIDGenerator returns an idGenerator used for sending traces to AWS X-Ray -func XRayIDGenerator() IDGenerator { - gen := &xRayIDGenerator{} +// NewIDGenerator returns an idGenerator used for sending traces to AWS X-Ray +func NewIDGenerator() *IDGenerator { + gen := &IDGenerator{} var rngSeed int64 err := binary.Read(crand.Reader, binary.LittleEndian, &rngSeed) if err != nil { @@ -52,33 +54,37 @@ func XRayIDGenerator() IDGenerator { } // NewSpanID returns a non-zero span ID from a randomly-chosen sequence. -func (gen *xRayIDGenerator) NewSpanID() spanID { +func (gen *IDGenerator) NewSpanID() trace.SpanID { gen.Lock() defer gen.Unlock() - sid := spanID{} + sid := trace.SpanID{} gen.randSource.Read(sid[:]) return sid } // NewTraceID returns a non-zero trace ID based on AWS X-Ray TraceID format. // (https://docs.aws.amazon.com/xray/latest/devguide/xray-api-sendingdata.html#xray-api-traceids) -func (gen *xRayIDGenerator) NewTraceID() traceID { +func (gen *IDGenerator) NewTraceID() (trace.TraceID, error) { gen.Lock() defer gen.Unlock() - tid := traceID{} - currentTime := getCurrentTimeHex() + tid := trace.TraceID{} + currentTime, err := getCurrentTimeHex() + if err != nil { + var nilTraceID trace.TraceID + return nilTraceID, err + } copy(tid[:4], currentTime) gen.randSource.Read(tid[4:]) - return tid + return tid, nil } -func getCurrentTimeHex() []uint8 { +func getCurrentTimeHex() ([]uint8, error) { currentTime := time.Now().Unix() currentTimeHex, err := hex.DecodeString(strconv.FormatInt(currentTime, 16)) if err != nil { - log.Fatalf("%s: %v", "Could not convert timestamp to hex", err) + return nil, errConvertTimeToHex } - return currentTimeHex + return currentTimeHex, nil } diff --git a/idgenerator/aws/xray/aws_xray_idgenerator_test.go b/idgenerator/aws/xray/aws_xray_idgenerator_test.go index 30d2bfffed9..6d5598bd60a 100644 --- a/idgenerator/aws/xray/aws_xray_idgenerator_test.go +++ b/idgenerator/aws/xray/aws_xray_idgenerator_test.go @@ -16,86 +16,83 @@ package aws import ( "bytes" - "encoding/hex" "strconv" "testing" "time" "github.com/stretchr/testify/assert" -) - -func (t traceID) convertTraceIDToHexString() string { - return hex.EncodeToString(t[:]) -} -func (s spanID) convertSpanIDToHexString() string { - return hex.EncodeToString(s[:]) -} + "go.opentelemetry.io/otel/trace" +) func TestAwsXRayTraceIdIsValidLength(t *testing.T) { - idg := XRayIDGenerator() - traceIDHex := idg.NewTraceID().convertTraceIDToHexString() - traceIDLength := len(traceIDHex) - expectedTraceIDLength := 32 + idg := NewIDGenerator() + traceID, _ := idg.NewTraceID() - assert.Equal(t, traceIDLength, expectedTraceIDLength, "TraceID has incorrect length.") + expectedTraceIDLength := 32 + assert.Equal(t, len(traceID.String()), expectedTraceIDLength, "TraceID has incorrect length.") } func TestAwsXRayTraceIdIsUnique(t *testing.T) { - idg := XRayIDGenerator() - traceID1 := idg.NewTraceID().convertTraceIDToHexString() - traceID2 := idg.NewTraceID().convertTraceIDToHexString() + idg := NewIDGenerator() + traceID1, _ := idg.NewTraceID() + traceID2, _ := idg.NewTraceID() - assert.NotEqual(t, traceID1, traceID2, "TraceID should be unique") + assert.NotEqual(t, traceID1.String(), traceID2.String(), "TraceID should be unique") } func TestAwsXRayTraceIdTimeStampInBounds(t *testing.T) { - idg := XRayIDGenerator() + idg := NewIDGenerator() previousTime := time.Now().Unix() - traceIDHex := idg.NewTraceID().convertTraceIDToHexString() - currentTime, err := strconv.ParseInt(traceIDHex[0:8], 16, 64) - - nextTime := time.Now().Unix() + traceID, err := idg.NewTraceID() + if err != nil { + t.Error(err) + } + currentTime, err := strconv.ParseInt(traceID.String()[0:8], 16, 64) if err != nil { t.Error(err) } + nextTime := time.Now().Unix() + assert.LessOrEqual(t, previousTime, currentTime, "TraceID is generated incorrectly with the wrong timestamp.") assert.LessOrEqual(t, currentTime, nextTime, "TraceID is generated incorrectly with the wrong timestamp.") } func TestAwsXRayTraceIdIsNotNil(t *testing.T) { - var nilTraceID traceID - idg := XRayIDGenerator() - traceID := idg.NewTraceID() + var nilTraceID trace.TraceID + idg := NewIDGenerator() + traceID, err := idg.NewTraceID() + if err != nil { + t.Error(err) + } assert.False(t, bytes.Equal(traceID[:], nilTraceID[:]), "TraceID cannot be Nil.") } func TestAwsXRaySpanIdIsValidLength(t *testing.T) { - idg := XRayIDGenerator() - spanIDHex := idg.NewSpanID().convertSpanIDToHexString() - spanIDLength := len(spanIDHex) + idg := NewIDGenerator() + spanID := idg.NewSpanID() expectedSpanIDLength := 16 - assert.Equal(t, spanIDLength, expectedSpanIDLength, "SpanID has incorrect length") + assert.Equal(t, len(spanID.String()), expectedSpanIDLength, "SpanID has incorrect length") } func TestAwsXRaySpanIdIsUnique(t *testing.T) { - idg := XRayIDGenerator() - spanID1 := idg.NewSpanID().convertSpanIDToHexString() - spanID2 := idg.NewSpanID().convertSpanIDToHexString() + idg := NewIDGenerator() + spanID1 := idg.NewSpanID() + spanID2 := idg.NewSpanID() - assert.NotEqual(t, spanID1, spanID2, "SpanID should be unique") + assert.NotEqual(t, spanID1.String(), spanID2.String(), "SpanID should be unique") } func TestAwsXRaySpanIdIsNotNil(t *testing.T) { - var nilSpanID spanID - idg := XRayIDGenerator() + var nilSpanID trace.SpanID + idg := NewIDGenerator() spanID := idg.NewSpanID() assert.False(t, bytes.Equal(spanID[:], nilSpanID[:]), "SpanID cannot be Nil.")