feat: Introduce provisioning of Logs Table for new and existing users #643

Kevin101Zhang · 2024-04-05T20:06:30Z

First few commits is from this branch #640. Created this branch based off the initial branch.

This PR intends to introduce the creation of the logs table by provisioning the logsSchema and the follow CRON jobs for new Users but does not use or writeLogs to the new logsTable itself.

If this is merged by itself new users will have unused log table but the provisioning will occur.
To provision existing users - #636

removed useless test

…omnearqueryapipull608' of https://github.com/near/queryapi into 641-introduce-provisioning-of-logs

…omnearqueryapipull608' into 641-introduce-provisioning-of-logs

Kevin101Zhang · 2024-04-10T18:17:22Z

runner/src/indexer/indexer.ts

      }
    }
    return allMutations;
  }

+  // async getDatabaseConnectionParams(hasuraRoleName: string): Promise<DatabaseConnectionParameters>  {


Discussed with @morgsmccauley a method used for integration test to point to the proper port and host.

Huh is this like a stop gap solution? I'm wondering what the problem is here and if we can modify the function/class to support both use cases somehow.

…om/near/queryapi into 641-introduce-provisioning-of-logs

Kevin101Zhang · 2024-04-10T18:38:30Z

PR introduces provisioning for new users. With Morgans PR #636 that has also been merged from main into this PR and uncommented out we also provision existing user's log table conditionally.

PR mainly includes uncommenting out provisioning.ts code and removing the skip from provisioning test.
Added tested (locally and unit/integration) commented out code in Indexer.ts
Updating unit and integration test (commented out)

Kevin101Zhang · 2024-04-10T19:02:24Z

runner/src/indexer/indexer.test.ts

@@ -955,8 +965,9 @@ CREATE TABLE
      getDatabaseConnectionParameters: jest.fn().mockReturnValue(genericDbCredentials),
      fetchUserApiProvisioningStatus: jest.fn().mockReturnValue(true),
      provisionUserApi: jest.fn(),
+      provisionLogsIfNeeded: jest.fn(),


@morgsmccauley I added a few mocks in the test cases for the #636 pass test.

darunrs

Have you run this locally as well? Something like:

Create indexer with old code.
Run indexer with new code and confirm logs table provisioned correctly
Create brand new indexer and confirm logs table provisioned correctly
Ran both of those indexers with logs and verified logs appeared in both

If so, can approve.

runner/src/indexer/indexer.ts

darunrs · 2024-04-10T20:15:23Z

runner/src/indexer/indexer.ts

          this.database_connection_parameters ??= await this.deps.provisioner.getDatabaseConnectionParameters(hasuraRoleName) as DatabaseConnectionParameters;
-          // this.indexer_logger ??= new IndexerLogger(functionName, this.indexer_behavior.log_level, this.database_connection_parameters);
+          // this.database_connection_parameters = await this.getDatabaseConnectionParams(hasuraRoleName);


Why is there a get db conn params function owned by Indexer in addition to the one in provisioner?

darunrs · 2024-04-10T20:19:34Z

runner/src/indexer/indexer.ts

      }
    }
    return allMutations;
  }

+  // async getDatabaseConnectionParams(hasuraRoleName: string): Promise<DatabaseConnectionParameters>  {


Huh is this like a stop gap solution? I'm wondering what the problem is here and if we can modify the function/class to support both use cases somehow.

darunrs · 2024-04-10T20:21:38Z

runner/src/indexer/indexer.ts

@@ -463,13 +492,12 @@ export default class Indexer {

  // async writeLog (logEntry: LogEntry, logEntries: LogEntry[], functionName: string): Promise<any> {


We don't need to pass in functionName here btw. We can sue the class variable this.indexerConfig to get the function name or full name or whatever it is you need.

Ah I see this PR. Are you ok if I just add a separate ticket for this for when that PR is merged to directly use indexerConfig.functionName?

darunrs · 2024-04-10T20:24:40Z

runner/src/provisioner/provisioner.test.ts

@@ -178,6 +180,18 @@ describe('Provisioner', () => {
      await expect(provisioner.provisionUserApi(accountId, functionName, databaseSchema)).rejects.toThrow('Failed to provision endpoint: Failed to add datasource: some error');
    });

+    it('throws an error when it fails to run sql to create indexer sql', async () => {


nit: Use either run indexer sql or create indexer tables. Not a mix of both. Same for the below unit test. :)

Im a little confused - they are 2 separate test with 2 different error messages.
As oppose to calling provisioner.provisionUserApi im calling the methods directly and they both use executeSqlOnSchema to mock a reject error with different error messages.

provisioner.runIndexerSql
provisioner.runLogsSql

Kevin101Zhang · 2024-04-10T20:46:59Z

I ran locally in the following PR #657. In the following PR the base is this current one.

darunrs

Nice, if it's all locally tested, then LGTM! Make sure to observe dev to confirm the provisioning works in dev.

Check GCP logs for errors, check Hasura to see if indexers are running, and spot check particular indexers like access_keys_v1 to ensure that's running.

runner/src/indexer/indexer.ts

Uncommented functionality so we actually start writing logs to new the Tables that have been provisioned in #643. Old logging implementation remains untouched as still functions (although it has been renamed from writeLog -> writeLogOld). We are writing to both log tables. ### 1. Provisioning and Logging (to both tables) for a new Indexer https://www.loom.com/share/3ad6d6ea3368412e8896340a74759ffb?sid=4d5379e8-5401-41bf-9e38-d0f8e8c4eca5 ### 2. Logging (to both tables) for a existing Indexer https://www.loom.com/share/4ba411f2bcb740e1842650f695ffb347?sid=253ced68-9d4c-459f-871b-b0a3ee00cd91 ### Provisioning and Logging new logs table for a existing Indexer (that does not have logs table) https://www.loom.com/share/2aa7c0cc882f4dbdb9e51fc2a9e9b7b9?sid=1aa511fe-3054-4d27-9996-2b9fddc44ed8

Kevin101Zhang and others added 5 commits April 4, 2024 18:09

fix: relocated runLogsSql into setupPartitionLogsTable

4ada070

chore: add prefix for INDEXs in schema

8b57a79

feat: added log-entry class and test

a709dae

Update log-entry.test.ts

f4c7ccb

removed useless test

Update log-entry.test.ts

9de783f

Kevin101Zhang linked an issue Apr 5, 2024 that may be closed by this pull request

Introduce provisioning of logs for new indexers #641

Closed

3 tasks

Kevin101Zhang and others added 11 commits April 8, 2024 12:19

uncommented out provisioning

89fa865

feat: added additional static classes in LogEntry

8d0da62

fix: modified indexer-logger and test to use logEntry

91a3d1b

fix: typo

0be44dd

Update log-entry.test.ts

8a331b2

chore: removed console.log

1809f7a

fix: LogLevel and LogType are defined in log-entry

7254a50

fix: indexer-logger now matches exact query

749e9f2

Merge branch '639-fix-unresolved-comments-in-original-pr-httpsgithubc…

00a6e61

…omnearqueryapipull608' of https://github.com/near/queryapi into 641-introduce-provisioning-of-logs

fix: added test without bh

a582921

Merge branch '639-fix-unresolved-comments-in-original-pr-httpsgithubc…

8459863

…omnearqueryapipull608' of https://github.com/near/queryapi into 641-introduce-provisioning-of-logs

Kevin101Zhang changed the base branch from main to 639-fix-unresolved-comments-in-original-pr-httpsgithubcomnearqueryapipull608 April 8, 2024 22:37

Kevin101Zhang and others added 7 commits April 8, 2024 18:42

merge scripts

0301061

fix: used fake timers on indexer-logger test

97e6ebc

fix: writeLog exclusively accepts an array of LogEntry

4e431e9

Merge branch '639-fix-unresolved-comments-in-original-pr-httpsgithubc…

6193ade

…omnearqueryapipull608' into 641-introduce-provisioning-of-logs

chore: moved log-entry files to indexer-logger

954528e

rebase

70c9153

rebase

16e9b69

Base automatically changed from 639-fix-unresolved-comments-in-original-pr-httpsgithubcomnearqueryapipull608 to main April 9, 2024 22:45

Kevin101Zhang added 2 commits April 10, 2024 14:08

feat: provisioning of new users

030fa4c

merge with main includes - feat: Provision logs for existing users

f7de470

Kevin101Zhang marked this pull request as ready for review April 10, 2024 18:13

Kevin101Zhang requested a review from a team as a code owner April 10, 2024 18:13

Kevin101Zhang commented Apr 10, 2024

View reviewed changes

Kevin101Zhang added 2 commits April 10, 2024 14:19

chore: remove console.log

03ded3f

added back commented out test for metaDataTableDDL

8d48fc5

Kevin101Zhang marked this pull request as draft April 10, 2024 18:31

Kevin101Zhang added 3 commits April 10, 2024 14:32

uncommented skip

80eb9f1

Merge branch '641-introduce-provisioning-of-logs' of https://github.c…

451b5bc

…om/near/queryapi into 641-introduce-provisioning-of-logs

uncommented skip, removed redundant test cases

e097cbc

Kevin101Zhang added 2 commits April 10, 2024 14:39

chore: removed comment

c9fc515

chore: removed comment

c862042

Kevin101Zhang marked this pull request as ready for review April 10, 2024 18:41

Kevin101Zhang changed the title ~~Introduce provisioning of Logs Table~~ Introduce provisioning of Logs Table for new and existing users Apr 10, 2024

Kevin101Zhang marked this pull request as draft April 10, 2024 18:52

mocked provisionLogsIfNeeded for test

ef4d29c

Kevin101Zhang marked this pull request as ready for review April 10, 2024 19:00

Kevin101Zhang commented Apr 10, 2024

View reviewed changes

darunrs reviewed Apr 10, 2024

View reviewed changes

darunrs approved these changes Apr 10, 2024

View reviewed changes

Kevin101Zhang commented Apr 10, 2024

View reviewed changes

runner/src/indexer/indexer.ts Outdated Show resolved Hide resolved

chore: commented out unused code

e8de5b8

Kevin101Zhang merged commit 70f4fd8 into main Apr 10, 2024
3 checks passed

Kevin101Zhang deleted the 641-introduce-provisioning-of-logs branch April 10, 2024 21:05

This was referenced Apr 10, 2024

feat: Enable Logging functionality to both new and old Log Tables #657

Merged

use indexerConfig to get the function name for writeLogs #661

Closed

This was referenced Apr 22, 2024

Prod Release 23/04/24 #686

Merged

test stable branch git fix up #687

Closed

git fixup test 2 #688

Closed

Kevin101Zhang changed the title ~~Introduce provisioning of Logs Table for new and existing users~~ feat: Introduce provisioning of Logs Table for new and existing users Jul 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Introduce provisioning of Logs Table for new and existing users #643

feat: Introduce provisioning of Logs Table for new and existing users #643

Kevin101Zhang commented Apr 5, 2024 •

edited

Loading

Kevin101Zhang Apr 10, 2024

darunrs Apr 10, 2024

Kevin101Zhang commented Apr 10, 2024

Kevin101Zhang Apr 10, 2024

darunrs left a comment •

edited

Loading

darunrs Apr 10, 2024

darunrs Apr 10, 2024

darunrs Apr 10, 2024

Kevin101Zhang Apr 10, 2024 •

edited

Loading

darunrs Apr 10, 2024

Kevin101Zhang Apr 10, 2024

Kevin101Zhang commented Apr 10, 2024

darunrs left a comment

		@@ -463,13 +492,12 @@ export default class Indexer {

		// async writeLog (logEntry: LogEntry, logEntries: LogEntry[], functionName: string): Promise<any> {

feat: Introduce provisioning of Logs Table for new and existing users #643

feat: Introduce provisioning of Logs Table for new and existing users #643

Conversation

Kevin101Zhang commented Apr 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kevin101Zhang commented Apr 10, 2024

Choose a reason for hiding this comment

darunrs left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kevin101Zhang Apr 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kevin101Zhang commented Apr 10, 2024

darunrs left a comment

Choose a reason for hiding this comment

Kevin101Zhang commented Apr 5, 2024 •

edited

Loading

darunrs left a comment •

edited

Loading

Kevin101Zhang Apr 10, 2024 •

edited

Loading