fix: Improve timestamp parsing #16806
Conversation
The timestamp parsing is now able to handle scientific notation of floating point string representations correctly. While integers with more than 10 digits were already handled correctly as nanoseconds, floating point numbers were not. Signed-off-by: Christian Haudum <[email protected]>
Signed-off-by: Christian Haudum <[email protected]>
|
I'm still thinking about this, but I'm really reluctant to accept a floating point number as a timestamp because I believe it's impossible to accurately represent a nanosecond timestamp floating point number as a double precision float. I believe this would could lead a really difficult to troubleshoot scenario where folks are expecting log lines to be returned in a result but they are missing (or more lines are returned) because the floating point representation lacks enough precision to be accurate to the nanosecond level. |
Do you mean we should only allow second precision timestamp as floats or not floats at all? I find the way that floats work differently than integers really confusing. I'm totally up for disallowing floats completely. |
|
I would vote we either
I think we historically have the latter for prometehus compatibility, so it will be a breaking change to disallow allow all floats. I don't think it's a good idea to allow nansecond precision using floats. |
|
Follow up, what tooling is wanting to send floats with nanosecond precision? It seems like this is very niche? |
What this PR does / why we need it:
The timestamp parsing is now able to handle scientific notation of floating point string representations correctly.
While integers with more than 10 digits were already handled correctly as nanoseconds, floating point numbers were not.