Skip to content
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

Fix "add event" event handling #97

Merged
merged 1 commit into from
Aug 16, 2019

Conversation

krnowak
Copy link
Member

@krnowak krnowak commented Aug 15, 2019

The Event field is nowhere set. Instead Message and Attributes
fields are filled with information from the event. So when handling
the "add event" event, read the Message and Attributes fields,
instead of Event, which is always nil. This avoids a crash.

I found this issue when testing my opentracing bridge with stdout exporter.

The `Event` field is nowhere set. Instead `Message` and `Attributes`
fields are filled with information from the event. So when handling
the "add event" event, read the `Message` and `Attributes` fields,
instead of `Event`, which is always `nil`. This avoids a crash.
@krnowak
Copy link
Member Author

krnowak commented Aug 15, 2019

For the record, the stack trace I was getting before the fix:

$ ./lesson01 Foo
2019/08/15 14-47-23.826314 start say-hello, a root span [ service=hello-world span_id=4d65822107fcfd52 trace_id=78629a0f5f3f164fd5104dc76695721d ]
2019/08/15 14-47-23.826429 modify attr [ service=hello-world hello-to=Foo span_id=4d65822107fcfd52 trace_id=78629a0f5f3f164fd5104dc76695721d ]
2019/08/15 14-47-23.826479 finish say-hello (164.892µs) [ service=hello-world span_id=4d65822107fcfd52 trace_id=78629a0f5f3f164fd5104dc76695721d ]
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x635568]

goroutine 1 [running]:
go.opentelemetry.io/experimental/streaming/exporter/reader/format.AppendEvent(0xc000098520, 0x3, 0xbf4d72f6f142beb8, 0xc9818, 0x8f3ca0, 0x5, 0x0, 0x0, 0x0, 0x0, ...)
	/home/kv/projects/lightstep/opentelemetry-go/opentracing-bridge/experimental/streaming/exporter/reader/format/format.go:76 +0x298
go.opentelemetry.io/experimental/streaming/exporter/reader/format.EventToString(...)
	/home/kv/projects/lightstep/opentelemetry-go/opentracing-bridge/experimental/streaming/exporter/reader/format/format.go:135
go.opentelemetry.io/experimental/streaming/exporter/stdout.(*stdoutLog).Read(0x9104d8, 0x3, 0xbf4d72f6f142beb8, 0xc9818, 0x8f3ca0, 0x5, 0x0, 0x0, 0x0, 0x0, ...)
	/home/kv/projects/lightstep/opentelemetry-go/opentracing-bridge/experimental/streaming/exporter/stdout/stdout.go:32 +0xab
go.opentelemetry.io/experimental/streaming/exporter/reader.(*readerObserver).orderedObserve(0xc0000fe000, 0x5, 0xbf4d72f6f142beb8, 0xc9818, 0x8f3ca0, 0x3, 0x0, 0x0, 0x0, 0x0, ...)
	/home/kv/projects/lightstep/opentelemetry-go/opentracing-bridge/experimental/streaming/exporter/reader/reader.go:297 +0x2c5
go.opentelemetry.io/experimental/streaming/exporter/reader.(*readerObserver).Observe(0xc0000fe000, 0x5, 0xbf4d72f6f142beb8, 0xc9818, 0x8f3ca0, 0x3, 0x0, 0x0, 0x0, 0x0, ...)
	/home/kv/projects/lightstep/opentelemetry-go/opentracing-bridge/experimental/streaming/exporter/reader/reader.go:121 +0x6c
go.opentelemetry.io/experimental/streaming/exporter/observer.Record(0x5, 0xbf4d72f6f142beb8, 0xc9818, 0x8f3ca0, 0x3, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/home/kv/projects/lightstep/opentelemetry-go/opentracing-bridge/experimental/streaming/exporter/observer/observer.go:141 +0x16f
go.opentelemetry.io/experimental/streaming/sdk.(*span).AddEvent(0xc000090ea0, 0x72a1e0, 0xc0000a2018, 0x727740, 0xc000098500)
	/home/kv/projects/lightstep/opentelemetry-go/opentracing-bridge/experimental/streaming/sdk/span.go:107 +0x14e
go.opentelemetry.io/experimental/bridge/opentracing.(*bridgeSpan).LogFields(0xc0000983e0, 0xc000118000, 0x2, 0x2)
	/home/kv/projects/lightstep/opentelemetry-go/opentracing-bridge/experimental/bridge/opentracing/lib.go:114 +0xe4
main.main()
	/home/kv/projects/lightstep/opentelemetry-go/opentracing-bridge/experimental/bridge/opentracing/tutorial-test/lesson01/hello.go:30 +0x36d

Copy link

@tedsuo tedsuo left a comment

Choose a reason for hiding this comment

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

LGTM, Since this is experimental, going to merge quickly.

@tedsuo tedsuo merged commit 6dab220 into open-telemetry:master Aug 16, 2019
@krnowak krnowak deleted the krnowak/fix-add-event branch August 27, 2019 07:42
hstan referenced this pull request in hstan/opentelemetry-go Oct 15, 2020
…97)

As go mod tidy does have occasion to change `go.mod` files in addition
to `go.sum` (e.g. when a transitive dependency changes from an
implicit to explicit dependency), this new flag (`gomodsum_only`)
allows the build to pass so long as the only files changed are either
`go.mod` or `go.sum`
@pellared pellared added this to the untracked milestone Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants