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

schemas/tracing.yml: add span.id #882

Merged
merged 3 commits into from
Jul 2, 2020
Merged

Conversation

axw
Copy link
Member

@axw axw commented Jul 1, 2020

Add span.id to the tracing fields. This field may be useful for correlating logs with more specific operations, or where transaction.id is unavailable.

The transaction.id field is not always available for trace data received from other tracing systems, such as Jaeger and OpenTelemetry. We are guaranteed to have trace.id and span.id, but not necessarily transaction.id. Correlating logs with an entire trace (via trace.id) may result in too broad a set of results, compared to correlating with span.id.

@felixbarny
Copy link
Member

Does it mean we have to change the query that correlates traces and logs so that we also query for span.id: $transaction.id in the Logs UI?

felixbarny
felixbarny previously approved these changes Jul 1, 2020
@axw
Copy link
Member Author

axw commented Jul 1, 2020

@felixbarny not sure I follow, so I'll expand a bit on the problem.

OpenTelemetry has no concept of transaction; a trace is just a collection of causally-ordered spans. Spans have a "kind", e.g. Server (i.e. server-side of a request) and Client (client-side of a request). We convert Server kind spans into transactions, but there's no reference to the that span's ID in other service-local spans, aside from its children. Even then we have no way of knowing the parent is the local root.

So for OpenTelemetry, the best we can expect to have is:

  • trace.id and transaction.id OR
  • trace.id and span.id

We probably want to extend the Logs UI to query for span.id: $span.id if that field exists in the log.

@felixbarny
Copy link
Member

So far, we only correlate based on the trace.id. For the Trace logs action, I think we wouldn't want to change this.

When jumping from the logs UI to the trace view, we currently also just consider the trace.id.

So I guess we don't need immediate changes. But we'll have to consider this for a potential logs tab in the transaction view.

simitt
simitt previously approved these changes Jul 1, 2020
code/go/ecs/tracing.go Outdated Show resolved Hide resolved
@ebeahan ebeahan added the review label Jul 1, 2020
@axw axw dismissed stale reviews from simitt and felixbarny via c08a16c July 2, 2020 06:35
Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM. Thanks everyone :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants