-
Notifications
You must be signed in to change notification settings - Fork 45
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
Conversation
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)
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)); |
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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()
?
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 This update does expose a bug'ish on the GUI that we aren't compensating for the field Once the GUI corrects its schema check the display should look like |
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`.
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:
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:
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):(can also use swagger-ui at http://localhost:8080/api-docs)