You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
While working on #3998, it became clear to me that the code for propagating traces is overly complex. This issue documents concrete recommendations I have for how to improve the code's understandability and maintainability.
Most problems stem from the fact that trace information is duplicated between the scope and the transaction. Furthermore, the representations used to store the information in each are different. On the scope, we store trace information in the PropagationContext , while we use Baggage to store the information in the transaction.
The following sections list my recommendations.
Only store trace information on the scope
Since there is always a scope, but only sometimes a transaction, we should store the trace information only on the scope. The APIs for accessing trace information from the transaction can be modified to instead get the information from the scope. Having a single source of truth for trace information on the scope makes it easier to reason about where the trace information is stored, and makes it much simpler to add support for new fields on the propagation context.
Differentiate between communication protocols and data representations
The baggage header is a protocol for communicating trace information between SDKs across service boundaries. However, we also have a Baggage class to store "baggage" information on the transaction, even though we could (and should) be storing an abstract representation of the trace information (something like PropagationContext), instead.
Representing the trace information abstractly has the benefit that we can define APIs for more nicely accessing the various items in the propagation context, rather than needing to index a dictionary as we currently do. Having such APIs would also make it easier to understand what information the propagation context stores. Of course, we need to maintain future compatibility by allowing for "unknown" fields, but the abstract representation can handle this logic for us.
We should only use the concept of "baggage" at the service boundary when deserializing incoming trace data and serializing outgoing trace data.
POTel might make some/all of this issue irrelevant, but in case these recommendations still apply afterwards, we should refactor accordingly.
The text was updated successfully, but these errors were encountered:
While working on #3998, it became clear to me that the code for propagating traces is overly complex. This issue documents concrete recommendations I have for how to improve the code's understandability and maintainability.
Most problems stem from the fact that trace information is duplicated between the scope and the transaction. Furthermore, the representations used to store the information in each are different. On the scope, we store trace information in the
PropagationContext
, while we useBaggage
to store the information in the transaction.The following sections list my recommendations.
Only store trace information on the scope
Since there is always a scope, but only sometimes a transaction, we should store the trace information only on the scope. The APIs for accessing trace information from the transaction can be modified to instead get the information from the scope. Having a single source of truth for trace information on the scope makes it easier to reason about where the trace information is stored, and makes it much simpler to add support for new fields on the propagation context.
Differentiate between communication protocols and data representations
The baggage header is a protocol for communicating trace information between SDKs across service boundaries. However, we also have a
Baggage
class to store "baggage" information on the transaction, even though we could (and should) be storing an abstract representation of the trace information (something likePropagationContext
), instead.Representing the trace information abstractly has the benefit that we can define APIs for more nicely accessing the various items in the propagation context, rather than needing to index a dictionary as we currently do. Having such APIs would also make it easier to understand what information the propagation context stores. Of course, we need to maintain future compatibility by allowing for "unknown" fields, but the abstract representation can handle this logic for us.
We should only use the concept of "baggage" at the service boundary when deserializing incoming trace data and serializing outgoing trace data.
POTel might make some/all of this issue irrelevant, but in case these recommendations still apply afterwards, we should refactor accordingly.
The text was updated successfully, but these errors were encountered: