Skip to content

Conversation

@mydea
Copy link
Member

@mydea mydea commented Jun 25, 2024

While looking into #11646, I decided to add an E2E test for INP spans.

@mydea mydea requested review from AbhiPrasad and lforst June 25, 2024 08:21
@mydea mydea self-assigned this Jun 25, 2024
};

listener(Buffer.from(JSON.stringify(data)).toString('base64'));
try {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also added this to clean up logs here a bit, replay events will always fail to parse because they are compressed and not JSON.

console.error(
'[event-proxy-server] Error: Failed to parse Sentry request envelope',
error,
proxyRequestBody,

Check warning

Code scanning / CodeQL

Log injection

Log entry depends on a [user-provided value](1).
},
description: 'body > div#root > input#exception-button[type="button"]',
op: 'ui.interaction.click',
parent_span_id: expect.any(String),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't parent span be undefined for INP spans?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it? 😅 well today it is not! if there is an ongoing transaction, it will be a child of that. is that not correct? for me this seems like it makes sense, but 🤷 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was under the impression that INP spans were always root spans, but maybe I'm mistaken.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @Lms24 when you're eventually back, you checked this out at the time. I'll merge this PR (as it shows the current behavior) but we can double check if this is fine.

@mydea mydea force-pushed the fn/inp-e2e-test branch from 65cb433 to 0d924fc Compare June 25, 2024 14:22
@mydea mydea merged commit 7b39a22 into develop Jun 25, 2024
@mydea mydea deleted the fn/inp-e2e-test branch June 25, 2024 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants