-
Notifications
You must be signed in to change notification settings - Fork 524
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
Define a data model for span links #7171
Comments
We definitely need the trace context: a trace ID and span ID. The span ID may correspond to either a transaction or span in our model. We could skip storing attributes for the moment, but leave room for introducing them later. I propose we use an array-of-objects encoding, to minimise storage costs and simplify queries. Each object would have a trace ID and span ID, and maybe later some attributes/labels. Naming-wise, I can see a couple of options:
My current preference is for the latter. It's probably premature to try and extend the related fieldset. So if we go with
e.g. {
"trace": {
"id": "trace1"
},
"transaction": {
"id": "transaction1",
"name": "GET /foo"
},
"span": {
"links": [
{"trace": {"id": "trace1"}, "span": {"id": "span1"}},
{"trace": {"id": "trace2"}, "span": {"id": "span2"}},
]
}
} Due to the way arrays of objects are flattened, we won't be able to sensibly search for links by both trace ID and span ID. e.g. consider the example above, where there are two links to spans in different traces. If one were to search |
I have confirmation from @cauemarcondes that this won't pose a problem for the UI, as it only ever queries by |
OpenTelemetry has a concept of "span links", which is a property of a span that is a collection of span identifiers. This can be used, for example, when restarting a trace: in this case the incoming span context could be preserved in span link. For the same reasons, we would also like to support span links in Elastic APM: see elastic/apm#286 (comment).
We need to define how span links should be stored in Elasticsearch. See elastic/apm#122 for some previous discussions on this.
The text was updated successfully, but these errors were encountered: