Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 12 additions & 9 deletions kits/firestore-bigquery-export/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,12 @@ the instances cannot collide.
## Events

When `EVENTARC_CHANNEL` is configured, the function publishes `onStart` and
`onError` lifecycle events under
`firebase.extensions.firestore-bigquery-export.v1.*`. The extension's
`onSuccess` event is not published; see the events entry under
"Differences from the Stream Firestore to BigQuery extension" below.
`onError` lifecycle events. Each one is published twice, under
`firebase.extensions.firestore-bigquery-export.v1.*` and under the legacy
`firebase.extensions.firestore-counter.v1.*` type the extension also used, so
triggers written against either type keep firing. The extension's `onSuccess`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-blocking. This holds only when EXT_SELECTED_EVENTS is unset, which this README doesn't mention anywhere (the other kit READMEs do: firestore-counter, firestore-send-email, firestore-vector-search, speech-to-text). firebase-tools allowlisted EXT_SELECTED_EVENTS for .env and ext:export writes it, so a migrating user can land with it set to the firestore-bigquery-export types only, their legacy triggers stay dark, and nothing in the doc points at the knob.

Going the other way, unset (the kit default, since no kit manifest declares events) means both copies publish, so two Eventarc RPCs per write on a path where a publish failure rethrows in handleWrite and aborts the BigQuery export. A sentence naming EXT_SELECTED_EVENTS would cover both directions.

event is not published; see the events entry under "Differences from the Stream
Firestore to BigQuery extension" below.

## Provisioning

Expand Down Expand Up @@ -252,11 +254,12 @@ queue in the console, and the two knobs that tuned that queue,
queue handler, which is gone, so the kit publishes `onStart` and `onError`
only.

Events are published under `firebase.extensions.firestore-bigquery-export.v1.*`
only. The extension also published a duplicate copy of every event under
`firebase.extensions.firestore-counter.v1.*`, a historical naming mistake kept
for backwards compatibility. If you have Eventarc triggers listening on those
`firestore-counter` types, point them at the `firestore-bigquery-export` types.
Every event is still published twice, once under
`firebase.extensions.firestore-bigquery-export.v1.*` and once under
`firebase.extensions.firestore-counter.v1.*`. The `firestore-counter` type is a
historical naming mistake the extension kept for backwards compatibility, and
the kit keeps it for the same reason: triggers listening on it survive the
migration. Write new triggers against the `firestore-bigquery-export` types.

### Wildcard columns include the document ID

Expand Down
10 changes: 7 additions & 3 deletions kits/firestore-bigquery-export/src/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,17 @@ import * as eventArc from "firebase-admin/eventarc";
const { getEventarc } = eventArc;

/**
* Builds the Eventarc event type for this extension.
* Generates both the OLD and NEW event types to maintain backward compatibility.
*
* Old Event Type: firebase.extensions.firestore-counter.v1.{eventName}
* New Event Type: firebase.extensions.firestore-bigquery-export.v1.{eventName}
*
* @param eventName The name of the event (e.g., "onStart", "onError", etc.)
* @returns The event type string.
* @returns An array containing both the old and new event types
*/
const getEventTypes = (eventName: string) => [
`firebase.extensions.firestore-bigquery-export.v1.${eventName}`,
`firebase.extensions.firestore-counter.v1.${eventName}`, // OLD Event Type for backward compatibility

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-blocking, and I don't think the code should change here since this is exactly the parity the PR is restoring.

Worth noting though: this type is byte-identical to the type the firestore-counter kit publishes today (kits/firestore-counter/src/events.ts builds firebase.extensions.firestore-counter.v1.<name>). In a project running both kits against the same EVENTARC_CHANNEL, the counter kit's onStart trigger will now fire for every document write this kit exports, with a payload of {documentId, changeType, before, after, context} rather than the counter's {data, params}, so a handler reading event.data.params gets undefined. Eventarc triggers filter on type, so it can't be filtered downstream; the only escape is setting EXT_SELECTED_EVENTS on this kit to the new types only.

In the extension this was accepted history, but in the kits world both are first-class packages someone may deploy side by side, so it might be worth a line in the differences entry with that mitigation named.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hmm, this is something worth considering.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it's a valid point but like you say, for parity this is a follow-up

`firebase.extensions.firestore-bigquery-export.v1.${eventName}`, // NEW Event Type following the updated convention
];
Comment thread
CorieW marked this conversation as resolved.

let eventChannel: eventArc.Channel | undefined;
Expand Down
28 changes: 21 additions & 7 deletions kits/firestore-bigquery-export/tests/events.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,13 @@ describe("channel configured", () => {
setupEventChannel();
});

test("publishes the firestore-bigquery-export event type only", async () => {
test("publishes both the legacy and the current event type", async () => {
await recordStartEvent({ a: 1 });
expect(publish).toHaveBeenCalledTimes(1);
expect(publish).toHaveBeenCalledWith(
expect.objectContaining({
type: "firebase.extensions.firestore-bigquery-export.v1.onStart",
})
);
expect(publish).toHaveBeenCalledTimes(2);
expect(publish.mock.calls.map((c) => c[0].type)).toEqual([
"firebase.extensions.firestore-counter.v1.onStart",
"firebase.extensions.firestore-bigquery-export.v1.onStart",
]);
});

test("error / success / completion map to their event types", async () => {
Expand All @@ -76,9 +75,24 @@ describe("channel configured", () => {

const types = publish.mock.calls.map((c) => c[0].type);
expect(types).toEqual([
"firebase.extensions.firestore-counter.v1.onError",
"firebase.extensions.firestore-bigquery-export.v1.onError",
"firebase.extensions.firestore-counter.v1.onSuccess",
"firebase.extensions.firestore-bigquery-export.v1.onSuccess",
"firebase.extensions.firestore-counter.v1.onCompletion",
"firebase.extensions.firestore-bigquery-export.v1.onCompletion",
]);
});

test("the legacy copy carries the same payload as the current one", async () => {
await recordErrorEvent(new Error("boom"), "doc1");

const [legacy, current] = publish.mock.calls.map((c) => c[0]);
expect(legacy.data).toEqual({ message: "boom" });
expect(legacy.subject).toBe("doc1");
expect({ ...legacy, type: undefined }).toEqual({
...current,
type: undefined,
});
});
});
Loading