[O11y][Postgresql] Add support for user specific timezone map - #13681
Conversation
harnish-crest-data
added
enhancement
| source: |- | ||
| String get_timezone(def ctx) { | ||
| if (ctx.event?.timezone != null) { | ||
| String tz = ctx.event.timezone.trim().toUpperCase(); |
There was a problem hiding this comment.
Why not create the map inside script processor's "params" field? I do not think create a separate field is necessary?
See: https://www.elastic.co/docs/explore-analyze/scripting/modules-scripting-using#prefer-params
There was a problem hiding this comment.
Getting
postgresql/log test-postgresql-ubuntu-9-5.log:
[0] field "params" is used as array of objects, expected explicit definition with type group or nested
Also I have referred this script from cisco_ios: https://github.com/elastic/integrations/blob/main/packages/cisco_ios/data_stream/log/elasticsearch/ingest_pipeline/default.yml#L91
| return tz; | ||
| } | ||
|
|
||
| // Handle offset formats (+00:00, -00:00, +00, -00) |
There was a problem hiding this comment.
Also, why did we remove the offset handling?
There was a problem hiding this comment.
PostgreSQL logs always contains timezone in the log, as well user can have multiple timezones in theirs log file so no need to provide tz_offset to user. Using tz_short, tz_long we can handle the behavior!
| default: | ||
| - /var/log/postgresql/postgresql-*-*.log* | ||
| - /var/log/postgresql/postgresql-*-*.csv* | ||
| - name: tz_map |
|
Suggestion: It will be good to add a section (or a troubleshooting section or both) in the README related to the newly added map, as this is not a mandatory parameter to configure. |
Added |
🚀 Benchmarks reportTo see the full report comment with |
|
|
Hello, is there any issue blocking this PR ? |
|
Hello @shmsr is there any issue blocking this PR ?? |
|
Hello @shmsr is there anything issue blocking this PR ?? |
There was a problem hiding this comment.
Changes LGTM. Nothing I'd really change.
Can you also take a look @ishleenk17? Looks like there's nothing blocking and it should be merged.
The changes were done as per the design I discussed with Harnish. |
|
I've tested a few which were not included in the original list, such as NZDT or NOR and it works fine. |
|
/test |
💚 Build Succeeded
History
cc @lucian-ioan |
|
|
Package postgresql - 1.29.0 containing this change is available at https://epr.elastic.co/package/postgresql/1.29.0/ |
…c#13681) * Add support for user specific timezone map * update PR link * update readme --------- Co-authored-by: Ishleen Kaur <102962586+ishleenk17@users.noreply.github.com>
andrewkroh
added
the
documentation
9 participants
Proposed commit message
Description:
The original script extends Java's built-in ZoneId.SHORT_IDS map with CUSTOM_ZONE_IDS for additional abbreviations. This limitation makes it difficult for users to handle timezone abbreviations that are not included in the list.
Problem:
When a user encounters a timezone abbreviation that is not in the list, the script is unable to correctly map the timestamp to the corresponding timezone.
Goal:
To make the script more user-friendly by allowing users to specify their own timezone mappings. This can be achieved by:
Expected Behavior:
The script should be able to correctly handle timezone abbreviations that are not included in the default list, by using the user-provided mapping or falling back to event.timezone if no mapping is found.
Checklist
changelog.ymlfile.Related issues
Screenshots
Specified under the tz_map
Not specified under the tz_map