census-instrumentation / opencensus-erlang

A stats collection and distributed tracing framework
https://opencensus.io
Apache License 2.0
134 stars 39 forks source link

set logger metadata when a span is put into a context #98

Closed tsloughter closed 6 years ago

tsloughter commented 6 years ago

I'm not sure that setting the metadata in oc_trace:with_child_span/2 is the right thing to do... Since ocp is in charge of what the current process's current span context is we know the log message will be correct, this is less the case when dealing with a context variable.

If we could just pass the context variable itself to the logger for metadata it would be a moot point, that would work cleanly, but we'd have to also then create a custom logger formatter and require the user set that up or else it would blow up.

codecov-io commented 6 years ago

Codecov Report

Merging #98 into master will increase coverage by 2.27%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #98      +/-   ##
==========================================
+ Coverage   78.27%   80.54%   +2.27%     
==========================================
  Files          34       34              
  Lines         695      694       -1     
==========================================
+ Hits          544      559      +15     
+ Misses        151      135      -16
Impacted Files Coverage Δ
src/oc_trace.erl 100% <100%> (+25%) :arrow_up:
src/ocp.erl 81.39% <100%> (+0.44%) :arrow_up:

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 e5c18a1...3c63732. Read the comment docs.

deadtrickster commented 6 years ago

Can we have one more metadata key to distinguish sampled/not sampled?

I go to the logs and see trace_id/span_id then go to Zipkin and span data isn't necessarily there, with small sampling rates that could be a problem.

tsloughter commented 6 years ago

Ok, added the trace options.