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

Reduce dependency on futures crate #684

Merged
merged 1 commit into from
Dec 29, 2021

Conversation

robjtede
Copy link
Contributor

@robjtede robjtede commented Dec 29, 2021

Size of dep graph:

before after
opentelemetry 47 38
opentelemetry-contrib 122 113
opentelemetry-datadog 136 130
opentelemetry-http 53 44
opentelemetry-jaeger 129 120
opentelemetry-otlp 140 139
opentelemetry-prometheus 52 43
opentelemetry-aws 137 128
opentelemetry-semantic-conventions 48 39
opentelemetry-stackdriver 180 180
opentelemetry-zipkin 218 211
opentelemetry-zpages 149 149

Only opentelemetry-otlp could not have it's futures dep removed (yet). It is blocked on the release of tikv/grpc-rs#554 in order for the codegen to reference std's Future.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Dec 29, 2021

CLA Signed

The committers are authorized under a signed CLA.

@djc
Copy link
Contributor

djc commented Dec 29, 2021

Nice, would be happy to merge this.

@robjtede robjtede force-pushed the fut-dep-red branch 2 times, most recently from 1021477 to 9c45f19 Compare December 29, 2021 06:40
@robjtede robjtede marked this pull request as ready for review December 29, 2021 06:44
@robjtede robjtede requested a review from a team as a code owner December 29, 2021 06:44
@djc
Copy link
Contributor

djc commented Dec 29, 2021

How do you measure size of individual crate's dep graphs?

@robjtede
Copy link
Contributor Author

robjtede commented Dec 29, 2021

It's not scientific but: cargo tree -e=normal -p="$p" | wc -l.

I don't know any incantation to just get a flat, dupe-less list of deps in the graph.

@robjtede
Copy link
Contributor Author

robjtede commented Dec 29, 2021

Updated with slightly more scientific: cargo tree -e=normal --prefix=none -p="$p" | grep -v '*' | wc -l | xargs

@codecov
Copy link

codecov bot commented Dec 29, 2021

Codecov Report

Merging #684 (00d2666) into main (50510e3) will not change coverage.
The diff coverage is 85.71%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #684   +/-   ##
=======================================
  Coverage   63.81%   63.81%           
=======================================
  Files          96       96           
  Lines        7725     7725           
=======================================
  Hits         4930     4930           
  Misses       2795     2795           
Impacted Files Coverage Δ
opentelemetry/src/runtime.rs 69.76% <ø> (ø)
opentelemetry/src/sdk/export/metrics/stdout.rs 0.00% <ø> (ø)
opentelemetry/src/sdk/metrics/controllers/push.rs 0.00% <0.00%> (ø)
opentelemetry/src/sdk/trace/runtime.rs 56.00% <ø> (ø)
opentelemetry/src/trace/context.rs 46.42% <ø> (ø)
opentelemetry/src/trace/mod.rs 72.88% <ø> (ø)
opentelemetry-jaeger/src/exporter/mod.rs 58.72% <100.00%> (ø)
opentelemetry/src/sdk/trace/span_processor.rs 82.01% <100.00%> (ø)

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 50510e3...00d2666. Read the comment docs.

@TommyCpp TommyCpp merged commit 482772f into open-telemetry:main Dec 29, 2021
@robjtede robjtede deleted the fut-dep-red branch December 29, 2021 17:33
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.

None yet

3 participants