sdk-core icon indicating copy to clipboard operation
sdk-core copied to clipboard

[Feature Request] Managing dependencies for the Rust SDK

Open djc opened this issue 1 year ago • 3 comments

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 -- #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 avatar Feb 16 '24 15:02 djc

@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.

Sushisource avatar Feb 16 '24 17:02 Sushisource

mockall (can definitely be done, not necessarily easy)

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

djc avatar Feb 16 '24 21:02 djc

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: https://github.com/temporalio/sdk-core/blob/a7644056a651e5c3e828ff2537df1e42d9cfd904/core/src/replay/mod.rs#L74

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

Sushisource avatar Feb 16 '24 21:02 Sushisource