-
Notifications
You must be signed in to change notification settings - Fork 530
[servicenow] Add Parsing for ECS Timestamp Field #16884
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
🚀 Benchmarks reportTo see the full report comment with |
💚 Build Succeeded
|
| - yyyy-MM-dd H:mm:ss | ||
| - yyyy-MM-dd HH:mm:ss | ||
| - yyyy-MM-dd | ||
| - MM-dd-yyyy H:mm:ss | ||
| - MM-dd-yyyy HH:mm:ss | ||
| - MM-dd-yyyy | ||
| - dd-MM-yyyy H:mm:ss | ||
| - dd-MM-yyyy HH:mm:ss | ||
| - dd-MM-yyyy | ||
| - MM/dd/yyyy H:mm:ss | ||
| - MM/dd/yyyy HH:mm:ss | ||
| - MM/dd/yyyy | ||
| - dd/MM/yyyy H:mm:ss | ||
| - dd/MM/yyyy HH:mm:ss | ||
| - dd/MM/yyyy | ||
| - MM/dd/yy H:mm:ss | ||
| - MM/dd/yy HH:mm:ss | ||
| - MM/dd/yy | ||
| - dd.MM.yyyy H:mm:ss | ||
| - dd.MM.yyyy HH:mm:ss | ||
| - dd.MM.yyyy | ||
| - yyyy-MM-dd hh:mm:ss a | ||
| - ISO8601 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My concern here is with formats like
- MM-dd-yyyy
- dd-MM-yyyy
as these are ambiguous from a global timestamp perspective. In india it's common trend to use dd-MM-yyyy or dd/MM/yyyy or dd.MM.yyyy. But in many places we use mm-dd-yyyy and etc.
How do we distinguish if the date 01-02-2026 or 05-07-2026 is in MM-dd-yyyy format or dd-MM-yyyy format ? Do we expect '@timestamp' to be normalised in any way ? If not how can we decide upon the right order of execution here ? If '@timestamp' is normalised as month-day-year, why have support for both patterns ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree. This is a LOCALE issue and should be configurable if it is necessary to handle different formats. Given that the servicenow data sources are essentially a free-for-all, we cannot safely determine which to use without being told by the user during configuration.
But in many places we use mm-dd-yyyy
Depending on the scope of "place"; this ordering is a US localisation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue exists across the entire integration. All date processors currently support both MM-dd-yyyy and dd-MM-yyyy formats, while the preference is consistently set to MM-dd-yyyy across all processors.
I believe this is a separate concern, and we could open a new ticket to make the date format user-configurable instead of supporting multiple formats by default. Please let me know your thoughts, or if you think this should be addressed as part of the current PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's reasonable to use this given that the pattern is already widespread. We should fix this in a follow-up though since currently dd-MM-yyyy dates in the source data are being incorrectly parsed when dd is not greater than 12. The problem is that currently the effective default LOCALE is for the US format, so we would need to maintain that, and non-US formats would be required to turn on their LOCALE preference to ensure that timestamps are parsed when they are not in dd ≤ 12. This would be a breaking change, unless we make it relaxed rather than strict (we keep both orders, but the configuration changes the order of appearance in the formats list).
Proposed commit message
Checklist
changelog.ymlfile.How to test this PR locally
Related Issues