search

openzipkin-contrib / brave-ratpack

Brave instrumentation for Ratpack
Apache License 2.0
5 stars 4 forks source link

TraceContextHolder Class has to be Singleton #19

Closed kirenjolly closed 5 years ago

kirenjolly commented 5 years ago

TraceContextHolder Class has to be Singleton

Multiple instance of the same class is found in the registries of the Current Execution object. This causes RatpackCurrentTraceContext.get() function to return null context on creation of new spans. Hence forming separate traces.

screenshot 7

kirenjolly commented 5 years ago

@adriancole @llinder @beckje01 Please take a look.

llinder commented 5 years ago

Thanks. I would agree that this seems like a bug we should fix. I have opened a PR https://github.com/openzipkin-contrib/brave-ratpack/pull/20 but still need to sort out proper test before it is merged.

kirenjolly commented 5 years ago

@llinder Can you release a SNAPSHOT then for our local testing.

llinder commented 5 years ago

@kirenjolly I added some tests over the weekend and published a new release 2.4.1. Mind checking to see if this resolves things for you?

kirenjolly commented 5 years ago

Sure will take a look. Thanks

llinder commented 5 years ago

I'm going to assume this issue has been resolved. If not please reopen with comment and or tests showing whats failing.