openzipkin / zipkin-aws

Reporters and collectors for use in Amazon's cloud
Apache License 2.0
69 stars 34 forks source link

NullPointerException occurs on 0.23.2 #188

Closed ueokande closed 1 year ago

ueokande commented 3 years ago

Describe the Bug

When we upgrade brave-propagation-aws 0.23.1 to 0.23.2, we encounter NullPointerException occurs on test using @SpringBootTest and TestRestTemplate.

Steps to Reproduce

Here is a minimal code to reproduce it: https://github.com/ueokande/brave-npe

Clone the repository, move into the cloned directory, and you can confirm by the following commands:

# Test with brave-propagation-aws 0.23.1
$ mvn clean test
-> Pass all tests

# Test with brave-propagation-aws 0.23.2
$ sed -i 's/0.23.1/0.23.2/' pom.xml 
$ mvn clean test
-> Fail on HealthControllerTest

Expected Behaviour

Pass all tests

marcuslange commented 2 years ago

What is the status of this?

JochemKuijpers commented 1 year ago

@marcuslange I just tested this as I was also running into this issue and wondering if it was the same issue as this one. (it is.)

With brave-propagation 0.23.1 it indeed succeeds, with 0.23.2 and up it fails with a NPE. Thanks for the minimal code example that demonstrates this, @ueokande.

java.lang.NullPointerException: Cannot read field "customFields" because "amznTraceId" is null
    at brave.propagation.aws.AWSInjector.inject(AWSInjector.java:48)

This unconditional line is indeed introduced in 0.23.2. https://github.com/openzipkin/zipkin-aws/commit/7fd8b22a57babddaf26f3a72f83bc1ae20e4ea53#diff-c22355edcc7ee3318bb1a77b1578525ae30b987dde7bcf993825957353cdd41fR48 in #186 The problem is that traceContext.findExtra(AmznTraceId.class); may return null if AmznTraceId.class is not present among the extras in the TracingContext. However the next line assumes a non-null value, and thus causes a NPE.

Sadly downpatching to 0.23.1 does not work for me, other dependencies lock me into 0.23.2+.