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

[Feature Request] Managing dependencies for the Rust SDK #687

Open
djc opened this issue Feb 16, 2024 · 3 comments
Open

[Feature Request] Managing dependencies for the Rust SDK #687

djc opened this issue Feb 16, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@djc
Copy link
Contributor

djc commented Feb 16, 2024

Is your feature request related to a problem? Please describe.

It would be great if the Rust SDK dragged in as few dependencies as possible. Dependencies increase build times throughout downstream projects. While some dependencies provide invaluable functionalities, other times there might be a way to do the same work without taking on an additional dependency.

Our monorepo contains about 90k lines of Rust code and pulls in some 800 crates. Adding the temporal-sdk stack of crates added a bunch more. It would be nice to minimize the impact on build times for our repo (and I assume it might also help for your build times in lang SDKs).

Describe the solution you'd like

Make good trade-offs about which dependencies provide enough value vs being a net drag on compile time.

Additional context

Here are indirect dependencies that were added to our workspace Cargo.lock when adding temporal-sdk:

  • arc-swap 1.6.0
  • backoff 0.4.0
  • convert_case 0.4.0 (via derive_more -- convert_case 0.6.0 was already in our transitive dependencies)
  • crossbeam 0.8.4 (but its constituent dependencies were already in our transitive dependencies)
  • rustc_version (via derive_more)
  • enum-iterator 1.5.0
  • enum-iterator-derive 1.3.0
  • enum_dispatch 0.3.12
  • futures-retry 0.6.0
  • lru 0.11.1 [vs lru-cache?]
  • matchers 0.1.0 (enabled via tracing-subscriber)
    • regex-automata 0.1.10 (0.4.5 was already in our transitive dependencies)
  • mockall 0.11.4
    • downcast 0.11.0
    • fragile 2.0.0
    • mockall_derive 0.11.4
    • predicates 2.1.5
      • difflib 0.4.0
      • float-cmp 0.9.0
      • normalize-line-endings 0.3.0
      • predicates-core 1.0.6
    • predicates-tree 1.0.9
      • termtree 0.4.1
  • opentelemetry-prometheus 0.14.1
    • prometheus 0.13.3
    • protobuf 2.28.0
  • ringbuf 0.3.3
  • rustfsm 0.1.0
    • rustfsm-procmacro 0.1.0
    • rustfsm_trait 0.1.0
  • siphasher 1.0.0
  • slotmap 1.0.7
  • tonic-build 0.9.2 (0.11.0 was already in our transitive dependencies)
    • prettyplease 0.1.25 (0.2.16 was already in our transitive dependencies)
    • prost-build 0.11.9 (0.12.3 was already in our transitive dependencies)
    • prost-wkt 0.4.2
      • prost-wkt-build 0.4.2
      • prost-wkt-types 0.4.2
      • typetag 0.2.15
        • erased-serde 0.4.2
        • inventory 0.3.15
        • typetag-impl 0.2.15
    • protox 0.3.5 (0.6.0 was already in our transitive dependencies)
      • miette 5.10.0 (7.0.0 was already in our transitive dependencies)
        • miette-derive 5.10.0

From this, things that I'd like to see improved:

  • tonic & prost bump -- [Feature Request] Update to tonic 0.10 #626, I'm pushing on the opentelemetry side
  • It would be good if mockall could be constrained to dev-dependency only, if that? To the extent it is used in library code, maybe it can be replaced with something simpler?
  • I would like to make the opentelemetry-prometheus crate optional by making telemetry optional via Cargo features
    • This should probably also get rid of some of the tracing-subscriber features that pull in matchers
  • I personally prefer to avoid third-party convenience macros like derive_more, enum-iterator and enum-dispatch, IMO replacing this with hand-written implementations might sometimes require more code but that code is usually quite stable and low on maintenance (and easier to understand than the opaqueness of code generated by a third-party macro)
  • IIRC there's a sip hasher in the standard library, so I wonder why the separate crate is needed?

I am open to putting some time into this as we really prefer to limit build times in our work environment.

@djc djc added the enhancement New feature or request label Feb 16, 2024
@Sushisource
Copy link
Member

@djc I'm happy to accept pulls for removing mockall (can definitely be done, not necessarily easy) and otel (probably pretty easy to make optional).

The separate hasher crate is used for because the one in stdlib is deprecated.

The convenience macros I find pretty useful. The savings would have to be quite significant for me to want to remove them, and I have no idea what those savings would be without seeing. I can't imagine they're causing a huge amount of bloat.

And yes the tonic bump is just waiting on otel.

@djc
Copy link
Contributor Author

djc commented Feb 16, 2024

mockall (can definitely be done, not necessarily easy)

Can you briefly explain what the goal/use case of the MockManualWorkerClient is?

@Sushisource
Copy link
Member

mockall (can definitely be done, not necessarily easy)

Can you briefly explain what the goal/use case of the MockManualWorkerClient is?

For testing of course. But, in production it's used to enable replay workers:

let mut client = if let Some(c) = self.client_override {

This could be un-done and replaced with something else, but it's not trivial.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants