Skip to content

Commit

Permalink
alternative JustKnobs implementation for use in OSS and testing
Browse files Browse the repository at this point in the history
Summary:
JustKnobs are good for production but the testing lacks heavily there. There's no
good way to isolate test setup completely and it's easy to leak production configs into test setup.
Moreover the test primitives are based on `scopedConfigeratorFake` which use is discouraged and I'm not sure
how thread-safe it's usage is.

Since we need some way to set justknobs for OSS builds of our software anyway,
let's use simple implementation backed by cached_config abstraction. This way
we can create justknobs backed any config storage including JSON file on disk
for testing with no connection to production whatsoever.

This doesn't cover the usecase of having lightweight overrides for JK's for
unit tests. We can add them in separate diff in the future. (I think I can do it better than tunabled did).

The implementation was heavily inspired by tunables module that we want to replace.

Reviewed By: markbt

Differential Revision: D50422359

fbshipit-source-id: d6c6e0558877437589d6c88323f8941a16b6e4a3
  • Loading branch information
mitrandir77 authored and facebook-github-bot committed Oct 20, 2023
1 parent ecbb134 commit 78866ae
Show file tree
Hide file tree
Showing 12 changed files with 515 additions and 3 deletions.
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ members = [
"shed/hostcaps",
"shed/hostname",
"shed/justknobs_stub",
"shed/justknobs_stub/cached_config_thrift_struct",
"shed/justknobs_stub/cached_config_thrift_struct/types",
"shed/limited_async_read",
"shed/lock_ext",
"shed/memcache_stub",
Expand Down
3 changes: 2 additions & 1 deletion shed/cached_config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ mod handle;
mod oss;
mod refreshable_entities;
mod store;
mod test_source;
/// Test source for testing purposes.
pub mod test_source;
#[cfg(test)]
mod tests;

Expand Down
10 changes: 10 additions & 0 deletions shed/justknobs_stub/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,13 @@ license = "MIT OR Apache-2.0"

[dependencies]
anyhow = "=1.0.72"
arc-swap = "1.5"
cached_config = { version = "0.1.0", path = "../cached_config" }
just_knobs_struct = { version = "0.1.0", path = "cached_config_thrift_struct" }
serde = { version = "1.0.185", features = ["derive", "rc"] }
serde_json = { version = "1.0.100", features = ["float_roundtrip", "unbounded_depth"] }
slog = { version = "2.7", features = ["max_level_trace", "nested-values"] }
tokio = { version = "1.29.1", features = ["full", "test-util", "tracing"] }

[dev-dependencies]
slog_glog_fmt = { version = "0.1.0", path = "../slog_glog_fmt" }
36 changes: 36 additions & 0 deletions shed/justknobs_stub/cached_config_thrift_struct/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# @generated by autocargo

[package]
name = "just_knobs_struct"
version = "0.1.0"
authors = ["Facebook <opensource+rust-shed@fb.com>"]
edition = "2021"
readme = "../../../README.md"
repository = "https://github.com/facebookexperimental/rust-shed/"
license = "MIT OR Apache-2.0"
build = "thrift_build.rs"

[lib]
path = "thrift_lib.rs"
test = false
doctest = false

[dependencies]
anyhow = "=1.0.72"
async-trait = "0.1.71"
codegen_includer_proc_macro = { version = "0.1.0", path = "../../codegen_includer_proc_macro" }
const-cstr = "0.3.0"
fbthrift = { version = "0.0.1+unstable", git = "https://github.com/facebook/fbthrift.git", branch = "main" }
futures = { version = "0.3.28", features = ["async-await", "compat"] }
just_knobs_struct__types = { package = "just_knobs_struct_types", version = "0.1.0", path = "types" }
ref-cast = "1.0.18"
thiserror = "1.0.43"
tracing = "0.1.35"
tracing-futures = { version = "0.2.5", features = ["futures-03"] }

[build-dependencies]
thrift_compiler = { version = "0.1.0", path = "../../thrift_compiler" }

[features]
default = ["thrift_library_unittests_disabled"]
thrift_library_unittests_disabled = []
16 changes: 16 additions & 0 deletions shed/justknobs_stub/cached_config_thrift_struct/just_knobs.thrift
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under both the MIT license found in the
* LICENSE-MIT file in the root directory of this source tree and the Apache
* License, Version 2.0 found in the LICENSE-APACHE file in the root directory
* of this source tree.
*/

struct JustKnobs {
1: JustKnobInts ints;
2: JustKnobBools bools;
} (rust.exhaustive)

typedef map<string, bool> (rust.type = "HashMap") JustKnobBools
typedef map<string, i64> (rust.type = "HashMap") JustKnobInts
69 changes: 69 additions & 0 deletions shed/justknobs_stub/cached_config_thrift_struct/thrift_build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// @generated by autocargo
use std::env;
use std::fs;
use std::path::Path;

use thrift_compiler::Config;
use thrift_compiler::GenContext;

#[rustfmt::skip]
fn main() {
// Rerun if this gets rewritten.
println!("cargo:rerun-if-changed=thrift_build.rs");

let out_dir = env::var_os("OUT_DIR").expect("OUT_DIR env not provided");
let out_dir: &Path = out_dir.as_ref();
fs::write(
out_dir.join("cratemap"),
"just_knobs crate",
).expect("Failed to write cratemap");

let conf = {
let mut conf = Config::from_env(GenContext::Lib).expect("Failed to instantiate thrift_compiler::Config");

let path_from_manifest_to_base: &Path = "../../../../..".as_ref();
let cargo_manifest_dir =
env::var_os("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not provided");
let cargo_manifest_dir: &Path = cargo_manifest_dir.as_ref();
let base_path = cargo_manifest_dir
.join(path_from_manifest_to_base)
.canonicalize()
.expect("Failed to canonicalize base_path");
// TODO: replace canonicalize() with std::path::absolute() when
// https://github.com/rust-lang/rust/pull/91673 is available (~Rust 1.60)
// and remove this block.
#[cfg(windows)]
let base_path = Path::new(
base_path
.as_path()
.to_string_lossy()
.trim_start_matches(r"\\?\"),
)
.to_path_buf();

conf.base_path(base_path);

conf.types_crate("just_knobs_struct__types");

let options = "serde";
if !options.is_empty() {
conf.options(options);
}

let lib_include_srcs = vec![

];
let types_include_srcs = vec![

];
conf.lib_include_srcs(lib_include_srcs);
conf.types_include_srcs(types_include_srcs);

conf
};

let srcs: &[&str] = &[
"just_knobs.thrift"
];
conf.run(srcs).expect("Failed while running thrift compilation");
}
2 changes: 2 additions & 0 deletions shed/justknobs_stub/cached_config_thrift_struct/thrift_lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// @generated by autocargo
::codegen_includer_proc_macro::include!();
34 changes: 34 additions & 0 deletions shed/justknobs_stub/cached_config_thrift_struct/types/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# @generated by autocargo

[package]
name = "just_knobs_struct_types"
version = "0.1.0"
authors = ["Facebook <opensource+rust-shed@fb.com>"]
edition = "2021"
readme = "../../../../README.md"
repository = "https://github.com/facebookexperimental/rust-shed/"
license = "MIT OR Apache-2.0"
build = "thrift_build.rs"

[lib]
path = "thrift_lib.rs"
test = false
doctest = false

[dependencies]
anyhow = "=1.0.72"
codegen_includer_proc_macro = { version = "0.1.0", path = "../../../codegen_includer_proc_macro" }
fbthrift = { version = "0.0.1+unstable", git = "https://github.com/facebook/fbthrift.git", branch = "main" }
futures = { version = "0.3.28", features = ["async-await", "compat"] }
once_cell = "1.12"
ref-cast = "1.0.18"
serde = { version = "1.0.185", features = ["derive", "rc"] }
serde_derive = "1.0.185"
thiserror = "1.0.43"

[build-dependencies]
thrift_compiler = { version = "0.1.0", path = "../../../thrift_compiler" }

[features]
default = ["thrift_library_unittests_disabled"]
thrift_library_unittests_disabled = []
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// @generated by autocargo
use std::env;
use std::fs;
use std::path::Path;

use thrift_compiler::Config;
use thrift_compiler::GenContext;

#[rustfmt::skip]
fn main() {
// Rerun if this gets rewritten.
println!("cargo:rerun-if-changed=thrift_build.rs");

let out_dir = env::var_os("OUT_DIR").expect("OUT_DIR env not provided");
let out_dir: &Path = out_dir.as_ref();
fs::write(
out_dir.join("cratemap"),
"just_knobs crate",
).expect("Failed to write cratemap");

let conf = {
let mut conf = Config::from_env(GenContext::Types).expect("Failed to instantiate thrift_compiler::Config");

let path_from_manifest_to_base: &Path = "../../../../../..".as_ref();
let cargo_manifest_dir =
env::var_os("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not provided");
let cargo_manifest_dir: &Path = cargo_manifest_dir.as_ref();
let base_path = cargo_manifest_dir
.join(path_from_manifest_to_base)
.canonicalize()
.expect("Failed to canonicalize base_path");
// TODO: replace canonicalize() with std::path::absolute() when
// https://github.com/rust-lang/rust/pull/91673 is available (~Rust 1.60)
// and remove this block.
#[cfg(windows)]
let base_path = Path::new(
base_path
.as_path()
.to_string_lossy()
.trim_start_matches(r"\\?\"),
)
.to_path_buf();

conf.base_path(base_path);

conf.types_crate("just_knobs_struct__types");

let options = "serde";
if !options.is_empty() {
conf.options(options);
}

let lib_include_srcs = vec![

];
let types_include_srcs = vec![

];
conf.lib_include_srcs(lib_include_srcs);
conf.types_include_srcs(types_include_srcs);

conf
};

let srcs: &[&str] = &[
"../just_knobs.thrift"
];
conf.run(srcs).expect("Failed while running thrift compilation");
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// @generated by autocargo
::codegen_includer_proc_macro::include!();
Loading

0 comments on commit 78866ae

Please sign in to comment.