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

module/apmgrpc: fix trace propagation vs. sampling #602

Merged
merged 2 commits into from
Jul 31, 2019

Conversation

axw
Copy link
Member

@axw axw commented Jul 29, 2019

Fix a bug in the client interceptor which would break
trace propagation when facing dropped spans or non-sampled
transactions. In these cases we should propagate the
transaction's trace context, rather than sending nothing
in the outgoing metadata which would restart tracing.

Fixes #601

@axw axw force-pushed the apmgrpc-dropped-span-tracecontext branch from 0a6a5ea to 8166ff9 Compare July 29, 2019 05:24
axw added 2 commits July 30, 2019 12:11
Add WithTransaction and WithTransactionOptions
methods to RecordingTracer, and update the
top-level functions to be implemented in terms
of them. This enables calling those functions
with a customised tracer.
Fix a bug in the client interceptor which would break
trace propagation when facing dropped spans or non-sampled
transactions. In these cases we should propagate the
transaction's trace context, rather than sending nothing
in the outgoing metadata which would restart tracing.
@axw axw force-pushed the apmgrpc-dropped-span-tracecontext branch from 8166ff9 to faf2be7 Compare July 30, 2019 04:13
@axw
Copy link
Member Author

axw commented Jul 30, 2019

jenkins run the tests please

1 similar comment
@axw
Copy link
Member Author

axw commented Jul 31, 2019

jenkins run the tests please

@codecov-io
Copy link

codecov-io commented Jul 31, 2019

Codecov Report

Merging #602 into master will increase coverage by 0.04%.
The diff coverage is 96.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #602      +/-   ##
==========================================
+ Coverage   87.76%   87.81%   +0.04%     
==========================================
  Files         122      122              
  Lines        7456     7470      +14     
==========================================
+ Hits         6544     6560      +16     
+ Misses        811      810       -1     
+ Partials      101      100       -1
Impacted Files Coverage Δ
apmtest/withtransaction.go 100% <100%> (+7.14%) ⬆️
module/apmgrpc/client.go 97.29% <100%> (+1%) ⬆️
apmtest/recorder.go 90.47% <91.66%> (+1.58%) ⬆️
tracer.go 89.69% <0%> (+0.32%) ⬆️

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 f5dca77...faf2be7. Read the comment docs.

@axw axw merged commit 7796d5a into elastic:master Jul 31, 2019
@axw axw deleted the apmgrpc-dropped-span-tracecontext branch July 31, 2019 01:38
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.

module/apmgrpc: traces are broken by sampling
2 participants