Add output for tail being in sampling mode #3146
Conversation
🦋 Changeset detectedLatest commit: f8b03d0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/4888006090/npm-package-wrangler-3146You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/3146/npm-package-wrangler-3146Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/4888006090/npm-package-wrangler-3146 dev path/to/script.jsAdditional artifacts:npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/4888006090/npm-package-cloudflare-pages-shared-3146Note that these links will no longer work once the GitHub Actions artifact expires. |
jspspike
force-pushed
the
jspspike/tail-overload
branch
3 times, most recently
from
bf7aac9
to
962617a
Compare
jspspike
force-pushed
the
jspspike/tail-overload
branch
from
962617a
to
f8b03d0
Compare
Codecov Report
@@ Coverage Diff @@
## main #3146 +/- ##
==========================================
+ Coverage 74.83% 74.87% +0.04%
==========================================
Files 179 179
Lines 10813 10819 +6
Branches 2853 2856 +3
==========================================
+ Hits 8092 8101 +9
+ Misses 2721 2718 -3
|
|
Update on what I provided in the description above, I'm going to create a second PR with the pinned sampling mode warning and have this PR simply output the warning in the tail messages stream |
| function isTailInfo(event: TailEventMessage["event"]): event is TailInfo { | ||
| return Boolean(event && "message" in event && "type" in event); | ||
| } |
There was a problem hiding this comment.
Predicate Type Check, nice.
| @@ -48,6 +50,10 @@ export function prettyPrintLogs(data: WebSocket.RawData): void { | |||
| ).toLocaleString(); | |||
|
|
|||
| logger.log(`Alarm @ ${datetime} - ${outcome}`); | |||
| } else if (isTailInfo(eventMessage.event)) { | |||
There was a problem hiding this comment.
A lot of else if's starting to getting added, we may want to start thinking about the code design here.
There was a problem hiding this comment.
Maybe a switch/reducer kind of pattern, or something to define the event state.
If a tail enters sampling mode, it will forward a message saying that it's in sampling mode. The tail will send message every time the messages reaches over 100 requests per second. If we feel like that's not enough we could change this to happen periodically every 5 seconds.
One thing to keep in mind is that this will add messages to the tail stream that aren't actual trace events. Not sure if that is a concern, especially since the the non
--prettyformat is structured with event types which has different fields per message.Also for the--prettyformat we consider using something like ink to pin the sampling mode message? Not sure if that would be too much of a breaking change for users who might pipe the output of--prettyformat to other toolsReviewer has performed the following, where applicable: