Fix failing tests after Logstash 8 support microseconds #206
Conversation
… avoid the microseconds fraction
…on, if LS 8 then uses nanos precision
andsel
changed the title
| # could introduce error in representation that makes 0.192 millis to be expressed like 0.192000001 nanoseconds, | ||
| # so here we cut to millis, but there is a rounding when representing to to_iso8601, so 191998 micros becomes | ||
| # 192 millis in LogStash 8 while in previous versions it appears truncated like 191. | ||
| event[@target][k.to_s] = LogStash::Timestamp.at(secs, micros).to_iso8601 |
There was a problem hiding this comment.
Note for reviewers:
This force to update the test for flow[Start|End]Milliseconds, while in previous case when using LogStash::Timestamp.at(milliseconds) builder method resulted in cutting the to the 3 least significant digits, with this change is applies a rounding.
So the 191998 microseconds are rounded to 192 while previously was "cutted" at 191 milliseconds. This is the reason why some test were updated.
The motivation for this change is that with the previous code, in LS8 Timestamp that resulted in a value with nanoseconds precision, while in LS7 was with milliseconds precision.
| @@ -2038,7 +2050,7 @@ | |||
| "netscalerHttpReqUserAgent": "Mozilla/5.0 (Commodore 64; kobo.com) Gecko/20100101 Firefox/75.0", | |||
| "destinationTransportPort": 443, | |||
| "netscalerHttpReqCookie": "beer=123456789abcdefghijklmnopqrstuvw; AnotherCookie=1234567890abcdefghijklmnopqr; Shameless.Plug=Thankyou.Rakuten.Kobo.Inc.For.Allowing.me.time.to.work.on.this.and.contribute.back.to.the.community; Padding=aaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbccccccccccccccddddddddddddddddddddddeeeeeeeeeeeeeeeeeeeeeffffffffffffffffffffffgggggggggggggggggggggggghhhhhhhhhhhhhhhhhiiiiiiiiiiiiiiiiiiiiiijjjjjjjjjjjjjjjjjjjjjjjjkkkkkkkkkkkkkkkkkklllllllllllllllmmmmmmmmmm; more=less; GJquote=There.is.no.spoon; GarrySays=Nice!!; LastPadding=aaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbcccccccccccccccccccdddddddddddeeeeeeee", | |||
| "flowEndMicroseconds": "2016-11-11T12:09:19.000Z", | |||
| "flowEndMicroseconds": "2016-11-11T12:09:19.000#{nanos}Z", | |||
There was a problem hiding this comment.
Do you think it makes sense to have tests for flowEndMicroseconds and flowEndNanoseconds? I think we should at least have a test for µs precision as well as ms and ns
There was a problem hiding this comment.
I think that to insert that test we have to update the fixture data, in particular binary data files like:
ipfix_test_netscaler_tpl.datipfix_test_netscaler_data.dat
which I don't know where they came from. So it's not clear to me if (and how) we can update those binary files.
Release notes
Updates the milliseconds rounding for IPFIX start/end milliseconds fields. Fix the test to run on Logstash 8 with microseconds precision.
What does this PR do?
Updates the tests to adapt to Logstash 8 Timestamp nanoseconds precision.
Change the way it processes milliseconds for IPFIX fields related to start/end milliseconds, switching from a truncation at milliseconds to a rounding.
Why is it important/What is the impact to the user?
Permit to have a green CI tests.
Checklist
[ ] I have made corresponding changes to the documentation[ ] I have made corresponding change to the default configuration files (and/or docker env variables)[ ] I have added tests that prove my fix is effective or that my feature worksAuthor's Checklist
How to test this PR locally
Run the tests against a Logstash 8 checkout:
Related issues
Use cases
Screenshots
Logs