feat(telemetry-processor): Client Reports#15983
Conversation
Add a concept of how the telemetry processor handles client reports.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-Authored-By: Claude <noreply@anthropic.com>
Add a concept of how the telemetry processor should interact with rate limits. Slighly related to #15983.
| # Rate Limiting | ||
| The telemetry scheduler **SHOULD** add [client reports](/sdk/telemetry/client-reports/) to outgoing envelopes or periodically send standalone client report envelopes to the transport, following the [SDK side recommendations](/sdk/telemetry/client-reports/#sdk-side-recommendations). | ||
|
|
||
| Client report tracking **SHOULD NOT** be implemented in the telemetry scheduler. Instead, the SDK **SHOULD** have `ClientReports` accessible throughout the SDK, since data can be dropped in multiple places, as shown in the diagram below. The telemetry scheduler **SHOULD** request the current client reports from `ClientReports`. |
There was a problem hiding this comment.
This paragraph is quite vague and it lets many things up to interpretation. I want to make sure I understand this part, because this - to me - reads as, "the scheduler should implement extra logic for periodically sending client reports".
I think we need to make it a bit more clear how the ClientReports would interact with the scheduler:
- would
ClientReportshandle aggregation? - would an added report be on a
Bufferand the scheduler eventually flushes it - would the scheduler have custom logic to handle a report separately?
I think the problem originates from the previous Client Reports page being too vague, but it would be nice if we can clear this up a bit.
There was a problem hiding this comment.
I agree. It seems like this is suggesting the creation of another object in the mix here, a ClientReports system that the telemetry scheduler fetches reports from "periodically".
My approach in the JS SDK would be to keep the Client.recordDroppedEvent, as suggested, so that anyone anywhere can throw reports at it, but have the client be a thin pass-through exposing that functionality from the scheduler. So, the Telemetry Scheduler manages and defines how ClientReports are handled, but the Client exposes it to everywhere else (integrations, etc) to access.
There was a problem hiding this comment.
I think the problem originates from the previous Client Reports page being too vague, but it would be nice if we can clear this up a bit.
Yes, indeed. I also struggled a bit with the client reports spec being vague when I initially implemented them on the Cocoa SDK.
would ClientReports handle aggregation?
I'm not 100% sure what you mean by aggregation, but I guess aggregating individual records of dropped events. So yes, client reports should do that. On Cocoa, we keep a dictionary with the key reason-category and then increment the count, see code. The spec doesn't clearly specify this, but it could. Do you think such a dict would work for the backend @giortzisg and also JS @isaacs?
would an added report be on a Buffer and the scheduler eventually flushes it
I don't think it should be on a buffer. Instead, the telemetry scheduler can either periodically flush it or add it to outgoing envelopes.
would the scheduler have custom logic to handle a report separately?
Currently, yes, it would need to pull the client reports and prep an envelope.
My approach in the JS SDK would be to keep the Client.recordDroppedEvent, as suggested, so that anyone anywhere can throw reports at it, but have the client be a thin pass-through exposing that functionality from the scheduler. So, the Telemetry Scheduler manages and defines how ClientReports are handled, but the Client exposes it to everywhere else (integrations, etc) to access.
We can't only keep the client report / dropped events in the client. The telemetry scheduler also drops events due to queue overflow or rate limits and also the transport also needs to talk to it. So we could hold the object in the client, but we also need to pass it to the telemetry scheduler, telemtry buffer, and the transport. That's why I thought we should create an extra class / object that you can call from multiple places.
isaacs
left a comment
There was a problem hiding this comment.
This is not very far off from what the JS SDK does with client reports, but I agree with @giortzisg that it'd be nice (either in this PR or a subsequent edit) to clarify what exactly is being suggested.
|
Let's merge this for now, and we can expand later for a more detailed spec. |
DESCRIBE YOUR PR
Add a concept of how the telemetry processor handles client reports.
Slighly related to #16027.
IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.
SLA
Thanks in advance for your help!
PRE-MERGE CHECKLIST
Make sure you've checked the following before merging your changes: