From 918f45457091776f1e4cc8939c2c0149f8ceec6c Mon Sep 17 00:00:00 2001 From: nicoleczhu Date: Tue, 24 Nov 2020 14:29:44 -0800 Subject: [PATCH 1/3] feat!: getEntries filters 24 hour timestamp by default --- src/index.ts | 21 +++++++++++++++------ test/index.ts | 39 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/src/index.ts b/src/index.ts index b41c9f82..8cc7a9a2 100644 --- a/src/index.ts +++ b/src/index.ts @@ -462,6 +462,7 @@ class Logging { return new Entry(resource, data); } + // TODO: nicole fix this one getEntries(options?: GetEntriesRequest): Promise; getEntries(callback: GetEntriesCallback): void; getEntries(options: GetEntriesRequest, callback: GetEntriesCallback): void; @@ -552,12 +553,20 @@ class Logging { opts?: GetEntriesRequest | GetEntriesCallback ): Promise { const options = opts ? (opts as GetEntriesRequest) : {}; - const reqOpts = extend( - { - orderBy: 'timestamp desc', - }, - options - ); + + // By default, sort entries by descending timestamp + let reqOpts = extend({orderBy: 'timestamp desc'}, options); + + // By default, filter entries to last 24 hours only + const time = new Date(); + time.setDate(time.getDate() - 1); + const timeFilter = `timestamp >= "${time.toISOString()}"`; + if (!options.filter) { + reqOpts = extend({filter: timeFilter}, reqOpts); + } else if (!options.filter.includes('timestamp')) { + reqOpts.filter += ` AND ${timeFilter}`; + } + reqOpts.resourceNames = arrify(reqOpts.resourceNames!); this.projectId = await this.auth.getProjectId(); const resourceName = 'projects/' + this.projectId; diff --git a/test/index.ts b/test/index.ts index edd7bf43..67df7253 100644 --- a/test/index.ts +++ b/test/index.ts @@ -103,6 +103,12 @@ const originalFakeUtil = extend(true, {}, fakeUtil); function fakeV2() {} +function getDefaultTimeFilter(): string { + const time = new Date(); + time.setDate(time.getDate() - 1); + return `timestamp >= "${time.toISOString()}"`; +} + class FakeEntry { calledWith_: IArguments; constructor() { @@ -477,12 +483,13 @@ describe('Logging', () => { logging.auth.getProjectId = async () => PROJECT_ID; }); - it('should exec without options', async () => { + it('should exec without options (with defaults)', async () => { logging.loggingService.listLogEntries = async ( reqOpts: {}, gaxOpts: {} ) => { assert.deepStrictEqual(reqOpts, { + filter: `${getDefaultTimeFilter()}`, orderBy: 'timestamp desc', resourceNames: ['projects/' + logging.projectId], }); @@ -495,7 +502,32 @@ describe('Logging', () => { await logging.getEntries(); }); - it('should accept options', async () => { + it('should accept options (and not overwrite timestamp)', async () => { + const options = {filter: 'timestamp > "2020-11-11T15:01:23.045123456Z"'}; + + logging.loggingService.listLogEntries = async ( + reqOpts: {}, + gaxOpts: {} + ) => { + assert.deepStrictEqual( + reqOpts, + extend(options, { + filter: 'timestamp > "2020-11-11T15:01:23.045123456Z"', + orderBy: 'timestamp desc', + resourceNames: ['projects/' + logging.projectId], + }) + ); + + assert.deepStrictEqual(gaxOpts, { + autoPaginate: undefined, + }); + return [[]]; + }; + + await logging.getEntries(options); + }); + + it('should append default timestamp to existing filters', async () => { const options = {filter: 'test'}; logging.loggingService.listLogEntries = async ( @@ -505,7 +537,7 @@ describe('Logging', () => { assert.deepStrictEqual( reqOpts, extend(options, { - filter: 'test', + filter: `test AND ${getDefaultTimeFilter()}`, orderBy: 'timestamp desc', resourceNames: ['projects/' + logging.projectId], }) @@ -567,6 +599,7 @@ describe('Logging', () => { assert.deepStrictEqual(reqOpts, { a: 'b', c: 'd', + filter: `${getDefaultTimeFilter()}`, orderBy: 'timestamp desc', resourceNames: ['projects/' + logging.projectId], }); From aac80c9e81656ca0d05c18dcfdfb57e7cad35b20 Mon Sep 17 00:00:00 2001 From: nicoleczhu Date: Tue, 24 Nov 2020 14:31:23 -0800 Subject: [PATCH 2/3] chore: clean up comments --- src/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index 8cc7a9a2..3d08bc1f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -462,7 +462,6 @@ class Logging { return new Entry(resource, data); } - // TODO: nicole fix this one getEntries(options?: GetEntriesRequest): Promise; getEntries(callback: GetEntriesCallback): void; getEntries(options: GetEntriesRequest, callback: GetEntriesCallback): void; From 891f63ab952e2df473d172964b52eebceae0f3c1 Mon Sep 17 00:00:00 2001 From: nicoleczhu Date: Tue, 24 Nov 2020 15:28:57 -0800 Subject: [PATCH 3/3] test: fix tests breaking on millisecond difference --- test/index.ts | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/test/index.ts b/test/index.ts index 67df7253..fce53c12 100644 --- a/test/index.ts +++ b/test/index.ts @@ -103,12 +103,6 @@ const originalFakeUtil = extend(true, {}, fakeUtil); function fakeV2() {} -function getDefaultTimeFilter(): string { - const time = new Date(); - time.setDate(time.getDate() - 1); - return `timestamp >= "${time.toISOString()}"`; -} - class FakeEntry { calledWith_: IArguments; constructor() { @@ -485,17 +479,19 @@ describe('Logging', () => { it('should exec without options (with defaults)', async () => { logging.loggingService.listLogEntries = async ( - reqOpts: {}, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + reqOpts: any, gaxOpts: {} ) => { assert.deepStrictEqual(reqOpts, { - filter: `${getDefaultTimeFilter()}`, + filter: reqOpts?.filter, orderBy: 'timestamp desc', resourceNames: ['projects/' + logging.projectId], }); assert.deepStrictEqual(gaxOpts, { autoPaginate: undefined, }); + assert.ok(reqOpts?.filter.includes('timestamp')); return [[]]; }; @@ -531,21 +527,22 @@ describe('Logging', () => { const options = {filter: 'test'}; logging.loggingService.listLogEntries = async ( - reqOpts: {}, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + reqOpts: any, gaxOpts: {} ) => { assert.deepStrictEqual( reqOpts, extend(options, { - filter: `test AND ${getDefaultTimeFilter()}`, + filter: reqOpts?.filter, orderBy: 'timestamp desc', resourceNames: ['projects/' + logging.projectId], }) ); - assert.deepStrictEqual(gaxOpts, { autoPaginate: undefined, }); + assert.ok(reqOpts?.filter.includes('test AND timestamp')); return [[]]; }; @@ -599,13 +596,14 @@ describe('Logging', () => { assert.deepStrictEqual(reqOpts, { a: 'b', c: 'd', - filter: `${getDefaultTimeFilter()}`, + filter: reqOpts?.filter, orderBy: 'timestamp desc', resourceNames: ['projects/' + logging.projectId], }); // eslint-disable-next-line @typescript-eslint/no-explicit-any assert.strictEqual((reqOpts as any).gaxOptions, undefined); assert.deepStrictEqual(gaxOpts, options.gaxOptions); + assert.ok(reqOpts?.filter.includes('timestamp')); return [[]]; };