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

TimePeriod objects should support timezones #5374

Open
aleliga opened this issue Jun 22, 2017 · 20 comments
Open

TimePeriod objects should support timezones #5374

aleliga opened this issue Jun 22, 2017 · 20 comments
Labels
area/configuration DSL, parser, compiler, error handling area/notifications Notification events enhancement New feature or request help wanted Extra attention is needed needs-sponsoring Not low on priority but also not scheduled soon without any incentive queue/wishlist

Comments

@aleliga
Copy link

aleliga commented Jun 22, 2017

Hi,
as in the thread:
https://monitoring-portal.org/index.php?thread/40869-set-notifications-timeperiod-to-local-time-zone-not-utc/&postID=251295#post251295
the notification are set in UTC timezone, i had to convert my local time (Rome) to UTC and manually change it at every Daylight Saving change.
The same had to be done for different timezone clients

Maybe good the possibility to set timezone in host, to convert (automatically) notification time in local time (based on host timezone)

TY for all good work you done

@dnsmichi
Copy link
Contributor

Do you have a specific configuration example and logs? I don't understand the issue here nor on the provided URL.

@dnsmichi dnsmichi added area/notifications Notification events question labels Jun 23, 2017
@aleliga
Copy link
Author

aleliga commented Jun 23, 2017

Hi @dnsmichi
This is a feature request, not a bug, everything work fine.
Correct me if i'm wrong...
my setup for 9:00 to 17:00 it's like that:
Object 'T-8x5-summer-italy' of type 'TimePeriod':
friday = "07:00-15:00"
Because we are in CEST (in summer +2h from the UTC)
ours client in London are +1h from UTC and the same Timeperiod (in BST local time) has to be set like this:
Object 'T-8x5-summer-england' of type 'TimePeriod':
friday = "08:00-16:00"

In future release, could be good, if in the host definitions may be possible define the host timezone that, if set, calculate the start and ending time in local time
with a feature like that could be possible to set a global timeperiod that can be applied to all hosts in different timezones:
Object 'host-in-italy' of type 'Host'
* timezone = "CEST"
Object 'host-in-london' of type 'Host'
* timezone = "BST"

Object 'T-8x5-global' of type 'TimePeriod':
friday = "09:00-17:00"

@dnsmichi dnsmichi changed the title Request: set host timezone that affect notifications TimePeriod objects should support timezones Jan 16, 2018
@dnsmichi dnsmichi added area/configuration DSL, parser, compiler, error handling needs feedback We'll only proceed once we hear from you again wishlist and removed question labels Jan 16, 2018
@dnsmichi dnsmichi added needs-sponsoring Not low on priority but also not scheduled soon without any incentive and removed needs feedback We'll only proceed once we hear from you again labels Apr 5, 2018
@mdeguzis
Copy link

mdeguzis commented Apr 7, 2018

This would be very useful for us too. The general mantra is to keep things in UTC (especially for logs), but notifications and downtimes should have the ability to respect localtime. Meaning, our weekly sunday downtime would happen at 14:00 each time, not 13:00 or 14:00 based on DST.

Though... when setting up a recurring DT. not sure I understand. Since we are UTC-4, asking for 14:00 should have been 18:00, but had to use: sunday = "19:00-22:00" to achieve our sunday DT for 14:00 localtime. I am UTC-4 right now as the server time is 14:17 and local is 10:17

[root@icinga icinga-plugins]# date Sat Apr 7 14:46:36 UTC 2018

...yet I had to use 19:00 for the recurring dt object to target 14:00 for tomorrow with current UTC-4 in effect.

Maybe this is a misunderstanding on my part. I didn't want to open an issue if I just don't have enough coffee in my system yet :)

@peteeckel
Copy link
Contributor

peteeckel commented Jun 15, 2018

I strongly support this feature request.

Most data centres I deal with are running on UTC, but none of the support organisations are. The current implementation of the TimePeriods can't deal with this very at all ... while, i.e. "8to5" must be coded as "07:00-16:00" in the winter, it need to be changed to "06:00-15:00" when summer time becomes active.

Being able to specify a time zone in the TimePeriod object would magically fix this, and besides it would get rid of any uncertainty when configuring downtimes, notifications and checks.

@joncolas
Copy link

It would be great for us too!

@dnsmichi dnsmichi added the enhancement New feature or request label Jul 3, 2018
@philomory
Copy link

As a global organization monitoring systems in remote offices and stores that get shut off at the 'end of the day' local time, this would be huge for us as well.

@Rayn0r
Copy link

Rayn0r commented Sep 12, 2018

I'd like to see such a feature as well.
We are using a Scheduler inside AWS to boot and shutdown machines according to local time. Since Icinga uses UTC, time periods between Icinga and the Scheduler differ by 1-2 hours depending on DST.

@ianbamforth
Copy link

+1, would be very useful

@mpounsett
Copy link

This would be very useful for notifications for services that have "next business day" service level agreements. Basing the notification time off of the server (which is probably in UTC, but in some organizations might be in a different local time zone than the service organization) makes configuring these notification time periods more complicated than necessary.

@dnsmichi
Copy link
Contributor

Blocked by #7288.

@Al2Klimov Al2Klimov added the needs feedback We'll only proceed once we hear from you again label Mar 30, 2020
@Al2Klimov Al2Klimov added help wanted Extra attention is needed and removed needs feedback We'll only proceed once we hear from you again labels Apr 1, 2020
@mojothemonkey
Copy link

looks like the blocker is resolved, and we can continue?

+1 here too for timezone support.
we were running "bottom-up" config, and timeperiods were relative to client timezones.
but of course this is deprecated, and we have some windows clients, so I switched to "top-down command endpoint".
so now master is the scheduler, and all the timeperiods are relative to master. and for each timeperiod, we have to maintain a number of "offset" timeperiods to match the client timezones (all 9 of them!). which also need amending whenever master/client timezones have daylight saving out of line :(

@Icinga Icinga deleted a comment from carraroj Apr 9, 2021
@henningw
Copy link

An update about the current status would be indeed great.

@julianbrost
Copy link
Contributor

As far as I know, there is nobody working on this currently.

@julianbrost julianbrost modified the milestones: 2.13.0, 2.14.0 May 31, 2021
@dwt
Copy link

dwt commented Apr 27, 2022

Well, as I'm getting bitten by this too right now, I would like t add my support and ask: What is this blocked by? What can we as the community provide to move this forward?

What I'm seeing is a service that runs on a machine in the timezone 'Europe/Berlin', while icinga runs in UTC.

What I personally would like is a configuration like this:

apply ScheduledDowntime "restart-matomo" to Service {
	author = "icingaadmin"
	comment = "Scheduled downtime for service restart"
	
	fixed = false
	duration = 10m
	
	var restartTime = "01:00-02:00"
	timezone = "Europe/Berlin"
	
	ranges = {
		monday = restartTime
		tuesday = restartTime
		wednesday = restartTime
		thursday = restartTime
		friday = restartTime
		saturday = restartTime
		sunday = restartTime
	}
	
	assign where match("*matomo*", service.name)
}

My workaround is to make the flexible downtime twice as wide to account for DST, but it's a hacky solution.

@julianbrost
Copy link
Contributor

I see two problems with implementing this:

  1. Windows: I'm not sure if there's a common way to represent timezones on Linux and Windows (i.e. I don't know what it takes to make Windows understand "Europe/Berlin"). Just have a look at this beauty in the tests:
    // DST changes in America/Los_Angeles:
    // 2021-03-14: 01:59:59 PST (UTC-8) -> 03:00:00 PDT (UTC-7)
    // 2021-11-07: 01:59:59 PDT (UTC-7) -> 01:00:00 PST (UTC-8)
    #ifndef _WIN32
    static const char *dst_test_timezone = "America/Los_Angeles";
    #else /* _WIN32 */
    // Tests are using pacific time because Windows only really supports timezones following US DST rules with the TZ
    // environment variable. Format is "[Standard TZ][negative UTC offset][DST TZ]".
    // https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/tzset?view=msvc-160#remarks
    static const char *dst_test_timezone = "PST8PDT";
    #endif /* _WIN32 */
  2. The current code is based on mktime() and related functions which take their timezone information from the environment, so setting this on a per-object basis is tricky.

So supporting this would likely require a rewrite of the whole time period evaluation code and at the moment I don't even know how that would look like:

  • Can this be done without new dependencies?
  • If we need to, what are the options?
  • Would we to include timezone information in our builds (for example ship a timezone database on windows)?

@slalomsk8er
Copy link
Contributor

Maybe it would help if the SchduledDowntime would support a timezone and state filter ala "Europe/Berlin" & summer|winter.
This way one could define 2 SchduledDowntimes hat would only apply if icinga matches the timezone and the respecting sommer or winter version of it.

@dwt
Copy link

dwt commented May 4, 2022

@julianbrost I'm not a windows programmer, and googling does suggest that this timezone format is not easily parseable, as Windows does not use the IANNA database.

If you want to do this without new dependencies, I would guess that the best way forward is to only support the native way of entering timezones on the system. i.e. Europe/Berlin on Unix and something like AD on Windows.

If new dependencies are an option, and since you already are using boost, I would try to go with a boost library. It also seems possible to ship timezone information as csv for boost.

@community do you have any other suggestions for this?

In my code base, I would roughly try to serialize changes like this:

  1. Add a timezone string parameter to objects that take times interpreted however the current system wants it's timezones specified (maybe even specify that this is for documentation purposes at first, maybe document some known workarounds for the timezones). Maybe allow to add two different timezone names so config synced between Unix <-> Windows can be made to work.
  2. Start adding support for consulting timezone values when creating and displaying dates everywhere in the code. In my experience this will take quite some time, as going from timezone naive datetimes to timezone aware date times can be quite a challenge.
  3. Consider adding a standard timezone format and library to support it, possibly shipping IANA data on windows with the build.

Step 1. can probably be fast, step 2 will likely take quite some time to get right, step 3 can probably be worked on independently of step 2 whenever the need arises, but has the potential to make config handling quite a bit nicer.

@dwt
Copy link

dwt commented May 4, 2022

This library also looks really nice. I'm not enough of a C++ programmer to really understand what he's saying, but it seems that this library has become part of c++20's standard library.

@julianbrost julianbrost removed this from the 2.14.0 milestone Jan 12, 2023
@Al2Klimov
Copy link
Member

I guess this is another thing for Icinga Notifications, like #8741 (comment).

@peteeckel
Copy link
Contributor

I don't agree.

TimePeriods are also used for scheduling checks, which isn't covered by notifications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/configuration DSL, parser, compiler, error handling area/notifications Notification events enhancement New feature or request help wanted Extra attention is needed needs-sponsoring Not low on priority but also not scheduled soon without any incentive queue/wishlist
Projects
None yet
Development

No branches or pull requests