Skip to content
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(spans): Extract INP metrics from spans #2969

Merged
merged 21 commits into from
Jan 31, 2024
Merged

Conversation

phacops
Copy link
Contributor

@phacops phacops commented Jan 18, 2024

Fixes #2873.

@phacops phacops requested a review from a team as a code owner January 18, 2024 23:00
relay-event-schema/src/protocol/span.rs Outdated Show resolved Hide resolved
relay-dynamic-config/src/defaults.rs Outdated Show resolved Hide resolved
@phacops phacops requested review from Dav1dde and jjbayer January 19, 2024 17:07
Copy link
Member

@jjbayer jjbayer left a comment

Choose a reason for hiding this comment

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

Do we know what volume we're expecting for this change initially? Do SDKs already send this measurement in practice?

See comment about strings.py.

relay-dynamic-config/src/defaults.rs Show resolved Hide resolved
relay-event-schema/src/protocol/span.rs Show resolved Hide resolved
relay-dynamic-config/src/defaults.rs Show resolved Hide resolved
relay-event-normalization/src/event.rs Outdated Show resolved Hide resolved
relay-event-normalization/src/event.rs Outdated Show resolved Hide resolved
relay-event-normalization/src/event.rs Outdated Show resolved Hide resolved
relay-event-normalization/src/event.rs Outdated Show resolved Hide resolved
relay-server/src/services/processor/span/processing.rs Outdated Show resolved Hide resolved
@phacops
Copy link
Contributor Author

phacops commented Jan 31, 2024

SDKs are not sending this measurement yet, volume will be 0 until the next release.

@phacops phacops enabled auto-merge (squash) January 31, 2024 23:46
@phacops phacops merged commit 0f7e3d4 into master Jan 31, 2024
20 checks passed
@phacops phacops deleted the pierre/spans-inp-metrics branch January 31, 2024 23:51
Comment on lines +361 to +366
let mut event = Event {
contexts,
measurements: span.measurements.clone(),
spans: Annotated::from(vec![Annotated::new(span.clone())]),
..Default::default()
};
Copy link
Member

Choose a reason for hiding this comment

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

This turned out hackier than I thought, with all the cloning. We could instead create the pseudo-event before the retain_items loop, pass that into normalize() instead, and .take() measurements & spans in and out of that to prevent cloning. But I would not invest time into that, we should instead have a generic normalize_performance_score function that works on both spans and events.

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.

Extract INP and INP score from spans and into metrics
4 participants