nipwaayoni / elastic-apm-php-agent

PHP Agent for Elastic APM
Other
28 stars 15 forks source link

Timestamps don't work in 32-bit environment #43

Closed pgross41 closed 4 years ago

pgross41 commented 4 years ago

In Transaction.php and Span.php, and EventBean.php the timestamp variable is cast to an int. The * 1000000 to convert it to microseconds makes it a very large number but on my machine I'm getting random smaller numbers and they are sometimes negative, the value appears to be overflowing and looping around. Per the PHP_INT_MAX description I believe microseconds is too big to be cast to an int in a 32-bit environment. I am on a 64-bit Windows machine but I suspect PHP is running as a 32-bit application.

Example of what I'm seeing: This float round(1593065529.2766 * 1000000) correctly evaluates to 1.5930655292766E+15 But as an int (int) round(1593065529.2766 * 1000000) it evaluates to 2029648056

I know the APM spec wants an int but I think it needs to be a float in memory to support 32-bit environments. When it serializes it looks just like an int so APM won't know the difference.

dstepe commented 4 years ago

@pgross41 I merged this to the 7.2-dev branch and was testing it with a Laravel application. I'm seeing occasional rejections of spans by APM:

error validating JSON document against schema: I[#] S[#] doesn't validate with "span#"
  I[#] S[#/allOf/0] allOf failed
    I[#/timestamp] S[#/allOf/0/properties/timestamp/type] expected integer or null, but got number"

The timestamp value is being sent as a float in these cases:

"timestamp": 1593262177733446.8

The source is the setting of the query start time in the Laravel package, but I'm going to look for where we can fix it in the EventBean in this class. Just an FYI in case you run into this.

pgross41 commented 4 years ago

Thanks for the heads up

dstepe commented 4 years ago

It's not pretty, but I believe I have a functional implementation. I just released v7.2.0 if you could give it a try. I'll be curious how the official client which is in progress addresses this.