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

Move binary propagator and base64 format to contrib. #343

Merged
merged 5 commits into from
Nov 5, 2020

Conversation

TommyCpp
Copy link
Contributor

@TommyCpp TommyCpp commented Nov 5, 2020

  • Moved them as they are not part of the spec for now and should not be in main crate when GA. (Moved files and two features from main crate to contrib)
  • Moved binary propagator and base64 format into a binary module and reexport the public types.
  • Also restructure the contrib into two submodule, one for tracing and one for metric.

closes #336

* Moved them as they are not part of the spec for now and should not be in main crate when GA.

* Also restructure the contrib into two submodule, one for tracing and one for metric.
@TommyCpp TommyCpp requested a review from a team November 5, 2020 03:03
@codecov
Copy link

codecov bot commented Nov 5, 2020

Codecov Report

Merging #343 into master will decrease coverage by 0.83%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #343      +/-   ##
==========================================
- Coverage   54.03%   53.20%   -0.84%     
==========================================
  Files          71       69       -2     
  Lines        5879     5770     -109     
==========================================
- Hits         3177     3070     -107     
+ Misses       2702     2700       -2     
Impacted Files Coverage Δ
...ntelemetry/src/sdk/metrics/aggregators/ddsketch.rs 78.40% <0.00%> (-0.38%) ⬇️

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 ccf4599...3e7d24e. Read the comment docs.

@@ -17,7 +17,7 @@ fn bar() {
}

fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>> {
let (tracer, _uninstall) = opentelemetry_contrib::datadog::new_pipeline()
let (tracer, _uninstall) = opentelemetry_contrib::trace::exporter::datadog::new_pipeline()
Copy link
Member

Choose a reason for hiding this comment

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

small nit: may want to use this as above? calling a function 4 namespaces deep currently 😂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah definitely should have imported them at the top

@jtescher jtescher merged commit 3e1948e into open-telemetry:master Nov 5, 2020
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.

Consider moving base64 and binary propagators to contrib
2 participants