feat(web-vitals): Emit web-vitals as metrics by klochek · Pull Request #6118 · getsentry/relay

klochek · 2026-06-23T02:20:12Z

No description provided.

linear-code · 2026-06-23T02:20:16Z

jjbayer · 2026-06-23T11:32:08Z

+        name: "browser.web_vital.cls.value",
+        unit: MetricUnit::None,
+        attribute_keys: &[
+            "browser.web_vital.cls.value",


Is attribute_value always also part of attribute_keys? In that case I would rename attribute_keys to something like additional_attributes and omit the value-holding attribute from it.

On second inspection, why do we add the metric value as an attribute? Isn't that redundant?

I'll be double-checking with the web-vitals folks about this, but this is as outlined in their rfc.

jjbayer · 2026-06-23T11:40:50Z

        };

        message.try_accept(|span| {
+            self.produce_web_vital_metrics(scoping, received_at, &span)?;


Does this call need to be guarded by a global option or feature flag in any way?

Yes, it should--once I've got the rest of the pinned down.

cursor · 2026-07-06T20:09:07Z

+            s.send_to_store(span.wrap(item));
+        }
+    }
+}


Skips trace metrics feature flag

Medium Severity

Derived web-vital trace metrics are written straight to the store via produce_webvitals_metrics, so they never pass the trace-metrics processor’s Feature::TraceMetricsIngestion gate or its inbound filtering. Projects with trace metric ingestion disabled can still get these Kafka trace items.

Additional Locations (2)

relay-server/src/processing/spans/mod.rs#L276-L278

relay-server/src/processing/legacy_spans/mod.rs#L204-L206

^{Reviewed by Cursor Bugbot for commit 2c157e7. Configure here.}

This we'll have to check with Ben, but seems okay?

Dav1dde · 2026-07-07T07:09:16Z

+    span: &Managed<Box<crate::services::store::StoreSpanV2>>,
+    metrics: Vec<TraceMetric>,
+) {
+    use crate::processing::trace_metrics;


This doesn't seem necessary, the code is already in this module

Dav1dde · 2026-07-07T07:09:49Z


+#[cfg(feature = "processing")]
+/// Produce the supplied webvital trace metrics to kafka.
+pub fn produce_webvitals_metrics(


Maybe can move it into mod store and just pub use here?

Dav1dde · 2026-07-07T07:10:29Z

+            s.send_to_store(span.wrap(item));
+        }
+    }
+}


This we'll have to check with Ben, but seems okay?

Dav1dde · 2026-07-07T07:12:16Z

+/// Extract any web vitals metrics for the supplied v2 span.  Bad or missing metrics will be
+/// silently dropped.
+pub fn extract_web_vital_metrics(span: &SpanV2) -> Option<Vec<TraceMetric>> {
+    let Some(attrs) = &span.attributes.0 else {


let attrs = span.attributes.0.as_ref()? should do

Dav1dde · 2026-07-07T07:15:45Z

+    let op_name = attrs.get_value("sentry.op")?;
+
+    if !WEB_VITAL_SPAN_NAMES.contains(&op_name.as_str().unwrap_or_default()) {


Suggested change

let op_name = attrs.get_value("sentry.op")?;

if !WEB_VITAL_SPAN_NAMES.contains(&op_name.as_str().unwrap_or_default()) {

let op_name = attrs.get_value("sentry.op")?.as_str()?;

if !WEB_VITAL_SPAN_NAMES.contains(&op_name) {

return None;

}

Dav1dde · 2026-07-07T07:17:37Z

+        }
+
+        // This is for attribution: we'll be able to tell on a metric if it came from relay.
+        attributes.insert("sentry.metric.source", "span");


This (sentry.metric.source) needs to be defined sentry-conventions if it isn't already.

Dav1dde · 2026-07-07T07:20:09Z

+        // CLS webvitals are a little weird, in that they can have an arbitrary number of
+        // "source" attributes (with a .N postfix, 0-based, monotonically increasing), so we
+        // exhaustively look for them here.
+        if web_vital.attribute_value == "browser.web_vital.cls.value" {


Suggested change

if web_vital.attribute_value == "browser.web_vital.cls.value" {

if web_vital.attribute_value == BROWSER__WEB_VITAL__CLS__VALUE {

Dav1dde · 2026-07-07T07:20:32Z

+        // exhaustively look for them here.
+        if web_vital.attribute_value == "browser.web_vital.cls.value" {
+            for i in 0..MAX_CLS_SOURCES {
+                let attr_key = format!("browser.web_vital.cls.source.{i}");


Suggested change

let attr_key = format!("browser.web_vital.cls.source.{i}");

let attr_key = browser__web_vital__cls__source__key(i);

(Needs a small change first that the function accepts impl Display).

Dav1dde · 2026-07-07T07:21:13Z

+                if let Some(v) = attrs.get_attribute(&attr_key) {
+                    attributes.insert(attr_key, v.value.clone());
+                } else {
+                    break;
+                }


Just a small style nit, in Relay we often use match statements for small if-else constructs like this

Dav1dde · 2026-07-07T07:22:04Z

+    "sentry.pageload.span_id",
+    "sentry.origin",
+    "sentry.transaction",
+    "user_agent.original",
+    "sentry.release",
+    "sentry.environment",
+    "sentry.sdk.name",
+    "sentry.sdk.version",
+    "sentry.platform",


These also all need to come from conventions

loewenheim · 2026-07-07T07:51:51Z

+
+
+# Test standalone spans (just lcp)
+def test_v1_standalone_span(mini_sentry, relay_with_processing, items_consumer):


You could parametrize this test with mode (like some tests in test_spans_standalone) to see whether the legacy and experimental standalone span pipeline work alike.

loewenheim

Looks good, the main concern is around using attribute constants from relay-conventions (and possibly defining things you need in sentry-conventions) as @Dav1dde mentioned.

logaretm

Just caught a small issue with the CLS source starting index, otherwise LGTM.

logaretm · 2026-07-07T19:16:23Z

+                if let Some(v) = attrs.get_attribute(&attr_key) {
+                    attributes.insert(attr_key, v.value.clone());
+                } else {
+                    break;


The SDK is sending the CLS starting from 1 rather than 0 so this would actually skip all incoming sources from an actual SDK because there is no 0 index.

While this looked weird to me at first, it is according to spec tho. So we should start the loop at 1 instead. Maybe let's throw in a test as well for this.

logaretm · 2026-07-07T19:32:15Z

+        attributes.insert("sentry.metric.source", "span");
+
+        let trace_metric = TraceMetric {
+            timestamp: span.start_timestamp.clone(),


I thought about usingend_timestamp instead but both aren't really accurate so it doesn't matter, both are equally bad at approximating the actual metric timestamp. Just wanted to note this down.

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

^{Reviewed by Cursor Bugbot for commit 9b164ee. Configure here.}

Dav1dde · 2026-07-14T07:09:55Z

Can drop this when #6220 is merged.

Dav1dde · 2026-07-14T07:22:51Z

+];
+
+const COMMON_ATTRIBUTES: [&str; 9] = [
+    "sentry.pageload.span_id",


Added to conventions with #6220

Dav1dde · 2026-07-14T07:51:33Z

    "with-serde",
    "trace",
 ] }
+relay-metrics = { workspace = true }


Urgh we should change this, trace metrics should not depend on our metrics (doesn't need to be this PR).

I guess a simple "fix" is to re-export it from the trace metric protocol, or just actually define it there as it is independent.

Dav1dde · 2026-07-14T07:53:38Z


+#[cfg(feature = "processing")]
+/// Produce the supplied webvital trace metrics to kafka.
+pub fn produce_webvitals_metrics(


If you move this to store.rs and then pub import it, you don't need as many conditional imports and it keeps this module clean(er).

Which store.rs did you mean?

Sorry this is already in store.rs I got tricked because of the cfg(feature = "processing") flags - the entire module is already guarded behind the processing feature.

Dav1dde · 2026-07-14T07:54:18Z

 }

+#[cfg(feature = "processing")]
+/// Produce the supplied webvital trace metrics to kafka.


Some context here would be good why this is needed and what the intentions are. E.g. for now we need this for double writing and in the future the plan is to remove this and replace it by converting spans to metrics instead.

Dav1dde

Changelog entry makes sense here also a quick PR description describing intent + linking to the RFC would be useful to have (will get materialized in the commit message).

Dav1dde · 2026-07-15T08:05:44Z

Oops, the rare double approve.

sentry · 2026-07-16T17:31:23Z

+                let attr_key = browser__web_vital__cls__source__key(i.to_string());
+                match attrs.get_attribute(&attr_key) {
+                    Some(v) => attributes.insert(attr_key, v.value.clone()),
+                    None => break,
+                }
+            }


Bug: The function extract_web_vital_metrics only checks for new CLS attribute keys (browser.web_vital.cls.source.N), causing attributes from legacy spans with old keys (cls.source.N) to be dropped.
_{Severity: MEDIUM}

Suggested Fix

Modify extract_web_vital_metrics to handle both old and new CLS source attribute key formats. The function should first attempt to look up the new key (browser.web_vital.cls.source.N) and, if not found, fall back to checking for the old key (cls.source.N) before breaking the loop. This ensures that CLS source data from legacy spans is correctly processed.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: relay-spans/src/web_vitals.rs#L131-L136 Potential issue: When processing legacy spans, the `extract_web_vital_metrics` function fails to extract Cumulative Layout Shift (CLS) source attributes. The function iterates looking for attributes with the new key format, such as `browser.web_vital.cls.source.1`. However, legacy spans still use the old format, like `cls.source.1`. Because `attrs.get_attribute` returns `None` when searching for the new key in a legacy span, the loop terminates immediately via a `break` statement. This results in all CLS source attributes being silently dropped from the generated web vital metrics for these spans.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

sentry · 2026-07-16T17:38:43Z

+                if let Some(metrics) = relay_spans::extract_web_vital_metrics(&span.item) {
+                    processing::trace_metrics::produce_webvitals_metrics(s, &span, metrics);
+                }


Bug: The legacy span processing path calls extract_web_vital_metrics before running full attribute normalization, which can cause web vital metrics to be silently dropped.
_{Severity: MEDIUM}

Suggested Fix

Run the full attribute normalization pipeline, including normalize_attribute_names, on the converted SpanV2 object before calling extract_web_vital_metrics. This will align the legacy path with other transaction processing paths and ensure web vitals are extracted reliably.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: relay-server/src/processing/legacy_spans/mod.rs#L206-L208 Potential issue: In the legacy span processing path, `extract_web_vital_metrics` is invoked on a `SpanV2` object immediately after its conversion from `SpanV1`. This converted span does not undergo the full attribute normalization pipeline, specifically missing a call to `normalize_attribute_names`. The `extract_web_vital_metrics` function requires normalized attribute keys to correctly identify and extract web vital data. Because this normalization step is skipped, any spans with deprecated or non-standard attribute names will not be processed correctly, leading to the silent dropping of web vital metrics.

That legacy pipeline is gone within a week (hopefully). So doesn't matter.

Dav1dde reviewed Jun 23, 2026

View reviewed changes

jjbayer reviewed Jun 23, 2026

View reviewed changes

klochek force-pushed the christopherklochek/ingest-962-implement-double-writes-for-web-vitals branch from 4eb2c66 to 2c157e7 Compare July 6, 2026 19:15

klochek marked this pull request as ready for review July 6, 2026 20:05

klochek requested a review from a team as a code owner July 6, 2026 20:05

klochek requested a review from logaretm July 6, 2026 20:05

cursor Bot reviewed Jul 6, 2026

View reviewed changes

Dav1dde reviewed Jul 7, 2026

View reviewed changes

Dav1dde mentioned this pull request Jul 7, 2026

ref(conventions): Make key fn accept std::fmt::Display #6185

Merged

loewenheim reviewed Jul 7, 2026

View reviewed changes

logaretm reviewed Jul 7, 2026

View reviewed changes

klochek force-pushed the christopherklochek/ingest-962-implement-double-writes-for-web-vitals branch from 2c157e7 to 9b164ee Compare July 13, 2026 18:50

cursor Bot reviewed Jul 13, 2026

View reviewed changes

Comment thread relay-spans/src/web_vitals.rs

Dav1dde reviewed Jul 14, 2026

View reviewed changes

klochek force-pushed the christopherklochek/ingest-962-implement-double-writes-for-web-vitals branch from 57fe071 to 032738b Compare July 14, 2026 15:34

Dav1dde approved these changes Jul 15, 2026

View reviewed changes

sentry Bot reviewed Jul 16, 2026

View reviewed changes

klochek added 9 commits July 16, 2026 13:32

feat(web-vitals): Emit web-vitals as metrics

ddcfdca

various fixes

a279c9d

various fixes

22499d7

feedback

2162148

update sentry conventions

4d08e81

lint + test fixes

7238a21

feedback

1c920af

remove redundant feature cfgs

bc2ef49

changelog

11135f8

klochek force-pushed the christopherklochek/ingest-962-implement-double-writes-for-web-vitals branch from 761cd6d to 11135f8 Compare July 16, 2026 17:35

sentry Bot reviewed Jul 16, 2026

View reviewed changes

		let op_name = attrs.get_value("sentry.op")?;

		if !WEB_VITAL_SPAN_NAMES.contains(&op_name.as_str().unwrap_or_default()) {

-    let op_name = attrs.get_value("sentry.op")?;
-    if !WEB_VITAL_SPAN_NAMES.contains(&op_name.as_str().unwrap_or_default()) {
+    let op_name = attrs.get_value("sentry.op")?.as_str()?;
+    if !WEB_VITAL_SPAN_NAMES.contains(&op_name) {
+        return None;
+    }

	if web_vital.attribute_value == "browser.web_vital.cls.value" {
	if web_vital.attribute_value == BROWSER__WEB_VITAL__CLS__VALUE {

	let attr_key = format!("browser.web_vital.cls.source.{i}");
	let attr_key = browser__web_vital__cls__source__key(i);



		# Test standalone spans (just lcp)
		def test_v1_standalone_span(mini_sentry, relay_with_processing, items_consumer):

Uh oh!

Conversation

klochek commented Jun 23, 2026

Uh oh!

linear-code Bot commented Jun 23, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor Bot Jul 6, 2026

Choose a reason for hiding this comment

Skips trace metrics feature flag

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loewenheim left a comment

Choose a reason for hiding this comment

Uh oh!

logaretm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dav1dde Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Dav1dde Jul 15, 2026 •

edited

Loading