Repository navigation
fix(crashlytics): use v1alpha profilingManagerConfig endpoint for heap dump commands - #11457
Conversation
…p dump commands
Update `getProfilingManagerConfig` and `updateProfilingManagerConfig` to use the
shared `CRASHLYTICS_API_CLIENT` (`v1alpha`) and the singleton resource endpoint
`/v1alpha/projects/{projectNumber}/apps/{appId}/profilingManagerConfig`:
- Switch `getProfilingManagerConfig` to `GET /v1alpha/projects/{projectNumber}/apps/{appId}/profilingManagerConfig` and remove `getCrashlyticsV1Client()`.
- Switch `updateProfilingManagerConfig` to `PATCH /v1alpha/projects/{projectNumber}/apps/{appId}/profilingManagerConfig` with `updateMask`.
- Translate `gcsBucket` between bare bucket IDs used by the CLI and canonical `projects/_/buckets/{bucket}` resource names expected by the API.
- Simplify `disableHeapDumpCollection` to send a partial `PATCH` (`updateMask=heapDumpCollectionEnabled`) instead of a `GET` + `POST` pair so the existing `gcsBucket` is preserved atomically.
- Update `profilingManager.spec.ts` unit tests to cover the `v1alpha` resource paths, bucket resource-name formatting, and partial `updateMask` updates.
There was a problem hiding this comment.
Code Review
This pull request refactors the Crashlytics profiling manager to use the updated profilingManagerConfig API endpoint. Key changes include switching from POST to PATCH requests for updating configurations, implementing partial updates using updateMask query parameters, and introducing helper functions to convert between GCS bucket names and their resource path representations. Additionally, disableHeapDumpCollection has been simplified to perform a partial update without needing to fetch the current configuration first. Unit tests have been updated accordingly to reflect these API and endpoint changes. There are no review comments, and I have no additional feedback to provide.
kaibolay
left a comment
There was a problem hiding this comment.
As far as I can tell, the backend changes to production are currently rolling out. But even now you should be able to test this with preprod or staging.
- Rename gcsBucket to bucket on ProfilingManagerConfig and in the updateMask field paths to match the v1alpha API schema (AIP-122). - Return response.body directly from getProfilingManagerConfig. - Simplify updateProfilingManagerConfig to accept an explicit updateMask parameter and return Promise<void>. - Update crashlytics:heapdumps:status and unit tests to use bucket.
kaibolay
left a comment
There was a problem hiding this comment.
Just make sure it works before merging
Update
getProfilingManagerConfigandupdateProfilingManagerConfigto use the sharedCRASHLYTICS_API_CLIENT(v1alpha) and the singleton resource endpoint/v1alpha/projects/{projectNumber}/apps/{appId}/profilingManagerConfig:getProfilingManagerConfigtoGET /v1alpha/projects/{projectNumber}/apps/{appId}/profilingManagerConfigand removegetCrashlyticsV1Client().updateProfilingManagerConfigtoPATCH /v1alpha/projects/{projectNumber}/apps/{appId}/profilingManagerConfigwithupdateMask.gcsBucketbetween bare bucket IDs used by the CLI and canonicalprojects/_/buckets/{bucket}resource names expected by the API.disableHeapDumpCollectionto send a partialPATCH(updateMask=heapDumpCollectionEnabled) instead of aGET+POSTpair so the existinggcsBucketis preserved atomically.profilingManager.spec.tsunit tests to cover thev1alpharesource paths, bucket resource-name formatting, and partialupdateMaskupdates.