Skip to content

adding cloud trace storage provisioning to crashlytics onboarding - #11219

Open
ssakhamu wants to merge 3 commits into
mainfrom
welcome-span-to-cloud
Open

ssakhamu wants to merge 3 commits into
mainfrom
welcome-span-to-cloud

Conversation

@ssakhamu

@ssakhamu ssakhamu commented Oct 1, 2026

Copy link
Copy Markdown
Contributor
  • creates file for sending requests to cloudtrace and provisionTraceStorage call w/tests
  • adds call to crashlytics onboarding w/ updated tests

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request adds Cloud Trace integration to the Crashlytics onboarding process by enabling the Cloud Trace API and provisioning trace storage via a dummy span write. Feedback suggests refactoring the static traceClient instantiation into a dynamic helper function to correctly respect environment variable overrides, and utilizing the getError utility to safely wrap caught errors when throwing a FirebaseError.

Comment thread src/gcp/cloudtrace.ts Outdated
Comment thread src/gcp/cloudtrace.ts Outdated
Comment thread src/gcp/cloudtrace.ts Outdated
Comment thread src/gcp/cloudtrace.ts
@ssakhamu
ssakhamu force-pushed the welcome-span-to-cloud branch from ff31496 to d9376b8 Compare October 1, 2026 15:09
Comment thread src/crashlytics/onboarding.ts Outdated
Comment thread src/gcp/cloudtrace.ts Outdated
Comment thread src/gcp/cloudtrace.ts Outdated
@ssakhamu
ssakhamu force-pushed the welcome-span-to-cloud branch from f428142 to ea12d67 Compare October 2, 2026 18:06
"crashlytics",
false,
);
expect(ensureStub).to.not.have.been.calledWith(

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.

should this test just be to.not.have.been.called ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

esnureStub is called twice with different parameters above so this is to make sure it doesn't get called with these parameters. Otherwise, it would fail as it is called twice.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants