Icinga DB: Ensure all timestamps are in milliseconds

[julianbrost]

In Icinga DB heartbeats (i.e. icinga:stats messages), the startup time is currently in seconds, whereas other absolute timestamps are in milliseconds. Internally, we discussed today that we want to fix this in icinga2, even if this means that we have to require some icinga2 > 2.13.0 for icingadb 1.0.

So change this timestamp to milliseconds and also verify that there are no other timestamps left that are in seconds.

Other issues identified:

• flexible_duration is inconsistent between the downtime and downtime_history tables

refs Icinga/icingadb#357

[nilmerg]

this means that we have to require some icinga2 > 2.13.0 for icingadb 1.0.

It's no hard requirement. So anyone still using 2.13 can do so, but just won't see the correct start time. 🤷‍♂️

[julianbrost]

Part of the discussion was that because of #8953, we will probably end up with such a requirement anyways if we don't want to implement a workaround for that in icingadb, so we can fix this timestamp as well.

[julianbrost]

So I've looked over the code and everywhere a timestamp is explicitly written to Redis, it is done in milliseconds. The start time is special as this is just taken from IcingaApplication::StatsFunc():

icinga2/lib/icinga/icingaapplication.cpp

Line 84 in 95cdc00

{ "program_start", Application::GetStartTime() },

Then there are also intervals/durations: almost all of these are given in seconds, with one exception: check_timeout in states (which it looks like Icinga DB doesn't even sync to MySQL, does Icinga DB Web use those values?):

icinga2/lib/icingadb/icingadb-objects.cpp

Lines 2286 to 2289 in 95cdc00

	if (checkable->GetCheckTimeout().IsEmpty())
	attrs->Set("check_timeout", TimestampToMilliseconds(checkable->GetCheckCommand()->GetTimeout()));
	else
	attrs->Set("check_timeout", TimestampToMilliseconds(checkable->GetCheckTimeout()));

[nilmerg]

does Icinga DB Web use those values?

Yes. But only in the inspection detail:

From Redis as milliseconds: https://github.com/Icinga/icingadb-web/blob/master/library/Icingadb/Common/ObjectInspectionDetail.php#L133
From Db as seconds: https://github.com/Icinga/icingadb-web/blob/master/library/Icingadb/Common/ObjectInspectionDetail.php#L168

But indeed, only the Redis section shows a timeout.

[Al2Klimov]

@julianbrost Huh? One refs PR, but no fixes one?

[julianbrost]

#8998 fixes one timestamp, but there might be others that could still be wrong (I haven't seen any others so far though).

[julianbrost]

flexible_duration is inconsistent between the downtime and downtime_history tables:

icinga2/lib/icingadb/icingadb-objects.cpp

Line 1333 in 6b3a2ee

attributes->Set("flexible_duration", TimestampToMilliseconds(downtime->GetDuration()));

icinga2/lib/icingadb/icingadb-objects.cpp

Line 1700 in 6b3a2ee

"flexible_duration", Convert::ToString(downtime->GetDuration()),

icinga2/lib/icingadb/icingadb-objects.cpp

Line 1786 in 6b3a2ee

"flexible_duration", Convert::ToString(downtime->GetDuration()),

[julianbrost]

Two things brought up in this issue have been fixed:

• Startup time was fixed in Icinga DB: set value in milliseconds for program_start in stats/heartbeat #8998
• Icinga DB: Ensure all timestamps are in milliseconds #8991 (comment) was implicitly fixed in Icinga DB: Add downtime.duration & service_state.host_id to Redis #9061

There's one thing left, but that one is confusing me: #8991 (comment)
Icinga DB itself (i.e. the daemon written in Go) should not convert between seconds and milliseconds anywhere, so it sounds wrong to me that web expects to read the same value as milliseconds from Redis but as seconds from SQL. Maybe @nilmerg can shed some light on this?