Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix processor overriding ReadBuf data #1099

Merged
merged 1 commit into from
Oct 7, 2018

Conversation

pavolloffay
Copy link
Member

ReadBuf byte slice can be overriden before processing in processor is finished.

{"level":"error","ts":1538663826.591028,"caller":"processors/thrift_processor.go:117","msg":"Processor failed","error":"*jaeger.Batch error reading struct: *jaeger.Span error reading struct: *jaeger.Tag error reading struct: *jaeger.Tag field 7 read error: don't know what type: %!s(thrift.tCompactType=14)","stacktrace":"github.com/jaegertracing/jaeger/cmd/agent/app/processors.(*ThriftProcessor).processBuffer\n\t/home/ploffay/projects/golang/src/github.com/jaegertracing/jaeger/cmd/agent/app/processors/thrift_processor.go:117"}
{"level":"error","ts":1538663827.887695,"caller":"processors/thrift_processor.go:117","msg":"Processor failed","error":"*jaeger.Batch error reading struct: *jaeger.Span error reading struct: *jaeger.Log error reading struct: don't know what type: %!s(thrift.tCompactType=14)","stacktrace":"github.com/jaegertracing/jaeger/cmd/agent/app/processors.(*ThriftProcessor).processBuffer\n\t/home/ploffay/projects/golang/src/github.com/jaegertracing/jaeger/cmd/agent/app/processors/thrift_processor.go:117"}
{"level":"error","ts":1538663827.9029524,"caller":"processors/thrift_processor.go:117","msg":"Processor failed","error":"*jaeger.Batch error reading struct: *jaeger.Span error reading struct: *jaeger.Tag error reading struct: error reading field 1: Invalid data length","stacktrace":"github.com/jaegertracing/jaeger/cmd/agent/app/processors.(*ThriftProcessor).processBuffer\n\t/home/ploffay/projects/golang/src/github.com/jaegertracing/jaeger/cmd/agent/app/processors/thrift_processor.go:117"}
{"level":"error","ts":1538663827.9916568,"caller":"processors/thrift_processor.go:117","msg":"Processor failed","error":"*jaeger.Batch error reading struct: *jaeger.Span error reading struct: *jaeger.Log error reading struct: *jaeger.Tag error reading struct: error reading field 1: Invalid data length","stacktrace":"github.com/jaegertracing/jaeger/cmd/agent/app/processors.(*ThriftProcessor).processBuffer\n\t/home/ploffay/projects/golang/src/github.com/jaegertracing/jaeger/cmd/agent/app/processors/thrift_processor.go:117"}
{"level":"error","ts":1538663828.0100384,"caller":"processors/thrift_processor.go:117","msg":"Processor failed","error":"*jaeger.Batch error reading struct: *jaeger.Span error reading struct: *jaeger.Tag error reading struct: *jaeger.Tag field -58 read error: don't know what type: %!s(thrift.tCompactType=13)","stacktrace":"github.com/jaegertracing/jaeger/cmd/agent/app/processors.(*ThriftProcessor).processBuffer\n\t/home/ploffay/projects/golang/src/github.com/jaegertracing/jaeger/cmd/agent/app/processors/thrift_processor.go:117"}
{"level":"error","ts":1538663828.0260687,"caller":"processors/thrift_processor.go:117","msg":"Processor failed","error":"*jaeger.Batch error reading struct: *jaeger.Span error reading struct: *jaeger.Tag error reading struct: Required field VType is not set","stacktrace":"github.com/jaegertracing/jaeger/cmd/agent/app/processors.(*ThriftProcessor).processBuffer\n\t/home/ploffay/projects/golang/src/github.com/jaegertracing/jaeger/cmd/agent/app/processors/thrift_processor.go:117"}
{"level":"error","ts":1538663828.0396416,"caller":"processors/thrift_processor.go:117","msg":"Processor failed","error":"*jaeger.Batch error reading struct: *jaeger.Span error reading struct: *jaeger.Tag error reading struct: Required field Key is not set","stacktrace":"github.com/jaegertracing/jaeger/cmd/agent/app/processors.(*ThriftProcessor).processBuffer\n\t/home/ploffay/projects/golang/src/github.com/jaegertracing/jaeger/cmd/agent/app/processors/thrift_processor.go:117"}
{"level":"error","ts":1538663828.0613725,"caller":"processors/thrift_processor.go:117","msg":"Processor failed","error":"*jaeger.Batch error reading struct: *jaeger.Span error reading struct: *jaeger.Tag error reading struct: don't know what type: %!s(thrift.tCompactType=14)","stacktrace":"github.com/jaegertracing/jaeger/cmd/agent/app/processors.(*ThriftProcessor).processBuffer\n\t/home/ploffay/projects/golang/src/github.com/jaegertracing/jaeger/cmd/agent/app/processors/thrift_processor.go:117"}
{"level":"error","ts":1538663828.0657265,"caller":"processors/thrift_processor.go:117","msg":"Processor failed","error":"*jaeger.Batch error reading struct: *jaeger.Span error reading struct: Required field TraceIdLow is not set","stacktrace":"github.com/jaegertracing/jaeger/cmd/agent/app/processors.(*ThriftProcessor).processBuffer\n\t/home/ploffay/projects/golang/src/github.com/jaegertracing/jaeger/cmd/agent/app/processors/thrift_processor.go:117"}
{"level":"error","ts":1538663828.1010654,"caller":"processors/thrift_processor.go:117","msg":"Processor failed","error":"*jaeger.Batch error reading struct: *jaeger.Span error reading struct: Required field TraceIdHigh is not set","stacktrace":"github.com/jaegertracing/jaeger/cmd/agent/app/processors.(*ThriftProcessor).processBuffer\n\t/home/ploffay/projects/golang/src/github.com/jaegertracing/jaeger/cmd/agent/app/processors/thrift_processor.go:117"}
{"level":"error","ts":1538663828.1204793,"caller":"processors/thrift_processor.go:117","msg":"Processor failed","error":"*jaeger.Batch error reading struct: *jaeger.Span error reading struct: *jaeger.Tag error reading struct: *jaeger.Tag field 13 read error: don't know what type: %!s(thrift.tCompactType=14)","stacktrace":"github.com/jaegertracing/jaeger/cmd/agent/app/processors.(*ThriftProcessor).processBuffer\n\t/home/ploffay/projects/golang/src/github.com/jaegertracing/jaeger/cmd/agent/app/processors/thrift_processor.go:117"}
{"level":"error","ts":1538663828.1882222,"caller":"processors/thrift_processor.go:117","msg":"Processor failed","error":"*jaeger.Batch error reading struct: *jaeger.Span error reading struct: *jaeger.Log error reading struct: *jaeger.Tag error reading struct: don't know what type: %!s(thrift.tCompactType=14)","stacktrace":"github.com/jaegertracing/jaeger/cmd/agent/app/processors.(*ThriftProcessor).processBuffer\n\t/home/ploffay/projects/golang/src/github.com/jaegertracing/jaeger/cmd/agent/app/processors/thrift_processor.go:117"}
{"level":"error","ts":1538663828.2727396,"caller":"processors/thrift_processor.go:117","msg":"Processor failed","error":"*jaeger.Batch error reading struct: *jaeger.Span error reading struct: *jaeger.Tag error reading struct: don't know what type: %!s(thrift.tCompactType=14)","stacktrace":"github.com/jaegertracing/jaeger/cmd/agent/app/processors.(*ThriftProcessor).processBuffer\n\t/home/ploffay/projects/golang/src/github.com/jaegertracing/jaeger/cmd/agent/app/processors/thrift_processor.go:117"}
{"level":"error","ts":1538663828.3243358,"caller":"processors/thrift_processor.go:117","msg":"Processor failed","error":"*jaeger.Batch error reading struct: *jaeger.Span error reading struct: *jaeger.Log error reading struct: *jaeger.Tag error reading struct: error reading field 3: Invalid data length","stacktrace":"github.com/jaegertracing/jaeger/cmd/agent/app/processors.(*ThriftProcessor).processBuffer\n\t/home/ploffay/projects/golang/src/github.com/jaegertracing/jaeger/cmd/agent/app/processors/thrift_processor.go:117"}
{"level":"error","ts":1538663828.3399134,"caller":"processors/thrift_processor.go:117","msg":"Processor failed","error":"*jaeger.Batch error reading struct: *jaeger.Span error reading struct: *jaeger.Log error reading struct: Required field Timestamp is not set","stacktrace":"github.com/jaegertracing/jaeger/cmd/agent/app/processors.(*ThriftProcessor).processBuffer\n\t/home/ploffay/projects/golang/src/github.com/jaegertracing/jaeger/cmd/agent/app/processors/thrift_processor.go:117"}
^C{"level":"info","ts":1538663830.0657444,"caller":"all-in-one/main.go:135","msg":"Shutting down"}
{"level":"info","ts":1538663830.0658135,"caller":"all-in-one/main.go:142","msg":"Shutdown complete"}

Signed-off-by: Pavol Loffay <ploffay@redhat.com>
@codecov
Copy link

codecov bot commented Oct 4, 2018

Codecov Report

Merging #1099 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1099   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         140     140           
  Lines        6621    6622    +1     
======================================
+ Hits         6621    6622    +1
Impacted Files Coverage Δ
cmd/agent/app/processors/thrift_processor.go 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0cd8a07...6f7450d. Read the comment docs.

Copy link
Contributor

@black-adder black-adder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! I learned something new about golang today. I was under the assumption that when you attempt to copy over a slice via [:], it doesn't create a copy but just creates a reference to the same array. I think there maybe more mistakes like this in the codebase. Quick playground https://play.golang.org/p/SDkvXsOxszA to confirm.

@black-adder black-adder merged commit 4ada2b9 into jaegertracing:master Oct 7, 2018
@ghost ghost removed the review label Oct 7, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants