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

How to handle partial transactions in intake v2? #1241

Closed
simitt opened this issue Aug 1, 2018 · 43 comments
Closed

How to handle partial transactions in intake v2? #1241

simitt opened this issue Aug 1, 2018 · 43 comments
Assignees
Milestone

Comments

@simitt
Copy link
Contributor

simitt commented Aug 1, 2018

Is it problematic if spans are missing for transactions?
If so can we detect missing spans server side and log or report them anywhere? What should the behaviour be if missing spans are detected?
Figure out a solution together with @elastic/apm-ui .

@roncohen roncohen mentioned this issue Aug 1, 2018
30 tasks
@sorenlouv
Copy link
Member

sorenlouv commented Aug 1, 2018

If the transaction can somehow contain the number of spans it should have (eg. transaction.spanCount.total), the UI can correlate this number with the number of spans currently available. If they don't match we show a warning to the user.

@jalvz
Copy link
Contributor

jalvz commented Aug 1, 2018

No idea of how this all plays together, but can it happen that with DT, a user might click on a span to eg. see a trace on an upstream service, so you have to lookup by span.parent_id or similar, and it is not found because it is missing?

@roncohen roncohen changed the title How to handle partial transactions? How to handle partial transactions in intake v2? Aug 1, 2018
@simitt
Copy link
Contributor Author

simitt commented Aug 23, 2018

@jalvz every span and transaction also have a trace_id as required field with distributed tracing.

@elastic/apm-agent-devs is it possible to send a transaction.spanCount.total as proposed by @sqren ?

It could also happen that a transaction is missing, but spans are sent. How would we show this?

@sorenlouv
Copy link
Member

It could also happen that a transaction is missing, but spans are sent. How would we show this?

Will the transaction eventually show up (and how long will that take) or can the transaction be permanently lost?

Today I don't think there is any way to view spans, if the transaction is missing, so the problem can't exist.
With distributed tracing it could happen. I'm okay with just showing the spans, and not the transaction, if the problem is only temporary (few minutes).

@simitt
Copy link
Contributor Author

simitt commented Aug 23, 2018

With v2 it could definitely happen that a transaction is permanently missing.
E.g. Internal Server queue was full, transaction was dropped, but spans are stored.

@formgeist
Copy link
Contributor

formgeist commented Aug 23, 2018

I'm not sure I fully understand what happens in V2. Is it possible we can have a zoom about it tomorrow?

@felixbarny
Copy link
Member

@elastic/apm-agent-devs is it possible to send a transaction.spanCount.total as proposed by @sqren ?

that should be possible

@watson
Copy link
Contributor

watson commented Aug 24, 2018

FYI: There's a long running discussion about this in the v2 protocol design proposal

@simitt
Copy link
Contributor Author

simitt commented Aug 24, 2018

Can all the @elastic/apm-agent-devs tick off if they are fine with adding the transaction.span_count.total information in v2, or add a comment otherwise?

  • go
  • java
  • nodejs
  • python
  • ruby
  • RUM

The UI would then take care off showing some indicator that spans are missing. We are aware that the agents can only count spans started before the transaction is finished and sent to the server.

For the part where spans are stored but transactions are missing, the UI will figure out a way to show the spans. The spans still store information like trace_id, transaction_id and information about the service.

@hmdhk
Copy link
Contributor

hmdhk commented Aug 24, 2018

The RUM agent is not affected by this since we have decided to send all spans for a given transaction in the same request (due to timestamp issue)!
So I would make transaction.span_count.total optional and use it in the UI only if available!

It is not a big problem to send transaction.span_count.total for the RUM agent as well. But I would rather not send an attribute which doesn't play any function for a particular agent.

@felixbarny
Copy link
Member

+1 for making it optional, for backwards compatibility

@simitt
Copy link
Contributor Author

simitt commented Aug 24, 2018

@jahtalab even if you ensure to send transaction + spans in the same http request, the server might only process parts of the http request. It could still happen that some spans are missing in ES eventually. Thus the counter could also be useful for RUM.

@felixbarny if we agree agents will always send the information we can make it required for v2, as this hasn't been released yet.

@hmdhk
Copy link
Contributor

hmdhk commented Aug 24, 2018

@simitt , What are the possible scenarios that a partial process of a request could happen?

@simitt
Copy link
Contributor Author

simitt commented Aug 24, 2018

e.g. if some of the events in an http request can be processed and stored, but then the internal Server queue is full and events are dropped.

@felixbarny
Copy link
Member

if we agree agents will always send the information we can make it required for v2, as this hasn't been released yet.

I see

simitt added a commit to simitt/apm-server that referenced this issue Sep 3, 2018
simitt added a commit to simitt/apm-server that referenced this issue Sep 3, 2018
@watson
Copy link
Contributor

watson commented Sep 10, 2018

@roncohen correct. And I like your suggestion

@simitt
Copy link
Contributor Author

simitt commented Sep 10, 2018

I am fine with renaming on the Intake API, but renaming span_count.dropped.total to span_count.dropped in ES is not possible in a minor update, as the type would change from object to int.

@roncohen
Copy link
Contributor

@simitt OK. Would it be an option to use the opportunity now with intake v2 to change it just in the API and keep it the way it is in the Elasticsearch mapping? For 7.0 we can change the ES mapping, without requiring an update to the API.

@simitt
Copy link
Contributor Author

simitt commented Sep 10, 2018

We can absolutely do that. Will add it.

@elastic/apm-agent-devs the v2 Intake API will then

  • require: transaction.span_count.started
  • optional: transaction.span_count.dropped
  • discard: transaction.span_count.dropped.total

@felixbarny
Copy link
Member

Is there a place which lists all the changes in v2? It would be also great to see which changes are already in the v2 branch and which changes are anticipated.

@mikker
Copy link

mikker commented Sep 11, 2018

@felixbarny No public place AFAIK. There's the Google Doc but we should preferably have some place public. Maybe move the contents of the google doc to this repo, @roncohen @watson ?

@simitt
Copy link
Contributor Author

simitt commented Sep 11, 2018

Can we please move this discussion outside of this issue, and to the V2 meta issue.

@roncohen
Copy link
Contributor

agree to use the v2 meta issue. I'm checking off boxes as things get merged.

simitt added a commit to simitt/apm-server that referenced this issue Sep 11, 2018
simitt added a commit that referenced this issue Sep 11, 2018
Require `transaction.span_count.started` on Intake API v2, not indexed in ES. 
Rename `transaction.span_count.dropped.total` to `transaction.span_count.dropped` on Intake API v2, leave unchanged in ES.

implements #1241
@simitt simitt closed this as completed Sep 11, 2018
felixbarny added a commit to felixbarny/apm-agent-java that referenced this issue Sep 11, 2018
felixbarny added a commit to elastic/apm-agent-java that referenced this issue Sep 11, 2018
roncohen pushed a commit to roncohen/apm-server that referenced this issue Oct 7, 2018
Require `transaction.span_count.started` on Intake API v2, not indexed in ES. 
Rename `transaction.span_count.dropped.total` to `transaction.span_count.dropped` on Intake API v2, leave unchanged in ES.

implements elastic#1241
roncohen pushed a commit to roncohen/apm-server that referenced this issue Oct 15, 2018
Require `transaction.span_count.started` on Intake API v2, not indexed in ES. 
Rename `transaction.span_count.dropped.total` to `transaction.span_count.dropped` on Intake API v2, leave unchanged in ES.

implements elastic#1241
roncohen pushed a commit to roncohen/apm-server that referenced this issue Oct 15, 2018
Require `transaction.span_count.started` on Intake API v2, not indexed in ES. 
Rename `transaction.span_count.dropped.total` to `transaction.span_count.dropped` on Intake API v2, leave unchanged in ES.

implements elastic#1241
roncohen pushed a commit to roncohen/apm-server that referenced this issue Oct 15, 2018
Require `transaction.span_count.started` on Intake API v2, not indexed in ES. 
Rename `transaction.span_count.dropped.total` to `transaction.span_count.dropped` on Intake API v2, leave unchanged in ES.

implements elastic#1241
roncohen pushed a commit to roncohen/apm-server that referenced this issue Oct 15, 2018
Require `transaction.span_count.started` on Intake API v2, not indexed in ES. 
Rename `transaction.span_count.dropped.total` to `transaction.span_count.dropped` on Intake API v2, leave unchanged in ES.

implements elastic#1241
roncohen pushed a commit to roncohen/apm-server that referenced this issue Oct 15, 2018
Require `transaction.span_count.started` on Intake API v2, not indexed in ES. 
Rename `transaction.span_count.dropped.total` to `transaction.span_count.dropped` on Intake API v2, leave unchanged in ES.

implements elastic#1241
roncohen pushed a commit to roncohen/apm-server that referenced this issue Oct 16, 2018
Require `transaction.span_count.started` on Intake API v2, not indexed in ES. 
Rename `transaction.span_count.dropped.total` to `transaction.span_count.dropped` on Intake API v2, leave unchanged in ES.

implements elastic#1241
roncohen pushed a commit that referenced this issue Oct 16, 2018
Require `transaction.span_count.started` on Intake API v2, not indexed in ES. 
Rename `transaction.span_count.dropped.total` to `transaction.span_count.dropped` on Intake API v2, leave unchanged in ES.

implements #1241
@alvarolobato alvarolobato added this to the 6.5 milestone Oct 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests