Skip to content

feat: add warning nudges for cross-region event triggers #10408

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

Open
shettyvarun268 wants to merge 7 commits into
base: next
Choose a base branch
from
+198 −1
Open

feat: add warning nudges for cross-region event triggers #10408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
980f7bb
feat: add warning nudges for cross-region event triggers
shettyvarun268 Apr 22, 2026
2cfd8a3
Gemini review
shettyvarun268 Apr 23, 2026
1b432e2
region pairing exempts
shettyvarun268 Apr 23, 2026
736308a
Changing logic and adding flag
shettyvarun268 Apr 23, 2026
ce514fb
Addressing Review comments
shettyvarun268 Apr 23, 2026
31a9f53
Fixing the flag conditions
shettyvarun268 Apr 23, 2026
d506809
Addressing review comments
shettyvarun268 Apr 24, 2026
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
163 changes: 163 additions & 0 deletions src/deploy/functions/triggerRegionHelper.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ import * as sinon from "sinon";
import * as backend from "./backend";
import * as storage from "../../gcp/storage";
import * as triggerRegionHelper from "./triggerRegionHelper";
import * as utils from "../../utils";
import * as firestore from "../../gcp/firestore";
import * as firestoreService from "./services/firestore";

const SPEC = {
region: "us-west1",
Expand All @@ -14,9 +17,14 @@ const SPEC = {
describe("TriggerRegionHelper", () => {
describe("ensureTriggerRegions", () => {
let storageStub: sinon.SinonStub;
let firestoreStub: sinon.SinonStub;

beforeEach(() => {
storageStub = sinon.stub(storage, "getBucket").throws("unexpected call to storage.getBucket");
firestoreStub = sinon
.stub(firestore, "getDatabase")
.throws("unexpected call to firestore.getDatabase");
firestoreService.clearCache();
});
Comment thread
shettyvarun268 marked this conversation as resolved.

afterEach(() => {
Expand Down Expand Up @@ -117,5 +125,160 @@ describe("TriggerRegionHelper", () => {
"A function in region europe-west4 cannot listen to a bucket in region us",
);
});

it("should warn on transatlantic latency hops", async () => {
firestoreStub.resolves({ locationId: "europe-west1" });
const wantFn: backend.Endpoint = {
id: "wantFn",
entryPoint: "wantFn",
platform: "gcfv2",
eventTrigger: {
eventType: "google.cloud.firestore.document.v1.written",
eventFilters: { database: "(default)" },
retry: false,
region: "europe-west1",
},
...SPEC,
region: "us-central1",
};

const logLabeledWarningStub = sinon.stub(utils, "logLabeledWarning");

await triggerRegionHelper.ensureTriggerRegions(backend.of(wantFn));

expect(logLabeledWarningStub).to.have.been.calledOnceWith(
"functions",
`The following functions have triggers in different regions than they are located:\n` +
`- wantFn (us-central1, Trigger: europe-west1)\n` +
`To avoid unnecessary cross-region network hops, consider assigning these functions to their trigger regions or collocating them. ` +
`To suppress this warning, set FIREBASE_SUPPRESS_REGION_WARNING=true in your environment variables.`,
);

logLabeledWarningStub.restore();
});

it("should warn on multiple transatlantic latency hops with a rolled-up log message", async () => {
firestoreStub.onFirstCall().resolves({ locationId: "europe-west1" });
firestoreStub.onSecondCall().resolves({ locationId: "asia-northeast1" });
const wantFn1: backend.Endpoint = {
id: "wantFn1",
entryPoint: "wantFn1",
platform: "gcfv2",
eventTrigger: {
eventType: "google.cloud.firestore.document.v1.written",
eventFilters: { database: "(default)" },
retry: false,
region: "europe-west1",
},
...SPEC,
region: "us-central1",
};

const wantFn2: backend.Endpoint = {
id: "wantFn2",
entryPoint: "wantFn2",
platform: "gcfv2",
eventTrigger: {
eventType: "google.cloud.firestore.document.v1.written",
eventFilters: { database: "my-secondary-db" },
retry: false,
region: "asia-northeast1",
},
...SPEC,
region: "us-central1",
};

const logLabeledWarningStub = sinon.stub(utils, "logLabeledWarning");

await triggerRegionHelper.ensureTriggerRegions(backend.of(wantFn1, wantFn2));

expect(logLabeledWarningStub).to.have.been.calledOnceWith(
"functions",
`The following functions have triggers in different regions than they are located:\n` +
`- wantFn1 (us-central1, Trigger: europe-west1)\n` +
`- wantFn2 (us-central1, Trigger: asia-northeast1)\n` +
`To avoid unnecessary cross-region network hops, consider assigning these functions to their trigger regions or collocating them. ` +
`To suppress this warning, set FIREBASE_SUPPRESS_REGION_WARNING=true in your environment variables.`,
);

logLabeledWarningStub.restore();
});

it("should be able to suppress warnings with an environment flag", async () => {
firestoreStub.resolves({ locationId: "europe-west1" });
const wantFn: backend.Endpoint = {
id: "wantFn",
entryPoint: "wantFn",
platform: "gcfv2",
eventTrigger: {
eventType: "google.cloud.firestore.document.v1.written",
eventFilters: { database: "(default)" },
retry: false,
region: "europe-west1",
},
...SPEC,
region: "us-central1",
};

process.env.FIREBASE_SUPPRESS_REGION_WARNING = "true";
const logLabeledWarningStub = sinon.stub(utils, "logLabeledWarning");

await triggerRegionHelper.ensureTriggerRegions(backend.of(wantFn));

expect(logLabeledWarningStub).to.not.have.been.called;

logLabeledWarningStub.restore();
delete process.env.FIREBASE_SUPPRESS_REGION_WARNING;
});

it("should not warn when regions match", async () => {
Comment thread
shettyvarun268 marked this conversation as resolved.
firestoreStub.resolves({ locationId: "us-central1" });
const wantFn: backend.Endpoint = {
id: "wantFn",
entryPoint: "wantFn",
platform: "gcfv2",
eventTrigger: {
eventType: "google.cloud.firestore.document.v1.written",
eventFilters: { database: "(default)" },
retry: false,
region: "us-central1",
},
...SPEC,
region: "us-central1",
};

const logLabeledWarningStub = sinon.stub(utils, "logLabeledWarning");

await triggerRegionHelper.ensureTriggerRegions(backend.of(wantFn));

expect(logLabeledWarningStub).to.not.have.been.called;

logLabeledWarningStub.restore();
});

it("should not warn when US function uses nam5 multi-region trigger", async () => {
firestoreStub.resolves({ locationId: "nam5" });
const wantFn: backend.Endpoint = {
id: "wantFn",
entryPoint: "wantFn",
platform: "gcfv2",
eventTrigger: {
eventType: "google.cloud.firestore.document.v1.written",
eventFilters: { database: "(default)" },
retry: false,
region: "nam5",
},
...SPEC,
region: "us-central1",
};

const logLabeledWarningStub = sinon.stub(utils, "logLabeledWarning");

await triggerRegionHelper.ensureTriggerRegions(backend.of(wantFn));

expect(logLabeledWarningStub).to.not.have.been.called;

logLabeledWarningStub.restore();
});
});
});
36 changes: 35 additions & 1 deletion src/deploy/functions/triggerRegionHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import * as backend from "./backend";
import { serviceForEndpoint } from "./services";
import * as utils from "../../utils";

/**
* Ensures the trigger regions are set and correct
* @param want the list of function specs we want to deploy
* @param have the list of function specs we have deployed
*/
export async function ensureTriggerRegions(want: backend.Backend): Promise<void> {
const regionLookups: Array<Promise<void>> = [];
Expand All @@ -16,4 +16,38 @@ export async function ensureTriggerRegions(want: backend.Backend): Promise<void>
regionLookups.push(serviceForEndpoint(ep).ensureTriggerRegion(ep));
}
await Promise.all(regionLookups);

// Warn if an event function defaults to or is assigned to us-central1 but its trigger is out of the US,
// to avoid unnecessary cross-region network hops (e.g., transatlantic).
if (process.env.FIREBASE_SUPPRESS_REGION_WARNING === "true") {
return;
}

const offendingFunctions: string[] = [];
for (const ep of backend.allEndpoints(want)) {
if (ep.platform === "gcfv1" || !backend.isEventTriggered(ep)) {
Copy link
Copy Markdown
Contributor

@ajperel ajperel Apr 23, 2026

Choose a reason for hiding this comment

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

Why are we ignoring v1 functions? Is there a reason those should stay with international hops?

Copy link
Copy Markdown
Contributor Author

@shettyvarun268 shettyvarun268 Apr 24, 2026

Choose a reason for hiding this comment

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

I am still debating this. One one hand I agree that the v1 users might get the biggest benefit out of this since they might have deployed in different regions. But at the same time v1 functions would require some additional lookup which would make the deploy process slower for them(especially with multiple functions).

Copy link
Copy Markdown
Contributor

@ajperel ajperel Apr 24, 2026

Choose a reason for hiding this comment

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

Can you quantify this at all? How complicated and slow is the additional lookup? If we're increasing the deployment time from like 1 minute to 1:02 ... I'm fine with that slowdown. If we were going from 0:30 to 1:30 that would be a problem.

Copy link
Copy Markdown
Contributor Author

@shettyvarun268 shettyvarun268 Apr 24, 2026

Choose a reason for hiding this comment

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

For small projects with a few functions, we could probably parallelize the lookups keeping the delay under 1 to 2 seconds (so within your limit!).

The problem really comes down to scale and permissions. If a user has 20+ V1 functions pointing at several distinct databases or buckets, making individual API hits to solve those missing region fields on V1 manifests could tack on 10+ seconds of dead weight to the deploy time. Also, doing high-speed sequential hits runs a risk of tagging rate limits or firing IAM check fails for developers on locked down corporate accounts.

continue;
}
const triggerRegion = ep.eventTrigger.region;
if (ep.region !== "us-central1" || !triggerRegion || triggerRegion === "global") {
continue;
}
if (!isUSRegion(triggerRegion)) {
offendingFunctions.push(`- ${ep.id} (us-central1, Trigger: ${triggerRegion})`);
}
}
Comment thread
shettyvarun268 marked this conversation as resolved.

if (offendingFunctions.length > 0) {
utils.logLabeledWarning(
"functions",
`The following functions have triggers in different regions than they are located:\n` +
offendingFunctions.join("\n") +
`\nTo avoid unnecessary cross-region network hops, consider assigning these functions to their trigger regions or collocating them. ` +
`To suppress this warning, set FIREBASE_SUPPRESS_REGION_WARNING=true in your environment variables.`,
);
}
}

function isUSRegion(region: string): boolean {
return region === "us" || region === "nam5" || region === "nam7" || region.startsWith("us-");
}
Loading
⚡