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

Make debug logging of typesense imports expicit and configurable #101

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

Make debug logging of typesense imports expicit and configurable #101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
55fcb8c
Add explicit config support for debug logging of typesense imports. T…
jsasitorn Dec 30, 2024
919cd53
Add test cases for logging tests using TestEnvironment class
jsasitorn Dec 30, 2024
181324c
Migrate Subcollection tests to use TestEnvironment and merge test-par…
jsasitorn Dec 30, 2024
e73c416
Merge branch 'typesense:master' into debug_logging
jsasitorn Jan 9, 2025
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
2 changes: 1 addition & 1 deletion .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module.exports = {
jest: true,
},
parserOptions: {
ecmaVersion: 2020,
ecmaVersion: 2022,
},
extends: ["eslint:recommended", "google"],
rules: {
Expand Down
12 changes: 12 additions & 0 deletions extension.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,18 @@ params:
value: true
default: false
required: false
- param: LOG_TYPESENSE_INSERTS
label: Log Typesense Inserts for Debugging
description: >-
Should data inserted into Typesense be logged in Cloud Logging? This can be useful for debugging, but should not be enabled in production.
type: select
options:
- label: No
value: false
- label: Yes
value: true
default: false
required: false
- param: LOCATION
label: Cloud Functions location
description: >-
Expand Down
6 changes: 5 additions & 1 deletion functions/src/backfill.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,11 @@ module.exports = functions.firestore.document(config.typesenseBackfillTriggerDoc
const pathParams = utils.pathMatchesSelector(docPath, config.firestoreCollectionPath);

if (!isGroupQuery || (isGroupQuery && pathParams !== null)) {
return await utils.typesenseDocumentFromSnapshot(doc, pathParams);
const typesenseDocument = await utils.typesenseDocumentFromSnapshot(doc, pathParams);
if (config.shouldLogTypesenseInserts) {
functions.logger.debug(`Backfilling document ${JSON.stringify(typesenseDocument)}`);
}
return typesenseDocument;
} else {
return null;
}
Expand Down
1 change: 1 addition & 0 deletions functions/src/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ module.exports = {
.map((f) => f.trim())
.filter((f) => f),
shouldFlattenNestedDocuments: process.env.FLATTEN_NESTED_DOCUMENTS === "true",
shouldLogTypesenseInserts: process.env.LOG_TYPESENSE_INSERTS === "true",
typesenseHosts:
(process.env.TYPESENSE_HOSTS || "").split(",").map((e) => e.trim()),
typesensePort: process.env.TYPESENSE_PORT || 443,
Expand Down
6 changes: 5 additions & 1 deletion functions/src/indexOnWrite.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ module.exports = functions.firestore.document(config.firestoreCollectionPath)
const latestSnapshot = await snapshot.after.ref.get();
const typesenseDocument = await utils.typesenseDocumentFromSnapshot(latestSnapshot, context.params);

functions.logger.debug(`Upserting document ${JSON.stringify(typesenseDocument)}`);
if (config.shouldLogTypesenseInserts) {
functions.logger.debug(`Upserting document ${JSON.stringify(typesenseDocument)}`);
} else {
functions.logger.debug(`Upserting document ${typesenseDocument.id}`);
}
return await typesense
.collections(encodeURIComponent(config.typesenseCollectionName))
.documents()
Expand Down
2 changes: 0 additions & 2 deletions functions/src/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,6 @@ exports.typesenseDocumentFromSnapshot = async (firestoreDocumentSnapshot, contex
// using flat to flatten nested objects for older versions of Typesense that did not support nested fields
// https://typesense.org/docs/0.22.2/api/collections.html#indexing-nested-fields
const typesenseDocument = config.shouldFlattenNestedDocuments ? flattenDocument(mappedDocument) : mappedDocument;
console.log("typesenseDocument", typesenseDocument);

typesenseDocument.id = firestoreDocumentSnapshot.id;

if (contextParams && Object.entries(contextParams).length) {
Expand Down
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is starting to be difficult to manage and read. We should rename the scripts to be able to identify them from the name itself. Maybe add some pretest scripts while we're at it, to break them into chunks as well. Something like this:

{
  "scripts": {
    "test": "npm run test:core && npm run test:features && npm run test:logging",
    
    "test:core": "npm run test:core:flattened && npm run test:core:unflattened",
    "test:core:flattened": "cross-env DOTENV_CONFIG=extensions/test-params-flatten-nested-true.local.env firebase emulators:exec --only functions,firestore,extensions 'jest --testRegex=\"backfill.spec\"'",
    "test:core:unflattened": "cross-env DOTENV_CONFIG=extensions/test-params-flatten-nested-false.local.env firebase emulators:exec --only functions,firestore,extensions 'jest --testRegex=\"WithoutFlattening\"'",
    // etc
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea, this a great follow up! Cleaning up overall test case organization is a bit out of scope for this PR though, which is focused on improving debug logging, and being able to turn off logging of data in production environments.

Also to note, that if you end up migrating more tests to the test environment, you could use jest style selectors, eg:
"test:core:unflattened": "jest --testRegex="WithoutFlattening""

Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
"emulator": "cross-env DOTENV_CONFIG=extensions/test-params-flatten-nested-false.local.env firebase emulators:start --import=emulator_data",
"export": "firebase emulators:export emulator_data",
"test": "npm run test-part-1 && npm run test-part-2 && npm run test-part-3",
"test-part-1": "cp -f extensions/test-params-flatten-nested-true.local.env extensions/firestore-typesense-search.env.local && cross-env NODE_OPTIONS=--experimental-vm-modules DOTENV_CONFIG=extensions/test-params-flatten-nested-true.local.env firebase emulators:exec --only functions,firestore,extensions 'jest --testPathIgnorePatterns=\"WithoutFlattening\" --testPathIgnorePatterns=\"Subcollection\"'",
"test-part-1": "cp -f extensions/test-params-flatten-nested-true.local.env extensions/firestore-typesense-search.env.local && cross-env NODE_OPTIONS=--experimental-vm-modules DOTENV_CONFIG=extensions/test-params-flatten-nested-true.local.env firebase emulators:exec --only functions,firestore,extensions 'jest --testRegex=\"WithFlattening\" --testRegex=\"backfill.spec\"'",
"test-part-2": "cp -f extensions/test-params-flatten-nested-false.local.env extensions/firestore-typesense-search.env.local && cross-env NODE_OPTIONS=--experimental-vm-modules DOTENV_CONFIG=extensions/test-params-flatten-nested-false.local.env firebase emulators:exec --only functions,firestore,extensions 'jest --testRegex=\"WithoutFlattening\"'",
"test-part-3": "cp -f extensions/test-params-subcategory-flatten-nested-false.local.env extensions/firestore-typesense-search.env.local && cross-env NODE_OPTIONS=--experimental-vm-modules DOTENV_CONFIG=extensions/test-params-subcategory-flatten-nested-false.local.env firebase emulators:exec --only functions,firestore,extensions 'jest --testRegex=\"Subcollection\"'",
"test-part-3": "jest --testRegex=\"writeLogging\" --testRegex=\"Subcollection\" --detectOpenHandles",
"typesenseServer": "docker compose up",
"lint:fix": "eslint . --fix",
"lint": "eslint ."
Expand Down
111 changes: 58 additions & 53 deletions test/backfillSubcollection.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,59 +1,56 @@
const firebase = require("firebase-admin");
const config = require("../functions/src/config.js");
const typesense = require("../functions/src/createTypesenseClient.js")();

const app = firebase.initializeApp({
// Use a special URL to talk to the Realtime Database emulator
databaseURL: `${process.env.FIREBASE_DATABASE_EMULATOR_HOST}?ns=${process.env.GCLOUD_PROJECT}`,
projectId: process.env.GCLOUD_PROJECT,
});
const firestore = app.firestore();
const {TestEnvironment} = require("./support/testEnvironment");

// test case configs
const TEST_FIRESTORE_PARENT_COLLECTION_PATH = "users";
const TEST_FIRESTORE_CHILD_FIELD_NAME = "books";

describe("backfillSubcollection", () => {
const parentCollectionPath = process.env.TEST_FIRESTORE_PARENT_COLLECTION_PATH;
let testEnvironment;

const parentCollectionPath = TEST_FIRESTORE_PARENT_COLLECTION_PATH;
const unrelatedCollectionPath = "unrelatedCollectionToNotBackfill";
const childFieldName = process.env.TEST_FIRESTORE_CHILD_FIELD_NAME;
const childFieldName = TEST_FIRESTORE_CHILD_FIELD_NAME;
let parentIdField = null;

beforeAll(() => {
const matches = config.firestoreCollectionPath.match(/{([^}]+)}/g);
expect(matches).toBeDefined();
expect(matches.length).toBe(1);
let config = null;
let firestore = null;
let typesense = null;

beforeAll((done) => {
testEnvironment = new TestEnvironment({
dotenvPath: "extensions/test-params-subcategory-flatten-nested-false.local.env",
outputAllEmulatorLogs: true,
});
testEnvironment.setupTestEnvironment((err) => {
const matches = testEnvironment.config.firestoreCollectionPath.match(/{([^}]+)}/g);
expect(matches).toBeDefined();
expect(matches.length).toBe(1);

parentIdField = matches[0].replace(/{|}/g, "");
expect(parentIdField).toBeDefined();

config = testEnvironment.config;
firestore = testEnvironment.firestore;
typesense = testEnvironment.typesense;
done();
});
});

parentIdField = matches[0].replace(/{|}/g, "");
expect(parentIdField).toBeDefined();
afterAll(async () => {
await testEnvironment.teardownTestEnvironment();
});

beforeEach(async () => {
// Clear the database between tests
// For subcollections, need special handling to clear parent collection. Deleting here triggers firebase functions
await firestore.recursiveDelete(firestore.collection(parentCollectionPath));
await firestore.recursiveDelete(firestore.collection(unrelatedCollectionPath));

// Clear any previously created collections
try {
await typesense.collections(encodeURIComponent(config.typesenseCollectionName)).delete();
} catch (e) {
console.info(`${config.typesenseCollectionName} collection not found, proceeding...`);
}

// Create a new Typesense collection
return typesense.collections().create({
name: config.typesenseCollectionName,
fields: [
{name: ".*", type: "auto"},
],
});
});

afterAll(async () => {
// clean up the firebase app after all tests have run
await app.delete();
await testEnvironment.clearAllData();
});

describe("when firestore_collections is not specified", () => {
it("backfills existing Firestore data in all collections to Typesense" +
" when `trigger: true` is set " +
` in ${config.typesenseBackfillTriggerDocumentInFirestore}`, async () => {
" when `trigger: true` is set on trigger document", async () => {
const parentDocData = {
nested_field: {
field1: "value1",
Expand All @@ -67,7 +64,7 @@ describe("backfillSubcollection", () => {
};

// create parent document in Firestore
const parentDocRef = await firestore.collection(parentCollectionPath).add(parentDocData);
const parentDocRef = await testEnvironment.firestore.collection(parentCollectionPath).add(parentDocData);

// create a subcollection with document under the parent document
const subCollectionRef = parentDocRef.collection(childFieldName);
Expand All @@ -78,24 +75,24 @@ describe("backfillSubcollection", () => {

// The above will automatically add the document to Typesense,
// so delete it so we can test backfill
await typesense.collections(encodeURIComponent(config.typesenseCollectionName)).delete();
await typesense.collections().create({
name: config.typesenseCollectionName,
await testEnvironment.typesense.collections(encodeURIComponent(testEnvironment.config.typesenseCollectionName)).delete();
await testEnvironment.typesense.collections().create({
name: testEnvironment.config.typesenseCollectionName,
fields: [
{name: ".*", type: "auto"},
],
});

await firestore
.collection(config.typesenseBackfillTriggerDocumentInFirestore.split("/")[0])
await testEnvironment.firestore
.collection(testEnvironment.config.typesenseBackfillTriggerDocumentInFirestore.split("/")[0])
.doc("backfill")
.set({trigger: true});
// Wait for firestore cloud function to write to Typesense
await new Promise((r) => setTimeout(r, 2000));

// Check that the data was backfilled
const typesenseDocsStr = await typesense
.collections(encodeURIComponent(config.typesenseCollectionName))
const typesenseDocsStr = await testEnvironment.typesense
.collections(encodeURIComponent(testEnvironment.config.typesenseCollectionName))
.documents()
.export();
const typesenseDocs = typesenseDocsStr.split("\n").map((s) => JSON.parse(s));
Expand All @@ -113,8 +110,7 @@ describe("backfillSubcollection", () => {
describe("when firestore_collections is specified", () => {
describe("when firestore_collections includes this collection", () => {
it("backfills existing Firestore data in this particular collection to Typesense" +
" when `trigger: true` is set " +
` in ${config.typesenseBackfillTriggerDocumentInFirestore}`, async () => {
" when `trigger: true` is set on trigger document", async () => {
const parentDocData = {
nested_field: {
field1: "value1",
Expand Down Expand Up @@ -163,6 +159,7 @@ describe("backfillSubcollection", () => {
.documents()
.export();
const typesenseDocs = typesenseDocsStr.split("\n").map((s) => JSON.parse(s));
console.log(typesenseDocs);
expect(typesenseDocs.length).toBe(1);
expect(typesenseDocs[0]).toStrictEqual({
id: subDocRef.id,
Expand All @@ -175,8 +172,7 @@ describe("backfillSubcollection", () => {

describe("when firestore_collections does not include this collection", () => {
it("does not backfill existing Firestore data in this particular collection to Typesense" +
" when `trigger: true` is set " +
` in ${config.typesenseBackfillTriggerDocumentInFirestore}`, async () => {
" when `trigger: true` is set on trigger document", async () => {
const parentDocData = {
nested_field: {
field1: "value1",
Expand All @@ -194,7 +190,7 @@ describe("backfillSubcollection", () => {

// create a subcollection with document under the parent document
const subCollectionRef = parentDocRef.collection(childFieldName);
await subCollectionRef.add(subDocData);
const subDocRef = await subCollectionRef.add(subDocData);
// Wait for firestore cloud function to write to Typesense
await new Promise((r) => setTimeout(r, 2000));

Expand Down Expand Up @@ -224,6 +220,15 @@ describe("backfillSubcollection", () => {
.documents()
.export();
expect(typesenseDocsStr).toEqual("");

// Check that the error was logged
testEnvironment.resetCapturedEmulatorLogs();
subDocRef.delete();
await new Promise((r) => setTimeout(r, 5000));

expect(testEnvironment.capturedEmulatorLogs).toContain(
`Could not find a document with id: ${subDocRef.id}`,
);
});
});
});
Expand Down
74 changes: 37 additions & 37 deletions test/indexOnWriteSubcollection.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,50 +1,50 @@
const firebase = require("firebase-admin");
const config = require("../functions/src/config.js");
const typesense = require("../functions/src/createTypesenseClient.js")();

const app = firebase.initializeApp({
// Use a special URL to talk to the Realtime Database emulator
databaseURL: `${process.env.FIREBASE_DATABASE_EMULATOR_HOST}?ns=${process.env.GCLOUD_PROJECT}`,
projectId: process.env.GCLOUD_PROJECT,
});
const firestore = app.firestore();
const {TestEnvironment} = require("./support/testEnvironment");

// test case configs
const TEST_FIRESTORE_PARENT_COLLECTION_PATH = "users";
const TEST_FIRESTORE_CHILD_FIELD_NAME = "books";


describe("indexOnWriteSubcollection", () => {
const parentCollectionPath = process.env.TEST_FIRESTORE_PARENT_COLLECTION_PATH;
const childFieldName = process.env.TEST_FIRESTORE_CHILD_FIELD_NAME;
let testEnvironment;

const parentCollectionPath = TEST_FIRESTORE_PARENT_COLLECTION_PATH;
const childFieldName = TEST_FIRESTORE_CHILD_FIELD_NAME;
let parentIdField = null;

beforeAll(async () => {
const matches = config.firestoreCollectionPath.match(/{([^}]+)}/g);
expect(matches).toBeDefined();
expect(matches.length).toBe(1);
let config = null;
let firestore = null;
let typesense = null;

parentIdField = matches[0].replace(/{|}/g, "");
expect(parentIdField).toBeDefined();
beforeAll((done) => {
testEnvironment = new TestEnvironment({
dotenvPath: "extensions/test-params-subcategory-flatten-nested-false.local.env",
outputAllEmulatorLogs: true,
});
testEnvironment.setupTestEnvironment((err) => {
const matches = testEnvironment.config.firestoreCollectionPath.match(/{([^}]+)}/g);
expect(matches).toBeDefined();
expect(matches.length).toBe(1);

parentIdField = matches[0].replace(/{|}/g, "");
expect(parentIdField).toBeDefined();

config = testEnvironment.config;
firestore = testEnvironment.firestore;
typesense = testEnvironment.typesense;
done();
});
});

afterAll(async () => {
await testEnvironment.teardownTestEnvironment();
});

beforeEach(async () => {
// delete the Firestore collection
// For subcollections, need special handling to clear parent collection. Deleting here triggers firebase functions
await firestore.recursiveDelete(firestore.collection(parentCollectionPath));

// Clear any previously created collections
try {
await typesense.collections(encodeURIComponent(config.typesenseCollectionName)).delete();
} catch (e) {
console.info(`${config.typesenseCollectionName} collection not found, proceeding...`);
}

// recreate the Typesense collection
await typesense.collections().create({
name: config.typesenseCollectionName,
fields: [{name: ".*", type: "auto"}],
enable_nested_fields: true,
});
});

afterAll(async () => {
// clean up the whole firebase app
await app.delete();
await testEnvironment.clearAllData();
});

describe("Backfill from dynamic subcollections", () => {
Expand Down
Loading