Skip to content

feat(eap): Convert measurements to attributes smartly#6007

Open
loewenheim wants to merge 3 commits into
masterfrom
sebastian/measurements-to-attributes
Open

feat(eap): Convert measurements to attributes smartly#6007
loewenheim wants to merge 3 commits into
masterfrom
sebastian/measurements-to-attributes

Conversation

@loewenheim
Copy link
Copy Markdown
Contributor

Based on #6006.

This uses measurement_to_attribute from the previous PR during span V1 -> V2 conversion to convert a measurement to a current attribute, if possible.

As can be seen from the tests, this means that in some cases the "directly translated" name (e.g. "cls") will no longer be present. If we judge this too risky, we can also double write in the conversion, i.e. insert both the bare measurement name and the proper attribute name.

ref: INGEST-909.

@loewenheim loewenheim requested a review from a team as a code owner May 19, 2026 13:04
@loewenheim loewenheim self-assigned this May 19, 2026
@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 19, 2026

INGEST-909

Comment on lines +594 to +595
"browser.web_vital.cls.value": 100.0,
"browser.web_vital.fcp.value": 200.0,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We need to make sure the product can actually deal with these new attributes first. Coalesce etc. must work.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is a range of options for how defensive to be here, from "always double write the replacement attribute name and the literal measurement name" to "check if the literal measurement name wants to be backfilled in sentry-conventions.

Base automatically changed from sebastian/conventions-measurements to master May 20, 2026 09:04
@loewenheim loewenheim force-pushed the sebastian/measurements-to-attributes branch from 4c75553 to a31a63a Compare May 20, 2026 10:09
@loewenheim
Copy link
Copy Markdown
Contributor Author

@Dav1dde I've now introduced a feature flag for "smart" measurements conversion. Would appreciate some stylistic critique (naming, how checking the feature/threading through the flag is done).

I've also expanded the relevant tests to work with or without the feature.

Comment thread relay-server/src/processing/legacy_spans/mod.rs Outdated
Comment thread relay-server/src/processing/spans/process.rs
Comment thread relay-spans/src/v1_to_v2.rs
@Dav1dde
Copy link
Copy Markdown
Member

Dav1dde commented May 20, 2026

I've now introduced a feature flag for "smart" measurements conversion. Would appreciate some stylistic critique (naming, how checking the feature/threading through the flag is done).

Looks good 👍, left some comments and a nit.

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.

2 participants