Index lcp.element as a tag

[sdzhong]

Is there an existing issue for this?

• I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• I have reviewed the documentation https://docs.sentry.io/
• I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/browser

SDK Version

8

Framework Version

No response

Link to Sentry event

No response

Reproduction Example/SDK Setup

Please see https://demo.sentry.io/performance/summary/tags/?project=5808623&query=&statsPeriod=14d&tagKey=backendType&transaction=%2Fproducts:

Steps to Reproduce

lcp-shows-but-no-lcp.element-tag.mp4

Expected Result

lcp.element and other lcp.* tags appear for the transaction

Actual Result

[sdzhong]

From internal conversations:

It’s because of this in v8

sentry-javascript/packages/browser-utils/src/metrics/browserMetrics.ts

Line 614 in 615c670

span.setAttribute('lcp.element', htmlTreeAsString(_lcpEntry.element));

lcp element is set as an attribute (span data) instead of a tag, so it’s not being indexed automatically in the transaction any more.

[Lms24]

Backlogging to triage after hackweek

[Lms24]

lcp element is set as an attribute (span data) instead of a tag, so it’s not being indexed automatically in the transaction any more.

Yes, that's correct. I diffed v7 and v8 payloads for the former LCP tags and this is the difference for LCP in the transaction event:

v7:

{
  "contexts": {
    "trace": {
      "tags": {
        "lcp.element": "body > div#app > div > h1",
        "lcp.size": 24827
      }
    }
  },
  "tags": {
    "lcp.element": "body > div#app > div > h1",
    "lcp.size": 24827
  },
  "measurements": {
    "lcp": { "value": 106.20000000298023, "unit": "millisecond" }
  }
}

v8:

{
  "contexts": {
    "trace": {
      "data": {
        "lcp.element": "body > div#app > div > h1",
        "lcp.size": 24827
      }
    }
  },
  "measurements": {
    "lcp": { "value": 146.20000000298023, "unit": "millisecond" }
  }
}

Why was this changed?

In v8, we removed the span.setTag API because OpenTelemetry has no concept of tags and we needed to stay consistent with span APIs in browser and in Node (where we switchted to OpenTelemetry). So therefore, what previously was set as span.setTag was converted to getCurrentScope.setTag or span.setAttribute. In case of web vital tags, we always converted to span.setAttribute because this would ensure that the values are only applied to the pageload span and not to other events (like errors). Values set via span.setAttribute end up in the event.trace.context.data field, as shown in the example payload above.

Now the question is, how do we fix it? We could special-case this in the SDK but it increases complexity and arguably bings us closer to tags again which we generally don't want to (also for span-only streaming). So I'm wondering if we can adjust the tag extraction logic in Relay. @edwardgou-sentry do you know if this would be possible in Relay?

[edwardgou-sentry]

@edwardgou-sentry do you know if this would be possible in Relay?

@Lms24 If we want to extract the lcp.element from the trace context and into tags, I think it should be possible. We should be able to do something similar to this:
https://github.com/getsentry/relay/blob/master/relay-event-normalization/src/event.rs#L688

I'm unsure if there would be potential issues with doing this though. Do we know if there are other tags we're missing from this change aside from the lcp ones?

[Lms24]

Here's a list of values we previously set as tags on the transaction and now as attributes on the root span in the browserTracingIntegration:

• effectiveConnectionType
• connectionType
• deviceMemory
• hardwareConcurrency
• lcp.element
• lcp.id
• lcp.url
• lcp.size
• cls.source.*
• visibilitychange -> setAttribute('sentry.cancellation_reason', 'visibilitychange')

I'm not sure which of these data points actually need to be extracted to tags. I was under the impression that we audited these and checked with the performance team. However, I didn't find the issue I had in mind, so I'm not too sure anymore.

Anyway, I discussed this with the team and we'd prefer the necessary values being extracted in Relay. Re-introducing attribute->tag conversion logic in the SDK (which will be intransparent for users) seems wrong, given the span only future where tags on spans will not exist anymore.

[edwardgou-sentry]

I think we can go with extracting lcp.element in Relay then. @bcoe has already added this ticket to the Insights board, so we can prioritize it and get someone to pick it up.

[Lms24]

Perfect, thanks! I'm gonna set this as done for the Web SDK Frontend board and unassign myself from the issue given it's not fixed in SDK code.

Also, should we transfer this issue to the relay repo?

[edwardgou-sentry]

@getsentry/ingest dropping this in the Ingest board for now since it seems like there's work to be done in Relay to support this.

Quick summary: JS SDK sends certain tags differently between v7 and v8 payloads. Specifically lcp.element and lcp.size gets sent under the trace context now: #13411 (comment)

Would it be possible to update relay to extract these tags from the new v8 structure instead?

[jjbayer]

@edwardgou-sentry yes if customers actually expect this field to show up as a tag, we can do this. Since you already identified a possible code location for the fix, would you be willing to implement this?

[edwardgou-sentry]

@edwardgou-sentry yes if customers actually expect this field to show up as a tag, we can do this. Since you already identified a possible code location for the fix, would you be willing to implement this?

@jjbayer Sure, I have some time to look at this today! Will drop a pr for review

[jjbayer]

Implemented in getsentry/relay#4074.