appuio / appuio-cloud-reporting

Reporting for APPUiO Cloud
BSD 3-Clause "New" or "Revised" License
0 stars 1 forks source link

Add time range to tenants #120

Closed anothertobi closed 1 year ago

anothertobi commented 1 year ago

Summary

Introduces a time range for the source/target links of a tenant.

Checklist

anothertobi commented 1 year ago

Draft since this does not yet handle existing tenants where the time is outside the time range.

When a tenant is created, the time of the run is used (oldTime). A subsequent run could have a time that is before the previously used lower bound (newTime).

Idea: Set lower bound of existing tenant to newTime (move during back in time) when source and target match and no other target exists in the time range. If another target for the source exists in the time range between newTime and the upper bound of a source, create a new tenant with the lower bound newTime and an upper bound of the lower bound of the other source (-1h).

@bastjan wdyt?

anothertobi commented 1 year ago

related PR: https://github.com/appuio/appuio-io-docs/pull/145

anothertobi commented 1 year ago

Decided to fail if there is an overlap in the time range for a tenant instead of covering small edge-cases.

bastjan commented 1 year ago

Also don't forget the invoice part, from my quick reading it should just work™️ but to feel better i'd add a split timerange to one of the invoice tests https://github.com/appuio/appuio-cloud-reporting/blob/master/pkg/invoice/invoice_test.go.

bastjan commented 1 year ago

I forgot our discussions already but did we not say we'll create tenants from query timestamp to infinity when upserting them? 🙈

anothertobi commented 1 year ago

I forgot our discussions already but did we not say we'll create tenants from query timestamp to infinity when upserting them? 🙈

I think we decided on the default of -infinity, infinity+ or fail because we can't guarantee queries to be run in order.