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

UI: Update dashboard client count query to use billing_start_timestamp #26729

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/26729.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
ui (enterprise): Update dashboard to make activity log query using the same start time as the metrics overview
```
8 changes: 4 additions & 4 deletions ui/app/components/clients/page/token.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<StatText
@label="Total clients"
@subText="The total number of entity and non-entity clients for this date range."
@value={{this.totalUsageCounts.clients}}
@value={{this.tokenStats.total}}
@tooltipText="This number is the total for the queried date range. The chart displays a monthly breakdown of total clients per month."
@size="l"
/>
Expand Down Expand Up @@ -100,21 +100,21 @@
<StatText
class="column"
@label="Total clients"
@value={{this.tokenUsageCounts.clients}}
@value={{this.tokenStats.total}}
@size="l"
@subText="The number of clients which interacted with Vault during this month. This is Vault’s primary billing metric."
/>
<StatText
class="column"
@label="Entity"
@value={{this.tokenUsageCounts.entity_clients}}
@value={{this.tokenStats.entity_clients}}
@size="l"
@subText="Representations of a particular user, client, or application that created a token via login."
/>
<StatText
class="column"
@label="Non-entity"
@value={{this.tokenUsageCounts.non_entity_clients}}
@value={{this.tokenStats.non_entity_clients}}
@size="l"
@subText="Clients created with a shared set of permissions, but not associated with an entity."
/>
Expand Down
4 changes: 2 additions & 2 deletions ui/app/components/clients/page/token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ export default class ClientsTokenPageComponent extends ActivityComponent {
return this.calculateClientAverages(this.byMonthNewClients);
}

get tokenUsageCounts() {
get tokenStats() {
if (this.totalUsageCounts) {
const { entity_clients, non_entity_clients } = this.totalUsageCounts;
return {
clients: entity_clients + non_entity_clients,
total: entity_clients + non_entity_clients,
entity_clients,
non_entity_clients,
};
Expand Down
21 changes: 3 additions & 18 deletions ui/app/components/dashboard/client-count-card.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,8 @@
<VaultLogoSpinner />
{{else}}
<div class="is-grid grid-2-columns grid-gap-2 has-top-margin-m grid-align-items-start is-flex-v-centered">
<StatText
@label="Total"
@value={{this.activityData.total.clients}}
@size="l"
@subText="The number of clients in this billing period ({{date-format
this.licenseStartTime
'MMM yyyy'
}} - {{date-format this.updatedAt 'MMM yyyy'}})."
data-test-stat-text="total-clients"
/>
<StatText
@label="New"
@value={{this.currentMonthActivityTotalCount}}
@size="l"
@subText="The number of clients new to Vault in the current month."
data-test-stat-text="new-clients"
/>
<StatText @label="Total" @value={{this.activityData.total.clients}} @size="l" @subText={{this.statSubText.total}} />
<StatText @label="New" @value={{this.currentMonthActivityTotalCount}} @size="l" @subText={{this.statSubText.new}} />
</div>

<div class="has-top-margin-l is-flex-center">
Expand All @@ -55,7 +40,7 @@
/>
<small class="has-left-margin-xs has-text-grey">
Updated
{{date-format this.updatedAt "MMM dd, yyyy hh:mm:ss"}}
{{date-format this.updatedAt "MMM d yyyy, h:mm:ss aaa" withTimeZone=true}}
Copy link
Contributor Author

@hellobontempo hellobontempo May 1, 2024

Choose a reason for hiding this comment

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

This format is consistent with the format we show response timestamps elsewhere in the app
Screenshot 2024-05-01 at 1 52 17 PM

</small>
</div>
{{/if}}
Expand Down
34 changes: 25 additions & 9 deletions ui/app/components/dashboard/client-count-card.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,50 +4,66 @@
*/

import Component from '@glimmer/component';
import getStorage from 'vault/lib/token-storage';
import timestamp from 'core/utils/timestamp';
import { task } from 'ember-concurrency';
import { waitFor } from '@ember/test-waiters';
import { tracked } from '@glimmer/tracking';
import { service } from '@ember/service';
import { setStartTimeQuery } from 'core/utils/client-count-utils';
import { dateFormat } from 'core/helpers/date-format';

/**
* @module DashboardClientCountCard
* DashboardClientCountCard component are used to display total and new client count information
*
* @example
* ```js
* <Dashboard::ClientCountCard @license={{@model.license}} />
* ```
* @param {object} license - license object passed from the parent
*
* <Dashboard::ClientCountCard @isEnterprise={{@version.isEnterprise}} />
*
* @param {boolean} isEnterprise - used for setting the start time for the activity log query
*/

export default class DashboardClientCountCard extends Component {
@service store;

clientConfig = null;
licenseStartTime = null;
@tracked activityData = null;
@tracked clientConfig = null;
@tracked updatedAt = timestamp.now().toISOString();

constructor() {
super(...arguments);
this.fetchClientActivity.perform();
this.clientConfig = this.store.queryRecord('clients/config', {}).catch(() => {});
}

get currentMonthActivityTotalCount() {
return this.activityData?.byMonth?.lastObject?.new_clients.clients;
}

get licenseStartTime() {
return this.args.license.startTime || getStorage().getItem('vault:ui-inputted-start-date') || null;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we no longer saved the inputted user start date in local storage

get statSubText() {
const format = (date) => dateFormat([date, 'MMM yyyy'], {});
return this.licenseStartTime
? {
total: `The number of clients in this billing period (${format(this.licenseStartTime)} - ${format(
this.updatedAt
)}).`,
new: 'The number of clients new to Vault in the current month.',
}
: { total: 'No total client data available.', new: 'No new client data available.' };
}

@task
@waitFor
*fetchClientActivity(e) {
if (e) e.preventDefault();
this.updatedAt = timestamp.now().toISOString();

if (!this.clientConfig) {
// set config and license start time when component initializes
this.clientConfig = yield this.store.queryRecord('clients/config', {}).catch(() => {});
this.licenseStartTime = setStartTimeQuery(this.args.isEnterprise, this.clientConfig);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

technically I could just pass true here instead of this.args.isEnterprise because this component is wrapped in a conditional and only renders for enterprise versions. however, in case a future refactor shuffled conditionals, I wanted to preserve the logic in setStartTimeQuery which prevents from accidentally setting the billing start time to 0001-01-01T00:00:00Z for community versions

}

// only make the network request if we have a start_time
if (!this.licenseStartTime) return {};
try {
Expand Down
6 changes: 2 additions & 4 deletions ui/app/components/dashboard/overview.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@

<div class="has-bottom-margin-xl">
<div class="is-flex-row gap-24">
{{#if (and @version.isEnterprise (or @license @isRootNamespace))}}
{{#if (and @version.isEnterprise @isRootNamespace)}}
<div class="is-flex-column is-flex-1 gap-24">
{{#if @license}}
<Dashboard::ClientCountCard @license={{@license}} />
{{/if}}
<Dashboard::ClientCountCard @isEnterprise={{@version.isEnterprise}} />
{{#if
(and @isRootNamespace (has-permission "status" routeParams="replication") (not (is-empty-value @replication)))
}}
Expand Down
12 changes: 3 additions & 9 deletions ui/app/routes/vault/cluster/clients/counts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ import type VersionService from 'vault/services/version';

import type { ModelFrom } from 'vault/vault/route';
import type ClientsRoute from '../clients';
import type ClientsConfigModel from 'vault/models/clients/config';
import type ClientsActivityModel from 'vault/models/clients/activity';
import type ClientsCountsController from 'vault/controllers/vault/cluster/clients/counts';
import { setStartTimeQuery } from 'core/utils/client-count-utils';

export interface ClientsCountsRouteParams {
start_time?: string | number | undefined;
end_time?: string | number | undefined;
Expand Down Expand Up @@ -86,10 +87,7 @@ export default class ClientsCountsRoute extends Route {
async model(params: ClientsCountsRouteParams) {
const { config, versionHistory } = this.modelFor('vault.cluster.clients') as ModelFrom<ClientsRoute>;
// only enterprise versions will have a relevant billing start date, if null users must select initial start time
let startTime = null;
if (this.version.isEnterprise && this._hasConfig(config)) {
startTime = getUnixTime(config.billingStartTimestamp);
}
const startTime = setStartTimeQuery(this.version.isEnterprise, config);
Copy link
Contributor Author

@hellobontempo hellobontempo May 1, 2024

Choose a reason for hiding this comment

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

Although this logic is fairly simple and only used in two places, I wanted to consolidate it so that setting the start time was handled the same in both places the /activity log is queried. See Jira VAULT-25315 for more context around the bug introduced by not checking the version.


const startTimestamp = Number(params.start_time) || startTime;
const endTimestamp = Number(params.end_time) || getUnixTime(timestamp.now());
Expand Down Expand Up @@ -118,8 +116,4 @@ export default class ClientsCountsRoute extends Route {
});
}
}

_hasConfig(model: ClientsConfigModel | object): model is ClientsConfigModel {
return 'billingStartTimestamp' in model;
}
}
1 change: 0 additions & 1 deletion ui/app/routes/vault/cluster/dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ export default class VaultClusterDashboardRoute extends Route.extend(ClusterRout
return hash({
replication,
secretsEngines: this.store.query('secret-engine', {}),
license: this.store.queryRecord('license', {}).catch(() => null),
isRootNamespace: this.namespace.inRootNamespace && !hasChroot,
version: this.version,
vaultConfiguration: hasChroot ? null : this.getVaultConfiguration(),
Expand Down
1 change: 0 additions & 1 deletion ui/app/templates/vault/cluster/dashboard.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
<Dashboard::Overview
@replication={{this.model.replication}}
@secretsEngines={{this.model.secretsEngines}}
@license={{this.model.license}}
@isRootNamespace={{this.model.isRootNamespace}}
@version={{this.model.version}}
@vaultConfiguration={{this.model.vaultConfiguration}}
Expand Down
19 changes: 18 additions & 1 deletion ui/lib/core/addon/utils/client-count-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import { parseAPITimestamp } from 'core/utils/date-formatters';
import { compareAsc, getUnixTime, isWithinInterval } from 'date-fns';

import type ClientsConfigModel from 'vault/models/clients/config';
import type ClientsVersionHistoryModel from 'vault/vault/models/clients/version-history';

/*
Expand Down Expand Up @@ -59,6 +60,18 @@ export const filterVersionHistory = (
return [];
};

export const setStartTimeQuery = (
isEnterprise: boolean,
config: ClientsConfigModel | Record<string, never>
) => {
// CE versions have no license and so the start time defaults to "0001-01-01T00:00:00Z"
if (isEnterprise && _hasConfig(config)) {
return getUnixTime(config.billingStartTimestamp);
}
return null;
};

// METHODS FOR SERIALIZING ACTIVITY RESPONSE
export const formatDateObject = (dateObj: { monthIdx: number; year: number }, isEnd: boolean) => {
const { year, monthIdx } = dateObj;
// day=0 for Date.UTC() returns the last day of the month before
Expand Down Expand Up @@ -188,6 +201,11 @@ export const namespaceArrayToObject = (
};

// type guards for conditionals
function _hasConfig(model: ClientsConfigModel | object): model is ClientsConfigModel {
if (!model) return false;
return 'billingStartTimestamp' in model;
}

export function hasMountsKey(
obj: ByMonthNewClients | NamespaceNewClients | MountNewClients
): obj is NamespaceNewClients {
Expand All @@ -201,7 +219,6 @@ export function hasNamespacesKey(
}

// TYPES RETURNED BY UTILS (serialized)

export interface TotalClients {
clients: number;
entity_clients: number;
Expand Down
12 changes: 5 additions & 7 deletions ui/tests/acceptance/dashboard-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -404,16 +404,14 @@ module('Acceptance | landing page dashboard', function (hooks) {
assert.dom(DASHBOARD.cardName('client-count')).exists();
const response = await this.store.peekRecord('clients/activity', 'some-activity-id');
assert.dom('[data-test-client-count-title]').hasText('Client count');
assert.dom('[data-test-stat-text="total-clients"] .stat-label').hasText('Total');
assert.dom('[data-test-stat-text="Total"] .stat-label').hasText('Total');
assert.dom('[data-test-stat-text="Total"] .stat-value').hasText(formatNumber([response.total.clients]));
assert.dom('[data-test-stat-text="New"] .stat-label').hasText('New');
assert
.dom('[data-test-stat-text="total-clients"] .stat-value')
.hasText(formatNumber([response.total.clients]));
assert.dom('[data-test-stat-text="new-clients"] .stat-label').hasText('New');
assert
.dom('[data-test-stat-text="new-clients"] .stat-text')
.dom('[data-test-stat-text="New"] .stat-text')
.hasText('The number of clients new to Vault in the current month.');
assert
.dom('[data-test-stat-text="new-clients"] .stat-value')
.dom('[data-test-stat-text="New"] .stat-value')
.hasText(formatNumber([response.byMonth.lastObject.new_clients.clients]));
});
});
Expand Down
12 changes: 8 additions & 4 deletions ui/tests/integration/components/clients/page/token-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,21 +69,25 @@ module('Integration | Component | clients | Clients::Page::Token', function (hoo

test('it should render monthly total chart', async function (assert) {
const count = this.activity.byMonth.length;
assert.expect(count + 7);
const { entity_clients, non_entity_clients } = this.activity.total;
assert.expect(count + 8);
const getAverage = (data) => {
const average = ['entity_clients', 'non_entity_clients'].reduce((count, key) => {
return (count += calculateAverage(data, key) || 0);
}, 0);
return formatNumber([average]);
};
const expectedTotal = getAverage(this.activity.byMonth);
const expectedAvg = getAverage(this.activity.byMonth);
const expectedTotal = formatNumber([entity_clients + non_entity_clients]);
const chart = CHARTS.container('Entity/Non-entity clients usage');

await this.renderComponent();

assert
.dom(CLIENT_COUNT.statTextValue('Average total clients per month'))
.dom(CLIENT_COUNT.statTextValue('Total clients'))
.hasText(expectedTotal, 'renders correct total clients');
assert
.dom(CLIENT_COUNT.statTextValue('Average total clients per month'))
.hasText(expectedAvg, 'renders correct average clients');

// assert bar chart is correct
assert.dom(`${chart} ${CHARTS.xAxis}`).hasText('7/23 8/23 9/23 10/23 11/23 12/23 1/24');
Expand Down
Loading
Loading