Skip to content

Conversation

@loewenheim
Copy link
Contributor

@loewenheim loewenheim commented Jun 10, 2025

This populates data from sentry_tags for spans in StoreService::produce_span. This logic already exists in Snuba: https://github.com/getsentry/snuba/blob/4cf53a83cc5051551612b3ae8a5556228588f731/rust_snuba/src/processors/eap_items_span.rs#L153-L162.

This makes it necessary to parse SpanKafkaMessage::data as a BTreeMap instead of a RawValue because otherwise it can't be modified.

There is still an open question about what should happen if a key that would be inserted into data based on a tag is already present. Right now I'm overwriting it, but it would be simple enough not to.

Ref #4797. Ref RELAY-83.

#skip-changelog

@loewenheim loewenheim requested a review from a team as a code owner June 10, 2025 14:47
@loewenheim loewenheim requested a review from jan-auer June 10, 2025 14:48
Co-authored-by: David Herberth <david.herberth@sentry.io>
@Dav1dde
Copy link
Member

Dav1dde commented Jun 12, 2025

Wondering if we should do tags as well right now.

@loewenheim
Copy link
Contributor Author

I.e. the next paragraph in the linked snuba code? Is there a reason not to?

@Dav1dde
Copy link
Member

Dav1dde commented Jun 12, 2025

Discussed with the performance team, we should do all of the Snuba logic in Relay in the same order. But we can do it in a separate PR.

@loewenheim loewenheim merged commit 33d0d32 into master Jun 12, 2025
28 of 29 checks passed
@loewenheim loewenheim deleted the sebastian/span-sentry-tags-data branch June 12, 2025 16:01
loewenheim added a commit that referenced this pull request Jun 18, 2025
Continuing on from #4803, this populates `data` from `tags` for spans in
`StoreService::produce_span`. The logic is based on
https://github.com/getsentry/snuba/blob/4cf53a83cc5051551612b3ae8a5556228588f731/rust_snuba/src/processors/eap_items_span.rs#L164-L173.
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.

4 participants