-
Notifications
You must be signed in to change notification settings - Fork 129
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: payload capture - move timing into copied plugin #902
Conversation
Size Change: +697 B (0%) Total Size: 740 kB
ℹ️ View Unchanged
|
@@ -16,12 +16,8 @@ export const _isUint8Array = function (x: unknown): x is Uint8Array { | |||
// fails on only one very rare and deliberate custom object: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fun fact: the URL in this comment might have said something about JS in the past but it doesn't anymore 🤷
// readonly name: string; | ||
// readonly startTime: DOMHighResTimeStamp; | ||
// properties below here are ALPHA, don't rely on them, they may change without notice | ||
export type CapturedNetworkRequest = Omit<PerformanceEntry, 'toJSON'> & { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not important because it covered all important stuff, but in case you are looking for ideas of what to track: https://opentelemetry.io/docs/specs/semconv/http/http-spans/#common-attributes
Eg HTTP version, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tentatively this all looks good!
src/types.ts
Outdated
/** Modify the network request before it is captured. Returning null stops it being captured */ | ||
// TODO this has to work for both capture mechanisms? 😱 | ||
// deprecated - use maskCapturedNetworkRequestFn instead |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead remove the original jsdoc and make it
/** @deprecated - use maskCapturedNetworkRequestFn instead */
requires: PostHog/posthog-js#902 We don't want to support running posthog/network and rrweb/network plugin at the same time. These changes support that change improve timeline display allow raw json view for debugging
As discussed https://posthog.slack.com/archives/C03PB072FMJ/p1700077249746539
first draft (while looking after poorly #4) of removing our own timings plugin in favour of (a copy of) the rrweb plugin
TODO