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

ENT-3674: Change PAYG tally output to running total format #415

Merged
merged 2 commits into from
Apr 21, 2021

Conversation

kahowell
Copy link
Contributor

This introduces into the TallyResource the concept of "running total format". Running total format is currently enabled when the product ID supports hourly granularity.

Running total format does two things:

  1. It transforms tally snapshot data by making it additive (snapshot for a given date is the sum of previous dates in the response.
  2. It alters the filler to use previous snapshots for generating snapshots (rather than 0-valued snapshots).

Another related change I made was if we are producing a default snapshot for a given timestamp in the future, the values are set to null. This should cause the graph to omit the data point (making the dropoff abrupt at the edge of data).

Testing Suggestions

Deploy via RBAC_USE_STUB=true ./gradlew bootRun

Truncate existing tally data and insert some new:

truncate table tally_snapshots cascade;
INSERT INTO tally_snapshots (id, product_id, account_number, granularity, snapshot_date, sla, usage) VALUES ('1718a2fd-12eb-4c51-a7f1-7774bce33bfd', 'OpenShift-dedicated-metrics', 'account123', 'DAILY', '2021-04-01', '_ANY', '_ANY');
INSERT INTO tally_measurements (snapshot_id, measurement_type, uom, value) VALUES ('1718a2fd-12eb-4c51-a7f1-7774bce33bfd', 'TOTAL', 'CORES', 2);
INSERT INTO tally_snapshots (id, product_id, account_number, granularity, snapshot_date, sla, usage) VALUES ('dbb8e7c7-84da-4e80-a512-85e6a560b833', 'OpenShift-dedicated-metrics', 'account123', 'DAILY', '2021-04-02', '_ANY', '_ANY');
INSERT INTO tally_measurements (snapshot_id, measurement_type, uom, value) VALUES ('dbb8e7c7-84da-4e80-a512-85e6a560b833', 'TOTAL', 'CORES', 4);
INSERT INTO tally_snapshots (id, product_id, account_number, granularity, snapshot_date, sla, usage) VALUES ('96e8f3cb-8f71-4b3e-98cf-94c170e59e51', 'OpenShift-dedicated-metrics', 'account123', 'DAILY', '2021-04-08', '_ANY', '_ANY');
INSERT INTO tally_measurements (snapshot_id, measurement_type, uom, value) VALUES ('96e8f3cb-8f71-4b3e-98cf-94c170e59e51', 'TOTAL', 'CORES', 16);

Next, hit the endpoint for April 2021, and see the resulting snapshot values (esp. notice that future dates have null to visually indicate there is definitely no data for them yet):

curl -X 'GET' \
  'http://localhost:8080/api/rhsm-subscriptions/v1/tally/products/OpenShift-dedicated-metrics?granularity=Daily&beginning=2021-04-01T00%3A00%3A00Z&ending=2021-04-30T23%3A59%3A59.999Z' \
  -H 'accept: application/vnd.api+json' \
  -H 'x-rh-identity: eyJpZGVudGl0eSI6eyJhY2NvdW50X251bWJlciI6ImFjY291bnQxMjMiLCJ0eXBlIjoiVXNlciIsInVzZXIiOnsiaXNfb3JnX2FkbWluIjp0cnVlfSwiaW50ZXJuYWwiOnsib3JnX2lkIjoib3JnMTIzIn19fQo='

(can also use swagger-ui at http://localhost:8080/api-docs)

This introduces into the TallyResource the concept of "running total
format". Running total format is currently enabled when the product ID
supports hourly granularity.

Running total format does two things:
1. It transforms tally snapshot data by making it additive (snapshot for
   a given date is the sum of previous dates in the response).
2. It alters the filler to use previous snapshots for generating
   snapshots (rather than 0-valued snapshots).

Another related change I made was if we are producing a default snapshot
for a given timestamp in the future, the values are set to `null`. This
should cause the graph to omit the data point (making the dropoff abrupt
at the edge of data).

Testing Suggestions
-------------------

Deploy via `RBAC_USE_STUB=true ./gradlew bootRun`

Truncate existing tally data and insert some new:

```sql
truncate table tally_snapshots cascade;
INSERT INTO tally_snapshots (id, product_id, account_number, granularity, snapshot_date, sla, usage) VALUES ('1718a2fd-12eb-4c51-a7f1-7774bce33bfd', 'OpenShift-dedicated-metrics', 'account123', 'DAILY', '2021-04-01', '_ANY', '_ANY');
INSERT INTO tally_measurements (snapshot_id, measurement_type, uom, value) VALUES ('1718a2fd-12eb-4c51-a7f1-7774bce33bfd', 'TOTAL', 'CORES', 2);
INSERT INTO tally_snapshots (id, product_id, account_number, granularity, snapshot_date, sla, usage) VALUES ('dbb8e7c7-84da-4e80-a512-85e6a560b833', 'OpenShift-dedicated-metrics', 'account123', 'DAILY', '2021-04-02', '_ANY', '_ANY');
INSERT INTO tally_measurements (snapshot_id, measurement_type, uom, value) VALUES ('dbb8e7c7-84da-4e80-a512-85e6a560b833', 'TOTAL', 'CORES', 4);
INSERT INTO tally_snapshots (id, product_id, account_number, granularity, snapshot_date, sla, usage) VALUES ('96e8f3cb-8f71-4b3e-98cf-94c170e59e51', 'OpenShift-dedicated-metrics', 'account123', 'DAILY', '2021-04-08', '_ANY', '_ANY');
INSERT INTO tally_measurements (snapshot_id, measurement_type, uom, value) VALUES ('96e8f3cb-8f71-4b3e-98cf-94c170e59e51', 'TOTAL', 'CORES', 16);
```

Next, hit the endpoint for April 2021, and see the resulting snapshot
values (esp. notice that future dates have `null` to visually indicate
there is definitely no data for them yet):

```
curl -X 'GET' \
  'http://localhost:8080/api/rhsm-subscriptions/v1/tally/products/OpenShift-dedicated-metrics?granularity=Daily&beginning=2021-04-01T00%3A00%3A00Z&ending=2021-04-30T23%3A59%3A59.999Z' \
  -H 'accept: application/vnd.api+json' \
  -H 'x-rh-identity: eyJpZGVudGl0eSI6eyJhY2NvdW50X251bWJlciI6ImFjY291bnQxMjMiLCJ0eXBlIjoiVXNlciIsInVzZXIiOnsiaXNfb3JnX2FkbWluIjp0cnVlfSwiaW50ZXJuYWwiOnsib3JnX2lkIjoib3JnMTIzIn19fQo='
```

(can also use swagger-ui at http://localhost:8080/api-docs)
@kahowell
Copy link
Contributor Author

Reviewer, please don't merge until @cdcabrera confirms that output such as

{
  "date": "2021-04-21T00:00:00Z",
  "has_data": false,
  "has_cloudigrade_data": false,
  "has_cloudigrade_mismatch": false
}

won't cause issues for the GUI. @cdcabrera, please leave any feedback here.


// If no snaps contain dates, just use the start of the range. Otherwise,
// fill from the date of the last snapshot found, to the end of the range.
if (lastSnapDate == null) {
result.addAll(fillWithRange(firstDate, lastDate, offset));
result.addAll(fillWithRange(firstDate, lastDate, offset, null, useRunningTotalFormat));
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think of making overloaded fillWithRange, so there's not this "magic number null" parameter when reading through this logic? Something like fillWithRange(date,date,offset) which will pass "null" and "false" as the last two parameters of fillWithRange(date, date, offset, lastSnap, useRunningTotalFormat)

private TallySnapshot createDefaultSnapshot(OffsetDateTime snapshotDate) {
private TallySnapshot createDefaultSnapshot(OffsetDateTime snapshotDate, TallySnapshot previous,
boolean useRunningTotalFormat) {
if (snapshotDate.isBefore(clock.now()) && useRunningTotalFormat && previous != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Random question, why do we always pull the ApplicationClock in for this instead of just doing Calendar.getInstance().getTime()?

@lindseyburnett lindseyburnett self-assigned this Apr 21, 2021
@cdcabrera
Copy link
Member

cdcabrera commented Apr 21, 2021

For the most part the GUI will compensate for any missing fields, if it knows about them, via our internal schema check. That applies towards the graph, graph tooltips, and any potential inventory alterations.

Currently, if the graph is missing a field, and it knows about it, the fallback is to mark that field undefined and assign it a 0 value. The GUI doesn't compensate for missing dates, if the dates are missing, the graph skips them.

This update does expose a bug'ish on the GUI that we aren't compensating for the field core_hours so we'll be pushing a Jira issue (ENT-3809) and update to coordinate with this update, otherwise the GUI appears to render like

date-cutoff

Once the GUI corrects its schema check the display should look like

Screen Shot 2021-04-21 at 1 31 36 PM

@mirekdlugosz
@ntkathole

@lindseyburnett lindseyburnett merged commit 8134d3b into develop Apr 21, 2021
@lindseyburnett lindseyburnett deleted the khowell/ent-3674-payg-graph-changes branch April 21, 2021 21:19
kahowell added a commit that referenced this pull request Apr 21, 2021
Tiny follow-up to #415

Without this change, days without any recorded usage show up in the
tooltip as "no data", rather than the cumulative total.

See RedHatInsights/curiosity-frontend#282 for
explanation of GUI logic wrt `has_data`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants