search

spandex-project / spandex

A platform agnostic tracing library
MIT License
333 stars 53 forks source link

Parse datadog distributed headers properly #51

Closed aspett closed 6 years ago

aspett commented 6 years ago

I made a boo-boo in the original distributed tracing; datadog expects the trace ID and parent span ID to be integers (i.e. not in string format). As headers are strings across the network, we need to parse 'em back out to integers. Have tested this now and am seeing lovely distributed traces.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.1%) to 80.488% when pulling 32036f40a2cad6e808b5d0e7c1401ed2a2efe02a on aspett:fix-distributed-header-parsing into 4e744334747e2086c9423b4efc69fdcf81967230 on zachdaniel:master.

aspett commented 6 years ago 
Finished in 0.8 seconds
35 tests, 0 failures
Randomized with seed 70464
Successfully uploaded the report to 'https://coveralls.io'.
The command "mix coveralls.travis" exited with 0.
after_script.1
1.00s$ mix deps.get --only docs
Resolving Hex dependencies...
Dependency resolution completed:
Unchanged:
  mime 1.1.0
  msgpax 1.1.0
  optimal 0.3.2
  plug 1.3.5
All dependencies up to date
after_script.2
8.00s$ MIX_ENV=docs mix inch.report
==> optimal
Compiling 5 files (.ex)
Generated optimal app
==> mime
Compiling 1 file (.ex)
warning: String.strip/1 is deprecated, use String.trim/1
  lib/mime.ex:28
Generated mime app
==> plug
Compiling 44 files (.ex)
warning: Atom.to_char_list/1 is deprecated, use Atom.to_charlist/1
  lib/plug/builder.ex:186
warning: String.rstrip/1 is deprecated, use String.trim_trailing/1
  lib/plug/templates/debugger.html.eex:635
warning: Kernel.to_char_list/1 is deprecated, use Kernel.to_charlist/1
  lib/plug/adapters/cowboy.ex:220
warning: Kernel.to_char_list/1 is deprecated, use Kernel.to_charlist/1
  lib/plug/adapters/cowboy.ex:238
Generated plug app
==> msgpax
Compiling 7 files (.ex)
Generated msgpax app
==> spandex
Compiling 11 files (.ex)
Generated spandex app
** (Mix) The task "inch.report" could not be found
Done. Your build exited with 1.
zachdaniel commented 6 years ago

Weird that inch caused an issue, but I think it can safely be ignored for now. I'll deal w/ that later.

zachdaniel commented 6 years ago

Left a comment, will merge and release once addressed.

zachdaniel commented 6 years ago

Also, while I've got you, curious if you are using any of the function decorators? I removed them due to language version issues and compilation issues. Just an FYI that you'd have to replace them to upgrade. Writing your own decorator would be relatively easy also.

aspett commented 6 years ago

Yeah I rewrote them the day after you released 1.4.0. I'll update that guard now

zachdaniel commented 6 years ago

@aspett nice. Sorry for all the thrash, working hard to make sure this library is stable and extensible, and all of the extra features that existed like Spandex.Task and Spandex.Logger and the function decorators just ended up making it very difficult to maintain.

aspett commented 6 years ago

Updated

aspett commented 6 years ago

No problem. I pulled out the bits and pieces we still needed for the meantime (ecto logger) and got that decorator going. I totally understand why you'd take the decorator out; it can be very finnicky and doesn't play well in some scenarios. The Tracer pattern is a nice improvement.

asummers commented 6 years ago

It also hides Dialyzer errors, which no one wants!