Skip to content

Commit

Permalink
refactor(server): remove unneeded access key metrics ID (Jigsaw-Code#…
Browse files Browse the repository at this point in the history
…1548)

* refactor(server): remove unneeded access key metrics ID

* One more.

* Remove unused imports.
  • Loading branch information
sbruens authored May 16, 2024
1 parent 99b1a0e commit 9a34981
Show file tree
Hide file tree
Showing 5 changed files with 2 additions and 40 deletions.
5 changes: 0 additions & 5 deletions src/shadowbox/model/access_key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
// limitations under the License.

export type AccessKeyId = string;
export type AccessKeyMetricsId = string;

// Parameters needed to access a Shadowsocks proxy.
export interface ProxyParams {
Expand All @@ -38,8 +37,6 @@ export interface AccessKey {
readonly id: AccessKeyId;
// Admin-controlled, editable name for this access key.
readonly name: string;
// Used in metrics reporting to decouple from the real id. Can change.
readonly metricsId: AccessKeyMetricsId;
// Parameters to access the proxy
readonly proxyParams: ProxyParams;
// Whether the access key has exceeded the data transfer limit.
Expand Down Expand Up @@ -78,8 +75,6 @@ export interface AccessKeyRepository {
setHostname(hostname: string): void;
// Apply the specified update to the specified access key. Throws on failure.
renameAccessKey(id: AccessKeyId, name: string): void;
// Gets the metrics id for a given Access Key.
getMetricsId(id: AccessKeyId): AccessKeyMetricsId | undefined;
// Sets a data transfer limit for all access keys.
setDefaultDataLimit(limit: DataLimit): void;
// Removes the access key data transfer limit.
Expand Down
9 changes: 0 additions & 9 deletions src/shadowbox/server/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import * as json_config from '../infrastructure/json_config';
import * as logging from '../infrastructure/logging';
import {PrometheusClient, startPrometheus} from '../infrastructure/prometheus_scraper';
import {RolloutTracker} from '../infrastructure/rollout';
import {AccessKeyId} from '../model/access_key';
import * as version from './version';

import {PrometheusManagerMetrics} from './manager_metrics';
Expand Down Expand Up @@ -216,21 +215,13 @@ async function main() {
);

const metricsReader = new PrometheusUsageMetrics(prometheusClient);
const toMetricsId = (id: AccessKeyId) => {
try {
return accessKeyRepository.getMetricsId(id);
} catch (e) {
logging.warn(`Failed to get metrics id for access key ${id}: ${e}`);
}
};
const managerMetrics = new PrometheusManagerMetrics(prometheusClient);
const metricsCollector = new RestMetricsCollectorClient(metricsCollectorUrl);
const metricsPublisher: SharedMetricsPublisher = new OutlineSharedMetricsPublisher(
new RealClock(),
serverConfig,
accessKeyConfig,
metricsReader,
toMetricsId,
metricsCollector
);
const managerService = new ShadowsocksManagerService(
Expand Down
14 changes: 1 addition & 13 deletions src/shadowbox/server/server_access_key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
// limitations under the License.

import * as randomstring from 'randomstring';
import * as uuidv4 from 'uuid/v4';

import {Clock} from '../infrastructure/clock';
import {isPortUsed} from '../infrastructure/get_port';
Expand All @@ -24,7 +23,6 @@ import {
AccessKey,
AccessKeyCreateParams,
AccessKeyId,
AccessKeyMetricsId,
AccessKeyRepository,
DataLimit,
ProxyParams,
Expand All @@ -36,7 +34,6 @@ import {PrometheusManagerMetrics} from './manager_metrics';
// The format as json of access keys in the config file.
interface AccessKeyStorageJson {
id: AccessKeyId;
metricsId: AccessKeyId;
name: string;
password: string;
port: number;
Expand All @@ -57,7 +54,6 @@ class ServerAccessKey implements AccessKey {
constructor(
readonly id: AccessKeyId,
public name: string,
public metricsId: AccessKeyMetricsId,
readonly proxyParams: ProxyParams,
public dataLimit?: DataLimit
) {}
Expand All @@ -79,7 +75,6 @@ function makeAccessKey(hostname: string, accessKeyJson: AccessKeyStorageJson): A
return new ServerAccessKey(
accessKeyJson.id,
accessKeyJson.name,
accessKeyJson.metricsId,
proxyParams,
accessKeyJson.dataLimit
);
Expand All @@ -88,7 +83,6 @@ function makeAccessKey(hostname: string, accessKeyJson: AccessKeyStorageJson): A
function accessKeyToStorageJson(accessKey: AccessKey): AccessKeyStorageJson {
return {
id: accessKey.id,
metricsId: accessKey.metricsId,
name: accessKey.name,
password: accessKey.proxyParams.password,
port: accessKey.proxyParams.portNumber,
Expand Down Expand Up @@ -213,7 +207,6 @@ export class ServerAccessKeyRepository implements AccessKeyRepository {
throw new errors.PasswordConflict(id);
}

const metricsId = uuidv4();
const password = params?.password ?? generatePassword();

const encryptionMethod = params?.encryptionMethod || this.NEW_USER_ENCRYPTION_METHOD;
Expand Down Expand Up @@ -241,7 +234,7 @@ export class ServerAccessKeyRepository implements AccessKeyRepository {
};
const name = params?.name ?? '';
const dataLimit = params?.dataLimit;
const accessKey = new ServerAccessKey(id, name, metricsId, proxyParams, dataLimit);
const accessKey = new ServerAccessKey(id, name, proxyParams, dataLimit);
this.accessKeys.push(accessKey);
this.saveAccessKeys();
await this.updateServer();
Expand Down Expand Up @@ -306,11 +299,6 @@ export class ServerAccessKeyRepository implements AccessKeyRepository {
this.enforceAccessKeyDataLimits();
}

getMetricsId(id: AccessKeyId): AccessKeyMetricsId | undefined {
const accessKey = this.getAccessKey(id);
return accessKey ? accessKey.metricsId : undefined;
}

// Compares access key usage with collected metrics, marking them as under or over limit.
// Updates access key data usage.
async enforceAccessKeyDataLimits() {
Expand Down
11 changes: 1 addition & 10 deletions src/shadowbox/server/shared_metrics.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

import {ManualClock} from '../infrastructure/clock';
import {InMemoryConfig} from '../infrastructure/json_config';
import {AccessKeyId, DataLimit} from '../model/access_key';
import {DataLimit} from '../model/access_key';
import * as version from './version';
import {AccessKeyConfigJson} from './server_access_key';

Expand All @@ -38,7 +38,6 @@ describe('OutlineSharedMetricsPublisher', () => {
serverConfig,
null,
null,
null,
null
);
expect(publisher.isSharingEnabled()).toBeFalsy();
Expand All @@ -58,7 +57,6 @@ describe('OutlineSharedMetricsPublisher', () => {
serverConfig,
null,
null,
null,
null
);
expect(publisher.isSharingEnabled()).toBeTruthy();
Expand All @@ -70,14 +68,12 @@ describe('OutlineSharedMetricsPublisher', () => {
let startTime = clock.nowMs;
const serverConfig = new InMemoryConfig<ServerConfigJson>({serverId: 'server-id'});
const usageMetrics = new ManualUsageMetrics();
const toMetricsId = (id: AccessKeyId) => `M(${id})`;
const metricsCollector = new FakeMetricsCollector();
const publisher = new OutlineSharedMetricsPublisher(
clock,
serverConfig,
null,
usageMetrics,
toMetricsId,
metricsCollector
);

Expand Down Expand Up @@ -130,14 +126,12 @@ describe('OutlineSharedMetricsPublisher', () => {
const startTime = clock.nowMs;
const serverConfig = new InMemoryConfig<ServerConfigJson>({serverId: 'server-id'});
const usageMetrics = new ManualUsageMetrics();
const toMetricsId = (id: AccessKeyId) => `M(${id})`;
const metricsCollector = new FakeMetricsCollector();
const publisher = new OutlineSharedMetricsPublisher(
clock,
serverConfig,
null,
usageMetrics,
toMetricsId,
metricsCollector
);

Expand Down Expand Up @@ -177,7 +171,6 @@ describe('OutlineSharedMetricsPublisher', () => {
const makeKeyJson = (dataLimit?: DataLimit) => {
return {
id: (keyId++).toString(),
metricsId: 'id',
name: 'name',
password: 'pass',
port: 12345,
Expand All @@ -193,7 +186,6 @@ describe('OutlineSharedMetricsPublisher', () => {
serverConfig,
keyConfig,
new ManualUsageMetrics(),
(_id: AccessKeyId) => '',
metricsCollector
);

Expand Down Expand Up @@ -242,7 +234,6 @@ describe('OutlineSharedMetricsPublisher', () => {
serverConfig,
new InMemoryConfig<AccessKeyConfigJson>({}),
new ManualUsageMetrics(),
(_id: AccessKeyId) => '',
metricsCollector
);

Expand Down
3 changes: 0 additions & 3 deletions src/shadowbox/server/shared_metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import * as follow_redirects from '../infrastructure/follow_redirects';
import {JsonConfig} from '../infrastructure/json_config';
import * as logging from '../infrastructure/logging';
import {PrometheusClient} from '../infrastructure/prometheus_scraper';
import {AccessKeyId, AccessKeyMetricsId} from '../model/access_key';
import * as version from './version';
import {AccessKeyConfigJson} from './server_access_key';

Expand Down Expand Up @@ -148,14 +147,12 @@ export class OutlineSharedMetricsPublisher implements SharedMetricsPublisher {
// serverConfig: where the enabled/disable setting is persisted
// keyConfig: where access keys are persisted
// usageMetrics: where we get the metrics from
// toMetricsId: maps Access key ids to metric ids
// metricsUrl: where to post the metrics
constructor(
private clock: Clock,
private serverConfig: JsonConfig<ServerConfigJson>,
private keyConfig: JsonConfig<AccessKeyConfigJson>,
usageMetrics: UsageMetrics,
private toMetricsId: (accessKeyId: AccessKeyId) => AccessKeyMetricsId,
private metricsCollector: MetricsCollectorClient
) {
// Start timer
Expand Down

0 comments on commit 9a34981

Please sign in to comment.