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.

Sign up for GitHub

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

Jump to bottom

feat(manager): move and update metrics cards in the server view #2241

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

feat(manager): move and update metrics cards in the server view #2241

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
90673c3
feat(manager): move and update metrics cards in the server view
daniellacosse Oct 10, 2024
51b17e9
getting close. trying to figure out this weird data model
daniellacosse Oct 16, 2024
4b51e45
style update
daniellacosse Oct 16, 2024
8d5d9a5
don't use reduce lol
daniellacosse Oct 16, 2024
80af4c7
okay it roughly works end to end
daniellacosse Oct 16, 2024
f30ccc7
add metrics messages
daniellacosse Oct 16, 2024
ee8c388
access key tab
daniellacosse Oct 16, 2024
ab29f2c
update to use new "endpoint" - feedback next
daniellacosse Nov 12, 2024
2a6076d
feedback
daniellacosse Nov 12, 2024
23bf19e
Merge branch 'master' into daniellacosse/tunneltime/move_metrics
daniellacosse Nov 14, 2024
2b1d7da
endpoint implemented
daniellacosse Nov 21, 2024
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
Binary file added server_manager/images/Material-Icons.woff2
View file
Open in desktop
Binary file not shown.
11 changes: 11 additions & 0 deletions server_manager/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,17 @@
http-equiv="Content-Security-Policy"
content="default-src 'self' 'unsafe-inline' outline: data:; connect-src https: 'self'; frame-src https://s3.amazonaws.com/outline-vpn/ ss:"
/>

<style>
/* We declare this here to sidestep webpack */
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you mean with sidestepping Webpack? Why do you need that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can try to figure out how to get this to work with webpack, but currently webpack wants to resolve the url link here but doesn't know how. This index.html isn't transformed by webpack so that's what I mean by "sidestepping"

@font-face {
font-family: 'Material Icons';
font-style: normal;
font-weight: 400;
src: url(images/Material-Icons.woff2) format('woff2');
}
</style>

<title>Outline Manager</title>
</head>
<body>
Expand Down
26 changes: 26 additions & 0 deletions server_manager/messages/master_messages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1051,6 +1051,16 @@
"message": "Access keys",
"description": "This string appears within the server view as a header of a table column that displays server access keys."
},
"server_access_keys_tab": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are we changing this? Can you keep the existing "Connections" for now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's part of the set of changes that UX wanted! Do you want me to move it to a separate PR? It's behind a feature flag currently.

"message": "Access keys ($KEY_COUNT$)",
"description": "This string is a tab header indicating to the user that they're currently managing their access keys.",
"placeholders": {
"KEY_COUNT": {
"content": "{keyCount}",
"example": "12"
}
}
},
"server_connections": {
"message": "Connections",
"description": "This string appears within the server view as a header of the section that displays server information and access keys."
Expand Down Expand Up @@ -1099,6 +1109,22 @@
"message": "Metrics",
"description": "This string appears within the server view as a header of the section that displays server metrics."
},
"server_metrics_data_transferred": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should decouple the window size from the message. How do we do the current "/ last 30 days"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's currently hardcoded:

outline-apps/server_manager/messages/master_messages.json

Lines 1068 to 1075 in ab29f2c

"server_data_transfer": {
"message": "Data transferred / last 30 days",
"description": "This string appears within the server view as the header of a card that displays the amount of data transferred by the server."
},
"server_data_used": {
"message": "Allowance used / last 30 days",
"description": "This string appears within the server view as the header of a card that displays the amount of data transferred by the server as a percentage of the total available data."
},

Would you be okay with me decoupling this when we add the ability for users to set their own time range?

"message": "Data transferred in the last 30 days",
"description": "This string indicates to the user that the metric displayed counts how much data was sent through the server over the last 30 days"
},
"server_metrics_user_hours": {
"message": "User hours spent on the VPN in the last 30 days",
"description": "This string indicates to the user that the metric displayed counts how many hours users used the VPN over the last 30 days."
},
"server_metrics_user_hours_unit": {
"message": "hours",
"description": "This string indicates to the user that the metric displayed is in hours."
},
"server_metrics_average_devices": {
"message": "Average number of devices used in the last 30 days",
"description": "This string indicates to the user that the metric displayed corresponds to the average number of devices that used the VPN over the last 30 days."
},
"server_my_access_key": {
"message": "My access key",
"description": "This string appears within the server view as the header for the default server access key. This key is meant to be used by the server administrator."
Expand Down
29 changes: 25 additions & 4 deletions server_manager/model/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ export interface Server {
// Lists the access keys for this server, including the admin.
listAccessKeys(): Promise<AccessKey[]>;

// Returns stats for bytes transferred across all access keys of this server.
getDataUsage(): Promise<BytesByAccessKey>;
// Returns server metrics
getServerMetrics(): Promise<ServerMetricsJson>;

// Adds a new access key to this server.
addAccessKey(): Promise<AccessKey>;
Expand Down Expand Up @@ -186,10 +186,31 @@ export interface AccessKey {
dataLimit?: DataLimit;
}

export type BytesByAccessKey = Map<AccessKeyId, number>;

// Data transfer allowance, measured in bytes.
// NOTE: Must be kept in sync with the definition in src/shadowbox/access_key.ts.
export interface DataLimit {
readonly bytes: number;
}

export type ServerMetricsJson = {
servers: {
location: string;
asn: number;
asOrg: string;
tunnelTime: {
seconds: number;
};
dataTransferred: {
bytes: number;
};
}[];
accessKeys: {
accessKeyId: number;
tunnelTime: {
seconds: number;
};
dataTransferred: {
bytes: number;
};
}[];
};
64 changes: 45 additions & 19 deletions server_manager/www/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -970,7 +970,7 @@ export class App {
console.error(`Failed to load access keys: ${error}`);
this.appRoot.showError(this.appRoot.localize('error-keys-get'));
}
this.showTransferStats(server, view);
this.showServerMetrics(server, view);
}, 0);
}

Expand Down Expand Up @@ -1035,30 +1035,56 @@ export class App {
}
}

private async refreshTransferStats(
private async refreshServerMetrics(
selectedServer: server_model.Server,
serverView: ServerView
) {
try {
const usageMap = await selectedServer.getDataUsage();
const keyTransfers = [...usageMap.values()];
const serverMetrics = await selectedServer.getServerMetrics();

let totalUserHours = 0;
for (const {
tunnelTime: {seconds},
} of serverMetrics.servers) {
// convert to hours
totalUserHours += seconds / (60 * 60);
}

serverView.totalUserHours = totalUserHours;
serverView.totalDevices = serverView.totalUserHours / (30 * 24);

let totalInboundBytes = 0;
for (const accessKeyBytes of keyTransfers) {
totalInboundBytes += accessKeyBytes;
for (const {
dataTransferred: {bytes},
} of serverMetrics.accessKeys) {
totalInboundBytes += bytes;
}

serverView.totalInboundBytes = totalInboundBytes;

// Update all the displayed access keys, even if usage didn't change, in case data limits did.
const keyDataTransferMap = serverMetrics.accessKeys.reduce(
(map, {accessKeyId, dataTransferred}) => {
map.set(String(accessKeyId), dataTransferred.bytes);
return map;
},
new Map<string, number>()
);

let keyTransferMax = 0;
let dataLimitMax = selectedServer.getDefaultDataLimit()?.bytes ?? 0;
for (const key of await selectedServer.listAccessKeys()) {
serverView.updateAccessKeyRow(key.id, {
transferredBytes: usageMap.get(key.id) ?? 0,
dataLimitBytes: key.dataLimit?.bytes,
for (const accessKey of await selectedServer.listAccessKeys()) {
serverView.updateAccessKeyRow(accessKey.id, {
transferredBytes: keyDataTransferMap.get(accessKey.id) ?? 0,
dataLimitBytes: accessKey.dataLimit?.bytes,
});
keyTransferMax = Math.max(keyTransferMax, usageMap.get(key.id) ?? 0);
dataLimitMax = Math.max(dataLimitMax, key.dataLimit?.bytes ?? 0);
keyTransferMax = Math.max(
keyTransferMax,
keyDataTransferMap.get(accessKey.id) ?? 0
);
dataLimitMax = Math.max(dataLimitMax, accessKey.dataLimit?.bytes ?? 0);
}

serverView.baselineDataTransfer = Math.max(keyTransferMax, dataLimitMax);
} catch (e) {
// Since failures are invisible to users we generally want exceptions here to bubble
Expand All @@ -1074,11 +1100,11 @@ export class App {
}
}

private showTransferStats(
private showServerMetrics(
selectedServer: server_model.Server,
serverView: ServerView
) {
this.refreshTransferStats(selectedServer, serverView);
this.refreshServerMetrics(selectedServer, serverView);
// Get transfer stats once per minute for as long as server is selected.
const statsRefreshRateMs = 60 * 1000;
const intervalId = setInterval(() => {
Expand All @@ -1087,7 +1113,7 @@ export class App {
clearInterval(intervalId);
return;
}
this.refreshTransferStats(selectedServer, serverView);
this.refreshServerMetrics(selectedServer, serverView);
}, statsRefreshRateMs);
}

Expand Down Expand Up @@ -1158,7 +1184,7 @@ export class App {
this.appRoot.showNotification(this.appRoot.localize('saved'));
serverView.defaultDataLimitBytes = limit?.bytes;
serverView.isDefaultDataLimitEnabled = true;
this.refreshTransferStats(this.selectedServer, serverView);
this.refreshServerMetrics(this.selectedServer, serverView);
// Don't display the feature collection disclaimer anymore.
serverView.showFeatureMetricsDisclaimer = false;
window.localStorage.setItem(
Expand All @@ -1184,7 +1210,7 @@ export class App {
await this.selectedServer.removeDefaultDataLimit();
serverView.isDefaultDataLimitEnabled = false;
this.appRoot.showNotification(this.appRoot.localize('saved'));
this.refreshTransferStats(this.selectedServer, serverView);
this.refreshServerMetrics(this.selectedServer, serverView);
} catch (error) {
console.error(`Failed to remove server default data limit: ${error}`);
this.appRoot.showError(this.appRoot.localize('error-remove-data-limit'));
Expand Down Expand Up @@ -1232,7 +1258,7 @@ export class App {
const serverView = await this.appRoot.getServerView(server.getId());
try {
await server.setAccessKeyDataLimit(keyId, {bytes: dataLimitBytes});
this.refreshTransferStats(server, serverView);
this.refreshServerMetrics(server, serverView);
this.appRoot.showNotification(this.appRoot.localize('saved'));
return true;
} catch (error) {
Expand All @@ -1253,7 +1279,7 @@ export class App {
const serverView = await this.appRoot.getServerView(server.getId());
try {
await server.removeAccessKeyDataLimit(keyId);
this.refreshTransferStats(server, serverView);
this.refreshServerMetrics(server, serverView);
this.appRoot.showNotification(this.appRoot.localize('saved'));
return true;
} catch (error) {
Expand Down
71 changes: 48 additions & 23 deletions server_manager/www/shadowbox_server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,19 +36,6 @@ interface ServerConfigJson {
accessKeyDataLimit?: server.DataLimit;
}

// Byte transfer stats for the past 30 days, including both inbound and outbound.
// TODO: this is copied at src/shadowbox/model/metrics.ts. Both copies should
// be kept in sync, until we can find a way to share code between the web_app
// and shadowbox.
interface DataUsageByAccessKeyJson {
// The accessKeyId should be of type AccessKeyId, however that results in the tsc
// error TS1023: An index signature parameter type must be 'string' or 'number'.
// See https://github.com/Microsoft/TypeScript/issues/2491
// TODO: this still says "UserId", changing to "AccessKeyId" will require
// a change on the shadowbox server.
bytesTransferredByUserId: {[accessKeyId: string]: number};
}

// Converts the access key JSON from the API to its model.
function makeAccessKeyModel(apiAccessKey: AccessKeyJson): server.AccessKey {
return apiAccessKey as server.AccessKey;
Expand Down Expand Up @@ -149,16 +136,54 @@ export class ShadowboxServer implements server.Server {
await this.api.request<void>(`access-keys/${keyId}/data-limit`, 'DELETE');
}

async getDataUsage(): Promise<server.BytesByAccessKey> {
const jsonResponse =
await this.api.request<DataUsageByAccessKeyJson>('metrics/transfer');
const usageMap = new Map<server.AccessKeyId, number>();
for (const [accessKeyId, bytes] of Object.entries(
jsonResponse.bytesTransferredByUserId
)) {
usageMap.set(accessKeyId, bytes ?? 0);
}
return usageMap;
async getServerMetrics(): Promise<server.ServerMetricsJson> {
//TODO: this.api.request<server.ServerMetricsJson>('server/metrics')
return {
servers: [
{
location: 'CA',
asn: 1,
asOrg: 'IDK',
tunnelTime: {
seconds: 10000,
},
dataTransferred: {
bytes: 10000,
},
},
{
location: 'US',
asn: 2,
asOrg: 'WHATEVER',
tunnelTime: {
seconds: 200000,
},
dataTransferred: {
bytes: 200000,
},
},
],
accessKeys: [
{
accessKeyId: 0,
tunnelTime: {
seconds: 10000,
},
dataTransferred: {
bytes: 10000,
},
},
{
accessKeyId: 1,
tunnelTime: {
seconds: 200000,
},
dataTransferred: {
bytes: 200000,
},
},
],
};
}

getName(): string {
Expand Down
28 changes: 28 additions & 0 deletions server_manager/www/testing/models.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,34 @@ export class FakeServer implements server.Server {
getDataUsage() {
return Promise.resolve(new Map<server.AccessKeyId, number>());
}
getServerMetrics() {
return Promise.resolve({
servers: [
{
location: '',
asn: 0,
asOrg: '',
tunnelTime: {
seconds: 0,
},
dataTransferred: {
bytes: 0,
},
},
],
accessKeys: [
{
accessKeyId: 0,
tunnelTime: {
seconds: 0,
},
dataTransferred: {
bytes: 0,
},
},
],
});
}
addAccessKey() {
const accessKey = {
id: Math.floor(Math.random()).toString(),
Expand Down
Loading
Loading