Skip to content

Commit 3fd589f

Browse files
surbhigarg92surbhigarg92
committed
fixed the metricstracerfactory instance, it is per projectid now
1 parent e8223b0 commit 3fd589f

File tree

11 files changed

+86
-40
lines changed

11 files changed

+86
-40
lines changed

‎src/index.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,7 @@ class Spanner extends GrpcService {
314314
instances_: Map<string, Instance>;
315315
instanceConfigs_: Map<string, InstanceConfig>;
316316
projectIdReplaced_: boolean;
317+
projectId_?: string;
317318
projectFormattedName_: string;
318319
commonHeaders_: {[k: string]: string};
319320
routeToLeaderEnabled = true;
@@ -1637,13 +1638,14 @@ class Spanner extends GrpcService {
16371638
!disableBuiltInMetrics &&
16381639
!this._isInSecureCredentials;
16391640
MetricsTracerFactory.enabled = metricsEnabled;
1640-
if (projectId === null || projectId === '') {
1641+
if (!projectId) {
16411642
console.error(
16421643
'Unable to get Project Id for client side metrics, will skip exporting client' +
16431644
' side metrics',
16441645
);
16451646
return;
16461647
}
1648+
this.projectId_ = projectId;
16471649
if (metricsEnabled) {
16481650
const factory = MetricsTracerFactory.getInstance(projectId);
16491651
const periodicReader = new PeriodicExportingMetricReader({
@@ -1812,9 +1814,9 @@ class Spanner extends GrpcService {
18121814
// eslint-disable-next-line @typescript-eslint/no-explicit-any
18131815
request(config: any, callback?: any): any {
18141816
let metricsTracer: MetricsTracer | null = null;
1815-
if (config.client === 'SpannerClient') {
1817+
if (config.client === 'SpannerClient' && this.projectId_) {
18161818
metricsTracer =
1817-
MetricsTracerFactory?.getInstance()?.createMetricsTracer(
1819+
MetricsTracerFactory?.getInstance(this.projectId_)?.createMetricsTracer(
18181820
config.method,
18191821
config.reqOpts.session ?? config.reqOpts.database,
18201822
config.headers['x-goog-spanner-request-id'],
@@ -1876,9 +1878,9 @@ class Spanner extends GrpcService {
18761878
// eslint-disable-next-line @typescript-eslint/no-explicit-any
18771879
requestStream(config): any {
18781880
let metricsTracer: MetricsTracer | null = null;
1879-
if (config.client === 'SpannerClient') {
1881+
if (config.client === 'SpannerClient' && this.projectId_) {
18801882
metricsTracer =
1881-
MetricsTracerFactory?.getInstance()?.createMetricsTracer(
1883+
MetricsTracerFactory?.getInstance(this.projectId_)?.createMetricsTracer(
18821884
config.method,
18831885
config.reqOpts.session ?? config.reqOpts.database,
18841886
config.headers['x-goog-spanner-request-id'],

‎src/metrics/interceptor.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,16 @@ export const MetricInterceptor = (options, nextCall) => {
3030
return new grpc.InterceptingCall(nextCall(options), {
3131
start: function (metadata, listener, next) {
3232
// Record attempt metric on request start
33-
const factory = MetricsTracerFactory.getInstance();
33+
// TODO
34+
const resourcePrefix = metadata.get(
35+
'google-cloud-resource-prefix',
36+
)[0] as string;
37+
const match = resourcePrefix?.match(/^projects\/([^/]+)\//);
38+
const projectId = match ? match[1] : undefined;
39+
let factory;
40+
if (projectId) {
41+
factory = MetricsTracerFactory.getInstance(projectId);
42+
}
3443
const requestId = metadata.get('x-goog-spanner-request-id')[0] as string;
3544
const metricsTracer = factory?.getCurrentTracer(requestId);
3645
metricsTracer?.recordAttemptStart();

‎src/metrics/metrics-tracer-factory.ts‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ const version = require('../../../package.json').version;
3838
* globally via the static `enabled` property, that is set from the SpannerClient.
3939
*/
4040
export class MetricsTracerFactory {
41-
private static _instance: MetricsTracerFactory | null = null;
41+
private static _instances: Map<string, MetricsTracerFactory> = new Map();
4242
private _meterProvider: MeterProvider | null = null;
4343
private _instrumentAttemptCounter!: Counter;
4444
private _instrumentAttemptLatency!: Histogram;
@@ -56,7 +56,6 @@ export class MetricsTracerFactory {
5656
private _currentOperationTracers = new Map();
5757
private _currentOperationLastUpdatedMs = new Map();
5858
private _intervalTracerCleanup: NodeJS.Timeout;
59-
public static _readers: MetricReader[] = [];
6059
public static enabled = true;
6160

6261
/**
@@ -101,16 +100,19 @@ export class MetricsTracerFactory {
101100
* @param projectId Optional GCP project ID for the factory instantiation. Does nothing for subsequent calls.
102101
* @returns The singleton MetricsTracerFactory instance or null if disabled.
103102
*/
104-
public static getInstance(projectId = ''): MetricsTracerFactory | null {
103+
public static getInstance(projectId: string): MetricsTracerFactory | null {
105104
if (!MetricsTracerFactory.enabled) {
106105
return null;
107106
}
108107

109108
// Create a singleton instance, enabling/disabling metrics can only be done on the initial call
110-
if (MetricsTracerFactory._instance === null) {
111-
MetricsTracerFactory._instance = new MetricsTracerFactory(projectId);
109+
if (!MetricsTracerFactory._instances.has(projectId)) {
110+
MetricsTracerFactory._instances.set(
111+
projectId,
112+
new MetricsTracerFactory(projectId),
113+
);
112114
}
113-
return MetricsTracerFactory!._instance;
115+
return MetricsTracerFactory._instances.get(projectId)!;
114116
}
115117

116118
/**
@@ -128,7 +130,6 @@ export class MetricsTracerFactory {
128130
[Constants.MONITORED_RES_LABEL_KEY_INSTANCE]: 'unknown',
129131
[Constants.MONITORED_RES_LABEL_KEY_INSTANCE_CONFIG]: 'unknown',
130132
});
131-
MetricsTracerFactory._readers = readers;
132133
this._meterProvider = new MeterProvider({
133134
resource: resource,
134135
readers: readers,
@@ -142,10 +143,21 @@ export class MetricsTracerFactory {
142143
/**
143144
* Resets the singleton instance of the MetricsTracerFactory.
144145
*/
145-
public static async resetInstance() {
146-
clearInterval(MetricsTracerFactory._instance?._intervalTracerCleanup);
147-
await MetricsTracerFactory._instance?.resetMeterProvider();
148-
MetricsTracerFactory._instance = null;
146+
public static async resetInstance(projectId?: string) {
147+
if (projectId) {
148+
const instance = MetricsTracerFactory._instances.get(projectId);
149+
if (instance) {
150+
clearInterval(instance._intervalTracerCleanup);
151+
await instance.resetMeterProvider();
152+
MetricsTracerFactory._instances.delete(projectId);
153+
}
154+
} else {
155+
for (const instance of MetricsTracerFactory._instances.values()) {
156+
clearInterval(instance._intervalTracerCleanup);
157+
await instance.resetMeterProvider();
158+
}
159+
MetricsTracerFactory._instances.clear();
160+
}
149161
}
150162

151163
/**
@@ -250,6 +262,7 @@ export class MetricsTracerFactory {
250262
MetricsTracerFactory.enabled,
251263
database,
252264
instance,
265+
this._projectId,
253266
method,
254267
operationRequest,
255268
);

‎src/metrics/metrics-tracer.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ export class MetricsTracer {
163163
public enabled: boolean,
164164
private _database: string,
165165
private _instance: string,
166+
private _projectId: string,
166167
private _methodName: string,
167168
private _request: string,
168169
) {
@@ -276,7 +277,9 @@ export class MetricsTracer {
276277
operationLatencyMilliseconds,
277278
operationAttributes,
278279
);
279-
MetricsTracerFactory.getInstance()!.clearCurrentTracer(this._request);
280+
MetricsTracerFactory.getInstance(this._projectId)!.clearCurrentTracer(
281+
this._request,
282+
);
280283
}
281284

282285
/**

‎src/metrics/spanner-metrics-exporter.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@ export const MAX_BATCH_EXPORT_SIZE = 200;
2828
* Format and sends metrics information to Google Cloud Monitoring.
2929
*/
3030
export class CloudMonitoringMetricsExporter implements PushMetricExporter {
31-
private _projectId: string | undefined;
31+
private _projectId: string;
3232
private _lastExported: Date = new Date(0);
3333
private readonly _client: MetricServiceClient;
3434
private _metricsExportFailureLogged = false;
3535

36-
constructor({auth}: ExporterOptions, projectId: string | undefined) {
36+
constructor({auth}: ExporterOptions, projectId: string) {
3737
this._client = new MetricServiceClient({auth: auth});
3838

3939
this._projectId = projectId;
@@ -79,8 +79,10 @@ export class CloudMonitoringMetricsExporter implements PushMetricExporter {
7979
private async _exportAsync(
8080
resourceMetrics: ResourceMetrics,
8181
): Promise<ExportResult> {
82-
const timeSeriesList =
83-
transformResourceMetricToTimeSeriesArray(resourceMetrics);
82+
const timeSeriesList = transformResourceMetricToTimeSeriesArray(
83+
resourceMetrics,
84+
this._projectId,
85+
);
8486

8587
let failure: {sendFailed: false} | {sendFailed: true; error: Error} = {
8688
sendFailed: false,

‎src/metrics/transform.ts‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ function _transformValueType(metric: MetricData): ValueType {
9090
*/
9191
export function transformResourceMetricToTimeSeriesArray(
9292
resourceMetrics: ResourceMetrics,
93+
projectId: string,
9394
) {
9495
const resource = resourceMetrics?.resource;
9596
const scopeMetrics = resourceMetrics?.scopeMetrics;
@@ -107,7 +108,7 @@ export function transformResourceMetricToTimeSeriesArray(
107108
// Flatmap the data points in each metric to create a TimeSeries for each point
108109
.flatMap(metric =>
109110
metric.dataPoints.flatMap(dataPoint =>
110-
_createTimeSeries(metric, dataPoint, resource),
111+
_createTimeSeries(metric, dataPoint, resource, projectId),
111112
),
112113
)
113114
);
@@ -119,14 +120,15 @@ export function transformResourceMetricToTimeSeriesArray(
119120
function _createTimeSeries<T>(
120121
metric: MetricData,
121122
dataPoint: DataPoint<T>,
122-
resource?: Resource,
123+
resource: Resource,
124+
projectId: string,
123125
) {
124126
const type = path.posix.join(CLIENT_METRICS_PREFIX, metric.descriptor.name);
125127
const resourceLabels = resource
126-
? _extractLabels(resource)
128+
? _extractLabels(resource, projectId)
127129
: {metricLabels: {}, monitoredResourceLabels: {}};
128130

129-
const dataLabels = _extractLabels(dataPoint);
131+
const dataLabels = _extractLabels(dataPoint, projectId);
130132

131133
const labels = {
132134
...resourceLabels.metricLabels,
@@ -205,8 +207,11 @@ function _transformPoint<T>(metric: MetricData, dataPoint: DataPoint<T>) {
205207
}
206208

207209
/** Extracts metric and monitored resource labels from data point */
208-
function _extractLabels<T>({attributes = {}}: DataPoint<T> | Resource) {
209-
const factory = MetricsTracerFactory.getInstance();
210+
function _extractLabels<T>(
211+
{attributes = {}}: DataPoint<T> | Resource,
212+
projectId: string,
213+
) {
214+
const factory = MetricsTracerFactory.getInstance(projectId);
210215
// Add Client name and Client UID metric labels
211216
attributes[METRIC_LABEL_KEY_CLIENT_UID] =
212217
factory?.clientUid ?? UNKNOWN_ATTRIBUTE;

‎test/metrics/interceptor.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,10 @@ describe('MetricInterceptor', () => {
117117
},
118118
};
119119
testMetadata = new grpc.Metadata();
120+
testMetadata.set(
121+
'google-cloud-resource-prefix',
122+
'projects/test-project/instances/instance/databases/database-1',
123+
);
120124
});
121125

122126
afterEach(() => {

‎test/metrics/metrics-tracer-factory.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ describe('MetricsTracerFactory', () => {
9393
});
9494

9595
it('should use the set meter provider', async () => {
96-
const factory = MetricsTracerFactory.getInstance();
96+
const factory = MetricsTracerFactory.getInstance('project-id');
9797
const tracer = factory!.createMetricsTracer(
9898
'some-method',
9999
'projects/project/instances/instance/databases/database',
@@ -126,7 +126,7 @@ describe('MetricsTracerFactory', () => {
126126
});
127127

128128
it('should initialize metric instruments when enabled', () => {
129-
const factory = MetricsTracerFactory.getInstance();
129+
const factory = MetricsTracerFactory.getInstance('project-id');
130130

131131
assert.deepStrictEqual(factory!.instrumentAttemptLatency, {
132132
record: recordAttemptLatencyStub,
@@ -149,7 +149,7 @@ describe('MetricsTracerFactory', () => {
149149
});
150150

151151
it('should create a MetricsTracer instance', () => {
152-
const factory = MetricsTracerFactory.getInstance();
152+
const factory = MetricsTracerFactory.getInstance('project-id');
153153
const tracer = factory!.createMetricsTracer(
154154
'some-method',
155155
'method-name',
@@ -159,7 +159,7 @@ describe('MetricsTracerFactory', () => {
159159
});
160160

161161
it('should correctly set default attributes', () => {
162-
const factory = MetricsTracerFactory.getInstance();
162+
const factory = MetricsTracerFactory.getInstance('project-id');
163163
const tracer = factory!.createMetricsTracer(
164164
'test-method',
165165
'projects/project/instances/instance/databases/database',

‎test/metrics/metrics-tracer.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {Spanner} from '../../src';
2323

2424
const DATABASE = 'test-db';
2525
const INSTANCE = 'instance';
26+
const PROJECT_ID = 'project_id';
2627
const METHOD = 'test-method';
2728
const REQUEST = 'test-request';
2829

@@ -83,6 +84,7 @@ describe('MetricsTracer', () => {
8384
true, // enabled,
8485
DATABASE,
8586
INSTANCE,
87+
PROJECT_ID,
8688
METHOD,
8789
REQUEST,
8890
);
@@ -249,6 +251,7 @@ describe('MetricsTracer', () => {
249251
true,
250252
DATABASE,
251253
INSTANCE,
254+
PROJECT_ID,
252255
METHOD,
253256
REQUEST,
254257
);

‎test/metrics/metrics.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ describe('Test metrics with mock server', () => {
4242
const selectSql = 'SELECT NUM, NAME FROM NUMBERS';
4343
const server = new grpc.Server();
4444
const spannerMock = mock.createMockSpanner(server);
45+
const PROJECT_ID = 'test-project';
4546

4647
class InMemoryMetricReader extends MetricReader {
4748
protected async onForceFlush(): Promise<void> {}
@@ -158,7 +159,7 @@ describe('Test metrics with mock server', () => {
158159
await MetricsTracerFactory.resetInstance();
159160
MetricsTracerFactory.enabled = true;
160161
spanner = new Spanner({
161-
projectId: 'test-project',
162+
projectId: PROJECT_ID,
162163
servicePath: 'localhost',
163164
port,
164165
sslCreds: grpc.credentials.createInsecure(),
@@ -210,7 +211,7 @@ describe('Test metrics with mock server', () => {
210211
spannerMock.removeExecutionTimes();
211212
// Reset the MetricsFactoryReader to an in-memory reader for the tests
212213
MetricsTracerFactory.enabled = true;
213-
factory = MetricsTracerFactory.getInstance();
214+
factory = MetricsTracerFactory.getInstance(PROJECT_ID);
214215
await factory!.resetMeterProvider();
215216
reader = new InMemoryMetricReader();
216217
factory!.getMeterProvider([reader]);

0 commit comments

Comments
 (0)