Skip to content
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

Creates a new rule config parameter that "use_local_time_for_query" #51

Merged
merged 6 commits into from
Feb 23, 2021

Conversation

dilaverdemirel
Copy link
Contributor

Hi,
I needed to query indices with local time. So, i created a common parameter that "use_local_time_for_query".


It provide that use local time when query in "run_query". So, filters able to use local time can create. Indices able to have @timestap field with different timezone from "UTC" can be.

use_local_time_for_query: Whether to convert UTC time to the local time zone in rule queries.
If false, start and end time of query will be used UTC. (Optional, boolean, default false)

It provide that use local time when query in "run_query".  So, filters able to use local time can create. Indices able to have @timestap field with different timezone from "UTC" can be.
@jertel
Copy link
Owner

jertel commented Feb 21, 2021

Thanks for the contribution. What you have seems to be useful as it is, but would it be more flexible if instead of using 'local' time, the parameter instead specified an alternate timezone to use? That might help be more explicit since some users may not be aware of the timezone on the server running Elastalert.

@dilaverdemirel
Copy link
Contributor Author

You are right, it can be more useful.

I will rename the parameter "use_local_time_for_query" to "query_timezone". is it proper for this purpose. What do you think?

For example;
query_timezone : Europe/Istanbul

@jertel
Copy link
Owner

jertel commented Feb 22, 2021

Yes that seems like the right idea.

If "query_timezone" set, elasticsearch query start and end date will convert from UTC to specified time zone.
@jertel
Copy link
Owner

jertel commented Feb 23, 2021

I am re-reading your PR description and have a question -- Can you tell me how you managed to get ES to store the timestamps in a non-UTC timezone? I wasn't aware you could do this.

@dilaverdemirel
Copy link
Contributor Author

We are using syslog-ng as log collector. syslog-ng sets the log @timestamp value automatically from server datetime. We already changed the timezone at the server.

It is possible with this way. I have not deep information about the elasticsearch and syslog-ng.

Copy link
Owner

@jertel jertel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll approve/merge your PR since it won't hurt existing functionality, but I suspect that the new flag might be allowing you to write the rule time window in local time, and then Elastalert could be sending the time in your local timezone over to ES, yet ES internally is probably converting it back to UTC and offsetting the time by the timezone shift. If the PR change makes your rule writing easier for you so that you don't have to do the timezone calculation manually, then that's what's important.

@jertel jertel merged commit 1da7fe2 into jertel:alt Feb 23, 2021
@dilaverdemirel
Copy link
Contributor Author

I got your worrying. i tested it. it worked that i expected. This flag only changes query time window on query.

There are indices of elastalert. After that changes, they are working as it is, with UTC time. I checked it. I don't want break any functionality in this app.

Thank you, for your positive comments.

ferozsalam added a commit to ferozsalam/elastalert2 that referenced this pull request Jun 20, 2021
I noticed today that `make test` had started failing on my machine
when run outside of the Docker container.

This was due to the fact that a timezone wasn't being set correctly
when run on my machine, which is on BST. The test worked fine with
the container because it was on UTC.

I traced the error down to jertel#51,
in particular, a check which would return True in some cases where
it wasn't intended, because get() returns a None value by default
instead of the expected empty string.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants