Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Preserve full Span.Description #674

Merged
merged 2 commits into from
Jul 27, 2020

Conversation

rhcarvalho
Copy link
Contributor

SDKs store potentially lenghty information in the description that is
only useful when complete. In particular, the DB integration in Python
sets the description to a SQL query. Users looking at slow spans want to
be able to read the SQL query as part of understanding why it was slow.

Truncation after 1024 bytes means often times not even the SELECT part
of the query is readable.

While we may consider changing SDKs to send the full query in a
different field and leverage that in the Sentry UI (e.g. syntax
highlighting), the expedient solution to fix the current UX is to remove
the truncation in Relay.

We could alternatively increase the limit, e.g.

#[metastructure(max_chars = "message")]

would allow for 8192 bytes, 8x more than 'summary' at 1024 bytes,
however, that becomes just yet another arbitrary truncation point, way
below the limit for total payload size.

Since too large payloads would have been discarded before they hit Relay
anyway, it seems fair to drop the truncation for now and observe.

We can safely revert this change later if necessary.

SDKs store potentially lenghty information in the description that is
only useful when complete. In particular, the DB integration in Python
sets the description to a SQL query. Users looking at slow spans want to
be able to read the SQL query as part of understanding why it was slow.

Truncation after 1024 bytes means often times not even the SELECT part
of the query is readable.

While we may consider changing SDKs to send the full query in a
different field and leverage that in the Sentry UI (e.g. syntax
highlighting), the expedient solution to fix the current UX is to remove
the truncation in Relay.

We could alternatively increase the limit, e.g.

  #[metastructure(max_chars = "message")]

would allow for 8192 bytes, 8x more than 'summary' at 1024 bytes,
however, that becomes just yet another arbitrary truncation point, way
below the limit for total payload size.

Since too large payloads would have been discarded before they hit Relay
anyway, it seems fair to drop the truncation for now and observe.

We can safely revert this change later if necessary.
@rhcarvalho rhcarvalho requested review from jan-auer and a team July 23, 2020 12:06
@rhcarvalho
Copy link
Contributor Author

@jan-auer tagging you for review since we agreed to do this.

Hmm let's see if tests actually tested truncation :)

@untitaker
Copy link
Member

I'm gonna let jan decide on this since I am lacking context. We'll have to carefully monitor this at the very least.

* master:
  meta: Upgrade git pre-commit hook to python3 (#676)
  test(server): Ensure invalid minidumps leave events creation intact (#675)
  feat(processing): Extract event timestamp from minidump (#662)
@jan-auer
Copy link
Member

Let's try this. I'm slightly in favor of having some limit, even if it is even higher than 8k (which no longer sounds all that useful), but this only becomes relevant if sizes increase too much.

@jan-auer jan-auer merged commit b8781d6 into master Jul 27, 2020
@jan-auer jan-auer deleted the rhcarvalho/span-description-limit branch July 27, 2020 09:32
mjq-sentry pushed a commit to getsentry/sentry that referenced this pull request Sep 1, 2022
…ncate

I thought we truncated the description field, but we haven't for two years:
getsentry/relay#674
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.

3 participants