Integrating TelemetryStore with rest of sdk - #10308
Conversation
|
There was a problem hiding this comment.
Code Review
This pull request introduces a centralized TelemetryStore to manage telemetry buffering for both logs and spans, replacing the separate internal buffers in OnErrorLogRecordProcessor and OnErrorSpanProcessor. In packages/crashlytics/src/api.ts, telemetryStore.clear() is called directly after destructuring from crashlytics. Since recordError is a public API, telemetryStore might not be defined on the passed object, which could lead to a runtime TypeError. It is recommended to use optional chaining (telemetryStore?.clear()) to safely clear the store.
| attributesStore: new AttributesStore({ projectId: 'fake-project' }) | ||
| attributesStore: new AttributesStore({ projectId: 'fake-project' }), | ||
| telemetryStore: { | ||
| clear: stub() |
There was a problem hiding this comment.
Is clear the only stub we need? Will this not call onStart and onEnd?
There was a problem hiding this comment.
clear() is the only function being called as a result of recordError being triggered by the tests. The other calls are dependent on onErrorSpanProcessor and onErrorLogRecordProcessor, which are ignored in these tests.
integrates TelemetryStore with rest of crashlytics sdk and modifies on error log/span processor tests