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

Add debug-wrapping backend #44

Closed

Conversation

ELLIOTTCABLE
Copy link
Collaborator

Was initially creating this as a utility for a test-harness; but realized it's a bit of a cleaner approach to debugging that's external to any particular collector.

For the moment, I've left the existing debug machinery in both the clients; but some of it may be redundant now?

Additionally, this adds a "noop" collector, which when used with the Debug_collector functor, results in an implementation that just prints to stdout.

Copy link
Member

@mattjbray mattjbray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice to me, but I think this is more @c-cube's area. I'll let him comment.

@c-cube
Copy link
Member

c-cube commented Nov 17, 2023

Sorry I didn't come around to reviewing that earlier! I think it can be useful. @ELLIOTTCABLE would you rebase it?

* upstream/master: (40 commits)
  chore: add upper bound on pbrt
  fix: regenerate code with a non-pinned ocaml-protoc
  debug git diff
  chore: use protoc 2.4 in CI
  format
  ocamlformat
  add missing proto directory
  refactor tests to avoid circular dependencies
  no open
  move protobuf code to opentelemetry.proto
  trace-collector: Use Trace_core instead of Trace for OCaml v5
  tests: Add every test-dep to every package
  chore: Prepare for v0.6
  chore: Add myself to maintainers
  chore: update CHANGES
  dune fmt
  trace-collector: Pass user-data to OTel metrics
  deps: Update ocaml-trace dep to 0.4
  trace-collector: Support for floats, etc from trace
  (- fix) Use new TLS-module naming from ac.0.1.0
  ...
@ELLIOTTCABLE
Copy link
Collaborator Author

Ah, sorry, this was already merged into master as 03f6f69 / as part of #34. 🙈

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.

3 participants