Skip to content

Decouple internal crate config types from amp-config #1790

@LNSD

Description

@LNSD

Problem

Internal crates (common, worker-core, monitoring) define configuration types that are directly imported and used by amp-config. This creates two problems:

  1. Internal crates must not depend on schemars — JSON schema generation is an ampd config concern, not an internal crate concern. Currently, schemars propagates into internal crates via the feature chain: amp-config[schemars]amp-worker-core/schemarscommon/schemars and amp-config[schemars]monitoring/schemars.

  2. Application-level defaults should be defined at the application level — Internal crates should define their own defaults independently. The amp-config crate should be able to define different defaults at the application-specific config level.

Approach

The Rust-idiomatic way to achieve this decoupling:

  • Duplicate the config types in amp-config. Define the defaults there, even if they are the same as those in the internal crate.
  • Do not use dependency types as config types. Always declare new types that wrap 3rd-party code in the interfaces of your code (App or API). A change in the dependency can cause a catastrophic effect in your app. After upgrading the dependency version, you should control the public API.
  • In internal crates, declare function params as config: impl Into<T> (where T is the crate's own config type).
  • In amp-config, implement impl From<AmpConfigT> for CrateSpecificT.

Additional requirements

  • Improve amp-config fields' documentation (rustdocs). Make it more customer-facing, more thorough. For example, documenting the Compression enum possible values and what is the purpose of that config param.

Affected Crates

Each crate below should be decoupled in its own PR, one crate at a time.

1. monitoring (crate: monitoring)

Types used by amp-config:

  • OpenTelemetryConfig — imported in config_file.rs and lib.rs

schemars coupling:

  • monitoring/Cargo.toml: schemars = { workspace = true, optional = true }
  • monitoring/src/config.rs: #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] on OpenTelemetryConfig
  • Feature activated by: amp-config[schemars]monitoring/schemars

Decoupling work:

  • Create a new OpenTelemetryConfig type in amp-config with serde + schemars derives
  • Implement impl From<amp_config::OpenTelemetryConfig> for monitoring::config::OpenTelemetryConfig
  • Remove schemars optional dependency from monitoring
  • Remove the cfg_attr(feature = "schemars", ...) annotations from monitoring::config

2. worker-core (crate: amp-worker-core)

Types used by amp-config:

  • ParquetConfig — imported in config_file.rs (for ConfigFile.writer) and lib.rs (for Config.parquet)
  • ConfigDuration — imported in config_file.rs (for ConfigFile.poll_interval_secs) and lib.rs (for WorkerEventsConfig.progress_interval)

schemars coupling:

  • worker-core/Cargo.toml: schemars = ["dep:schemars", "common/schemars"] — also propagates to common!
  • worker-core/src/config.rs: schemars annotations on ParquetConfig, ConfigDuration, CollectorConfig, CompactorConfig, CompactionAlgorithmConfig, SizeLimitConfig
  • Feature activated by: amp-config[schemars]amp-worker-core/schemarscommon/schemars

Decoupling work:

  • Create duplicate config types in amp-config: ParquetConfig, ConfigDuration, CollectorConfig, CompactorConfig, CompactionAlgorithmConfig, SizeLimitConfig
  • The amp-config versions should have serde + schemars derives and customer-facing documentation
  • Implement From conversions from amp-config types to worker-core types
  • Make worker-core accept impl Into<worker_core::ParquetConfig> (and similar) where config is consumed
  • Remove schemars optional dependency from worker-core
  • Remove the cfg_attr(feature = "schemars", ...) annotations from worker-core::config
  • Do NOT use datafusion::parquet::basic::Compression directly in amp-config's config type — wrap it

3. common (crate: common)

Types used by amp-config (transitively via worker-core):

  • Overflow — used in worker-core::SizeLimitConfig.overflow field

schemars coupling:

  • common/Cargo.toml: schemars = { workspace = true, optional = true }
  • common/src/metadata/size.rs: manual impl schemars::JsonSchema for Overflow
  • Feature activated by: amp-worker-core[schemars]common/schemars

Decoupling work:

  • Remove the schemars optional dependency from common
  • Remove the #[cfg(feature = "schemars")] impl schemars::JsonSchema for Overflow from common/src/metadata/size.rs
  • The Overflow schema will be defined in the amp-config version of SizeLimitConfig instead (part of worker-core decoupling PR)

Note: This work may be absorbed into the worker-core PR since the only schemars usage in common is for Overflow, which is transitively used through worker-core. Alternatively it can be a standalone PR that just removes the schemars dep from common after worker-core no longer propagates the feature.

Dependency Graph (Current)

amp-config ──depends on──► amp-worker-core ──depends on──► common
     │                          │
     │                          └─ schemars feature propagates to common
     │
     └──depends on──► monitoring

amp-config[schemars] feature chain:

amp-config[schemars]
├── dep:schemars
├── amp-worker-core/schemars
│   ├── dep:schemars
│   └── common/schemars
│       └── dep:schemars
└── monitoring/schemars
    └── dep:schemars

Dependency Graph (Target)

amp-config ──depends on──► amp-worker-core ──depends on──► common
     │                     (no schemars)                   (no schemars)
     │
     └──depends on──► monitoring
                      (no schemars)

amp-config[schemars] feature chain (target):

amp-config[schemars]
└── dep:schemars   (only amp-config itself needs schemars)

Suggested PR Order

  1. PR 1: monitoring — Smallest, simplest decoupling (1 type: OpenTelemetryConfig)
  2. PR 2: worker-core + common — Larger, handles ParquetConfig, ConfigDuration, and related types. Includes removing schemars from common since Overflow schemars impl is only needed through this chain.

Each PR should be independently reviewable and mergeable.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions