Improve transaction & span docs

[Tyrrrz]

As discussed on the call with @bruno-garcia and @maciejwalkowiak, the docs on transaction & span (i.e. the performance feature) are lacking. Currently, it's really hard to implement this feature correctly as it requires a lot of guessing, reverse-engineering and asking people questions, all of which should ideally be answered by the documentation.

Issues:

• Transaction docs doesn't contain an example of a full valid transaction payload. It has an example showing contexts.trace and spans but not the rest of the fields. Ideally, there should be an example of a minimum valid transaction payload (with only required fields set) and a maximum valid transaction payload (with all of the fields set).
• Transaction/span schemas are missing from https://github.com/getsentry/sentry-data-schemas/blob/main/relay. Adding them would be very helpful. (Issue raised here Add schema of Transaction sentry-data-schemas#2 )
• The docs on transaction mention that "Transactions are Events enriched with Span data". Does it imply that all event fields are valid and make sense on transaction? For example, would having exception data make sense on transaction? Currently, it's not clear what is the actual contract of the transaction payload.
• The docs should mention that relay may return 200 OK on invalid transactions (not sure if by design or a bug). For example, a transaction that doesn't have contexts.trace will get accepted by the relay but won't show up on the portal. Generally speaking, a list of common issues and solutions would be nice.
• The docs say that "transactions are events with span data", but only briefly mention that the span data is actually stored inside contexts.trace. I think this part should be emphasized because it makes no sense from the consumer perspective, but at the same time has critical implications on the design of the SDK.
• The docs say that the span data is included within contexts.trace, but it's not completely true. Some fields like start/end timestamps and description are included on the object itself, despite also being part of "span data". There is an example of contexts.trace in the docs, but again, it's not clear whether it's exhaustive or not. What about tags and data, are they also on contexts?
• The docs on spans don't cover the span_id/parent_span_id/sampled fields and potentially some others. What is the list of fields required/allowed on spans?
• SpanId and ParentSpanId appear to be half the length of other Id fields (i.e. 16 characters instead of 32). The docs don't cover how these IDs are generated, nor say anything about this limitations. Some SDKs are generating these IDs by truncating UUIDs, which is wrong. There should be some guidance regarding this. Edit: looks like it's not actually a problem
• The docs on spans start with "This interface is an object with a single values attribute containing an ordered list of Span objects. Spans in the list don't have to be ordered, they will be ordered by start / end time on the Server.", which is not true. The span does not have a values attribute (at least to my knowledge).
• The docs on spans show an array of spans as an example, which is confusing as it's not clear whether the span itself contains other spans or is just a list of spans from the transaction.
• The docs mention that transaction is a hierarchy of spans, but doesn't cover how they are stored. Does each span contain its own list of spans? Or do the spans gets projected onto a planar list inside of the transaction?
• The docs don't provide information regarding how the transaction and event get linked together.
• The docs on transaction/spans should link to https://develop.sentry.dev/sdk/unified-api/tracing. I had no idea this article existed and it could have probably answered many of my questions.
• Apparently, when running on the backend, the op must always be set to http.server. Would be nice if this was documented too.

[Tyrrrz]

Looking through https://develop.sentry.dev/sdk/unified-api/tracing, I think a lot of the issues I've encountered could've been avoided had I known that this article existed 😞
It would be nice if it was linked from other docs to increase discoverability.

[Tyrrrz]

I tried making a PR to improve the docs, but I can't understand how the code is actually mapped to what can be seen on the website. It seems the information is different in some places.

[bruno-garcia]

Another point to aling: https://develop.sentry.dev/sdk/unified-api/tracing#interaction-with-beforesend-and-event-processors

In JS beforeSend isn't called but EventProcessors are. This will have different side effects in different SDKs and we've decided not to pass transactions through event processors in .NET and Java. We need to align here or at least clarify on the docs.

[bruno-garcia]

/cc @brustolin

[bruno-garcia]

To consider:getsentry/sentry-docs#2741 (review)

[rhcarvalho]

I tried making a PR to improve the docs, but I can't understand how the code is actually mapped to what can be seen on the website. It seems the information is different in some places.

Unfortunately that might be a case of #170, last I heard this was a bug in Gatsby.

[bruno-garcia]

I tried making a PR to improve the docs, but I can't understand how the code is actually mapped to what can be seen on the website. It seems the information is different in some places.

Unfortunately that might be a case of #170, last I heard this was a bug in Gatsby.

Is this being tracked anywhere?

[marandaneto]

The docs should mention that relay may return 200 OK on invalid transactions (not sure if by design or a bug). For example, a transaction that doesn't have contexts.trace will get accepted by the relay but won't show up on the portal. Generally speaking, a list of common issues and solutions would be nice.

its generally true for events too, as there's an ingestion pipeline, and events are not validated right away

[marandaneto]

The docs say that "transactions are events with span data", but only briefly mention that the span data is actually stored inside contexts.trace. I think this part should be emphasized because it makes no sense from the consumer perspective, but at the same time has critical implications on the design of the SDK.

https://develop.sentry.dev/sdk/event-payloads/transaction/#contextstrace

[marandaneto]

The docs say that the span data is included within contexts.trace, but it's not completely true. Some fields like start/end timestamps and description are included on the object itself, despite also being part of "span data". There is an example of contexts.trace in the docs, but again, it's not clear whether it's exhaustive or not. What about tags and data, are they also on contexts?

partially covered by https://develop.sentry.dev/sdk/event-payloads/transaction/ and will be clearer when #316 is done too

[marandaneto]

The docs on spans start with "This interface is an object with a single values attribute containing an ordered list of Span objects. Spans in the list don't have to be ordered, they will be ordered by start / end time on the Server.", which is not true. The span does not have a values attribute (at least to my knowledge).

already fixed https://develop.sentry.dev/sdk/event-payloads/span/

A Transaction may contain zero or more Spans in an array attribute named spans. Spans in the list don't have to be ordered, they will be ordered by start / end time on the Server.

[marandaneto]

The docs on spans show an array of spans as an example, which is confusing as it's not clear whether the span itself contains other spans or is just a list of spans from the transaction.

it'll be also fixed by #316 when a full example with "real" data

[marandaneto]

The docs on transaction/spans should link to https://develop.sentry.dev/sdk/unified-api/tracing. I had no idea this article existed and it could have probably answered many of my questions.

now its https://develop.sentry.dev/sdk/performance/
I don't think this is an issue actually, one is the payload docs, and the other the SDK guidelines for implementing the feature.
both are top-level docs under the SDK DEVELOPMENT section

[marandaneto]

created separated issues for each item of the description, some of them were already fixed, some of them already existed under different issues, etc, closing it :)