feat(spans): Align otel attributes with sentry span#3457
Conversation
0dedad1 to
980b6d3
Compare
| pub replay_id: Annotated<Value>, | ||
|
|
||
| #[metastructure(field = "sdk.name")] | ||
| #[metastructure(field = "sentry.sdk.name")] |
There was a problem hiding this comment.
No legacy alias need here, this field was only introduced in #3456.
| #[serde(rename(deserialize = "exclusive_time"))] | ||
| exclusive_time_ms: f64, | ||
| #[serde(default)] | ||
| is_segment: bool, |
There was a problem hiding this comment.
When we cannot derive is_segment, is should be set to false.
| ("enduser.id", "user.id"), | ||
| ("http.request.cookies", "request.cookies"), | ||
| ("http.request.env", "request.env"), | ||
| ( | ||
| "http.request.headers.content-type", | ||
| "request.headers.content-type", | ||
| ), | ||
| ("http.request.method", "request.method"), | ||
| ("sentry.environment", "environment"), | ||
| ("sentry.origin", "origin"), | ||
| ("sentry.release", "release"), | ||
| ("sentry.sample_rate", "sample_rate"), | ||
| ("sentry.sdk.integrations", "sdk.integrations"), | ||
| ("sentry.sdk.name", "sdk.name"), | ||
| ("sentry.sdk.packages", "sdk.packages"), | ||
| ("sentry.sdk.version", "sdk.version"), | ||
| ("sentry.source", "source"), | ||
| ("sentry.user.email", "user.email"), | ||
| ("sentry.user.geo.city", "user.geo.city"), | ||
| ("sentry.user.geo.country_code", "user.geo.country_code"), | ||
| ("sentry.user.geo.region", "user.geo.region"), | ||
| ("sentry.user.ip_address", "user.ip_address"), | ||
| ("sentry.user.segment", "user.segment"), | ||
| ("sentry.user.username", "user.username"), | ||
| ("url.full", "request.url"), | ||
| ("url.query_string", "request.query_string"), |
There was a problem hiding this comment.
@phacops do you know if any of these mappings was already used for product features? If not, I'd rather remove the mapping and start fresh.
|
|
||
| /// The sentry environment. | ||
| #[metastructure(field = "environment")] | ||
| #[metastructure(field = "sentry.environment", legacy_alias = "environment")] |
There was a problem hiding this comment.
The disadvantage of this rename is that the new key will now be serialized into the payload. But span.data is not (yet) forwarded to sentry directly on standalone spans (we only extract some of it into sentry_tags, but that happens in relay). Spans as part of transactions are materialized into nodestore, but they don't rely on these particular span.data items AFAIK.
| // TODO: This is wrong, a segment could still have a parent in the trace. | ||
| let is_segment = parent_span_id.is_none().into(); | ||
|
|
There was a problem hiding this comment.
Relying on set_segment_attributes instead.
| if let Some(statement) = otel_value_to_string(value) { | ||
| description = statement; | ||
| } | ||
| match attribute.key.as_str() { |
There was a problem hiding this comment.
Sorry for the large diff, I converted this if to a match expression.
There was a problem hiding this comment.
Large diffs that clean up code make me happy.
| let otel_span: OtelSpan = serde_json::from_str(json).unwrap(); | ||
| let span_from_otel = otel_to_sentry_span(otel_span); | ||
|
|
||
| // TODO: measurements, metrics_summary |
There was a problem hiding this comment.
This will need to be handled in a follow-up.
| if let Some(statement) = otel_value_to_string(value) { | ||
| description = statement; | ||
| } | ||
| match attribute.key.as_str() { |
There was a problem hiding this comment.
Large diffs that clean up code make me happy.
Co-authored-by: David Herberth <david.herberth@sentry.io>
|
|
||
| /// The sentry SDK (see [`crate::protocol::ClientSdkInfo`]). | ||
| #[metastructure(field = "sdk.name")] | ||
| #[metastructure(field = "sentry.sdk.name")] |
There was a problem hiding this comment.
No need for an alias here, this field was only introduced in #3456.
To simplify conversion in the future, align attribute names in Otel's span attributes with keys in Sentry's
span.data. The old keys inspan.dataremain as legacy alias.See also getsentry/develop#1230 for the span schema / attribute convention.
ref: #3278