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

Report spans independently of transactions #188

Merged
merged 7 commits into from
Aug 31, 2018
Merged

Report spans independently of transactions #188

merged 7 commits into from
Aug 31, 2018

Conversation

axw
Copy link
Member

@axw axw commented Aug 31, 2018

Spans are no longer embedded in transactions,
and they are reported independently. A span
may outlive a transaction, so there is no
truncation, and we must take care never to
store references to transactions in a span.

axw added 3 commits August 31, 2018 09:58
Remove Transaction.Spans. Spans now stand on their own.

Also, drop the payload structures, which are no longer
used with v2.
@axw axw mentioned this pull request Aug 31, 2018
10 tasks
axw added 2 commits August 31, 2018 10:51
Spans are no longer embedded in transactions,
and they are reported independently. A span
may outlive a transaction, so there is no
truncation, and we must take care never to
store references to transactions in a span.
axw added 2 commits August 31, 2018 11:00
Another faulty assumption. We can't expect anything
about the contents of the initial request, as the
flush could come before any individual transaction
is picked up.
When closing the tracer, make sure the iochan.Reader
is closed first. This fixes a race in TestTracerRequestSize,
which ignores the ctx's cancellation.
@codecov-io
Copy link

Codecov Report

Merging #188 into v2 will decrease coverage by 0.07%.
The diff coverage is 86.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##               v2     #188      +/-   ##
==========================================
- Coverage   80.25%   80.17%   -0.08%     
==========================================
  Files          87       87              
  Lines        5166     5110      -56     
==========================================
- Hits         4146     4097      -49     
- Misses        815      817       +2     
+ Partials      205      196       -9
Impacted Files Coverage Δ
transaction.go 94.2% <ø> (-0.47%) ⬇️
apmtest/withtransaction.go 83.33% <100%> (ø) ⬆️
builtin_metrics.go 85.1% <100%> (+0.32%) ⬆️
transport/transporttest/recorder.go 73.33% <100%> (+1.4%) ⬆️
model/marshal_fastjson.go 87.27% <100%> (+0.78%) ⬆️
modelwriter.go 93.9% <100%> (+0.07%) ⬆️
internal/apmschema/schema.go 76.47% <100%> (+1.47%) ⬆️
tracer_stats.go 100% <100%> (ø) ⬆️
span.go 89.55% <61.53%> (-7.42%) ⬇️
tracer.go 82.55% <83.33%> (-0.52%) ⬇️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7702c6...438ac16. Read the comment docs.

@axw axw merged commit e5ff417 into elastic:v2 Aug 31, 2018
@axw axw deleted the v2-spans branch August 31, 2018 03:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants