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

feat(flags): more flags production-readiness #26998

Merged
merged 34 commits into from
Jan 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
446fc72
tweaks
dmarticus Dec 16, 2024
e51fde4
Merge branch 'master' into feat/flags-to-prod
dmarticus Dec 17, 2024
1a78aee
nothing much
dmarticus Dec 17, 2024
f45d6f6
Merge branch 'master' into feat/flags-to-prod
dmarticus Dec 17, 2024
22c3d49
progress is happening
dmarticus Dec 18, 2024
e41f4fe
Merge branch 'master' into feat/flags-to-prod
dmarticus Dec 28, 2024
14c29bf
not sure what this change was
dmarticus Jan 2, 2025
107c97f
Merge branch 'master' into feat/flags-to-prod
dmarticus Jan 13, 2025
8b0d057
Merge branch 'master' into feat/flags-to-prod
dmarticus Jan 15, 2025
8e0c426
Merge branch 'master' into feat/flags-to-prod
dmarticus Jan 16, 2025
48a762c
Merge branch 'feat/flags-to-prod' of github.com:PostHog/posthog into …
dmarticus Jan 16, 2025
055aae8
Merge branch 'master' into feat/flags-to-prod
dmarticus Jan 16, 2025
b280fa3
leaving this broken, will come back to it
dmarticus Jan 16, 2025
9c5de0d
Merge branch 'master' into feat/flags-to-prod
dmarticus Jan 17, 2025
07297d4
Merge branch 'master' into feat/flags-to-prod
dmarticus Jan 17, 2025
86d2b1b
more refactors
dmarticus Jan 17, 2025
eb4c1a9
linting and shearing
dmarticus Jan 17, 2025
1afa5a8
docker stuff
dmarticus Jan 17, 2025
77c8112
Merge branch 'master' into feat/flags-to-prod
dmarticus Jan 17, 2025
cfcddbf
clean up the docker compose
dmarticus Jan 17, 2025
16c1f18
handle date property matching
dmarticus Jan 18, 2025
920479d
yeah that
dmarticus Jan 18, 2025
3af7da0
actually resolving conflicts
dmarticus Jan 23, 2025
ecd72e1
Merge branch 'master' into feat/flags-to-prod
dmarticus Jan 23, 2025
fb8f4e1
Merge branch 'master' into feat/flags-to-prod
dmarticus Jan 23, 2025
28851a5
Merge branch 'master' into feat/flags-to-prod
dmarticus Jan 24, 2025
dc690eb
Merge branch 'master' into feat/flags-to-prod
dmarticus Jan 24, 2025
4fca396
this is working now
dmarticus Jan 24, 2025
e9b1fc1
Merge branch 'master' into feat/flags-to-prod
dmarticus Jan 24, 2025
1448733
small docker change
dmarticus Jan 24, 2025
27386ed
idk
dmarticus Jan 24, 2025
d677c5d
don't volume mount bc we're downloading it each time
dmarticus Jan 24, 2025
f3da199
YES
dmarticus Jan 24, 2025
44c94d4
don't include these logs
dmarticus Jan 24, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion .github/workflows/rust-docker-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ jobs:
dockerfile: ./rust/Dockerfile
- image: cymbal
dockerfile: ./rust/Dockerfile
- image: feature-flags
dockerfile: ./rust/Dockerfile
- image: batch-import-worker
dockerfile: ./rust/Dockerfile
runs-on: depot-ubuntu-22.04-4
Expand All @@ -52,7 +54,7 @@ jobs:
hook-worker_digest: ${{ steps.digest.outputs.hook-worker_digest }}
hook-migrator_digest: ${{ steps.digest.outputs.hook-migrator_digest }}
cymbal_digest: ${{ steps.digest.outputs.cymbal_digest }}

feature-flags_digest: ${{ steps.digest.outputs.feature-flags_digest }}
defaults:
run:
working-directory: rust
Expand Down Expand Up @@ -145,6 +147,10 @@ jobs:
values:
image:
sha: '${{ needs.build.outputs.property-defs-rs_digest }}'
- release: feature-flags
values:
image:
sha: '${{ needs.build.outputs.feature-flags_digest }}'
- release: batch-import-worker
values:
image:
Expand Down
27 changes: 27 additions & 0 deletions docker-compose.base.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ services:
path /capture*
}

@flags {
path /flags
path /flags*
}
Comment on lines +32 to +35
Copy link
Contributor Author

@dmarticus dmarticus Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this puts the flags service behind the reverse proxy that's handling our other rust services.


handle @capture {
reverse_proxy capture:3000
}
Expand All @@ -37,6 +42,10 @@ services:
reverse_proxy replay-capture:3000
}

handle @flags {
reverse_proxy feature-flags:3001
}
Comment on lines +45 to +47
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we run on port 3001, and then route it to port 8000


handle {
reverse_proxy web:8000
}
Expand Down Expand Up @@ -197,6 +206,24 @@ services:
SKIP_READS: 'false'
FILTER_MODE: 'opt-out'

feature-flags:
image: ghcr.io/posthog/posthog/feature-flags:master
build:
context: rust/
args:
BIN: feature-flags
restart: on-failure
volumes:
- ./share:/share
environment:
WRITE_DATABASE_URL: 'postgres://posthog:posthog@db:5432/posthog'
READ_DATABASE_URL: 'postgres://posthog:posthog@db:5432/posthog'
MAXMIND_DB_PATH: '/share/GeoLite2-City.mmdb'
REDIS_URL: 'redis://redis:6379/'
ADDRESS: '0.0.0.0:3001'
SKIP_WRITES: 'false'
SKIP_READS: 'false'

plugins:
command: ./bin/plugin-server --no-restart-loop
restart: on-failure
Expand Down
9 changes: 9 additions & 0 deletions docker-compose.dev-full.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ services:
depends_on:
- replay-capture
- capture
- feature-flags
- web
db:
extends:
Expand Down Expand Up @@ -150,6 +151,14 @@ services:
depends_on:
- kafka

feature-flags:
extends:
file: docker-compose.base.yml
service: feature-flags
depends_on:
- redis
- db

plugins:
extends:
file: docker-compose.base.yml
Expand Down
9 changes: 9 additions & 0 deletions docker-compose.dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ services:
depends_on:
- replay-capture
- capture
- feature-flags
extra_hosts:
- 'web:host-gateway'
db:
Expand Down Expand Up @@ -153,6 +154,14 @@ services:
depends_on:
- kafka

feature-flags:
extends:
file: docker-compose.base.yml
service: feature-flags
depends_on:
- redis
- db

livestream:
extends:
file: docker-compose.base.yml
Expand Down
3 changes: 2 additions & 1 deletion posthog/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
"s",
"static",
"_health",
"flags",
]

if DEBUG:
Expand All @@ -66,7 +67,7 @@
"samesite": "Strict",
}

cookie_api_paths_to_ignore = {"e", "s", "capture", "batch", "decide", "api", "track"}
cookie_api_paths_to_ignore = {"e", "s", "capture", "batch", "decide", "api", "track", "flags"}


class AllowIPMiddleware:
Expand Down
3 changes: 3 additions & 0 deletions rust/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions rust/feature-flags/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ anyhow = { workspace = true }
async-trait = { workspace = true }
axum = { workspace = true }
axum-client-ip = { workspace = true }
chrono = { workspace = true }
envconfig = { workspace = true }
tokio = { workspace = true }
tracing = { workspace = true }
Expand All @@ -29,6 +30,7 @@ serde-pickle = { version = "1.1.1"}
sha1 = "0.10.6"
regex = "1.10.4"
maxminddb = "0.17"
metrics = { workspace = true }
sqlx = { workspace = true }
uuid = { workspace = true }
base64.workspace = true
Expand All @@ -38,6 +40,7 @@ strum = { version = "0.26", features = ["derive"] }
health = { path = "../common/health" }
common-metrics = { path = "../common/metrics" }
tower = { workspace = true }
tower-http = { workspace = true }
derive_builder = "0.20.1"
petgraph = "0.6.5"
moka = { workspace = true }
Expand Down
10 changes: 8 additions & 2 deletions rust/feature-flags/src/api/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use std::net::IpAddr;

use crate::{
api::errors::FlagError,
api::handler::{process_request, FlagsQueryParams, RequestContext},
api::types::FlagsResponse,
api::request_handler::{process_request, FlagsQueryParams, RequestContext},
api::types::{FlagsOptionsResponse, FlagsResponse, FlagsResponseCode},
router,
};
// TODO: stream this instead
Expand Down Expand Up @@ -53,6 +53,12 @@ pub async fn flags(
Ok(Json(process_request(context).await?))
}

pub async fn options() -> Result<Json<FlagsOptionsResponse>, FlagError> {
Ok(Json(FlagsOptionsResponse {
status: FlagsResponseCode::Ok,
}))
}
Comment on lines +56 to +60
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


fn record_request_metadata(
headers: &HeaderMap,
method: &Method,
Expand Down
5 changes: 4 additions & 1 deletion rust/feature-flags/src/api/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ pub enum ClientFacingError {
Unauthorized(String),
#[error("Rate limited")]
RateLimited,
#[error("billing limit reached")]
BillingLimit,
Comment on lines +16 to +17
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will implement the quota limiting in another PR; I started on it and then it got kinda beefy. Leaving this in for now though since it's harmless to add the error type, the PR will land shortly anyway.

#[error("Service unavailable")]
ServiceUnavailable,
}
Expand Down Expand Up @@ -67,6 +69,7 @@ impl IntoResponse for FlagError {
FlagError::ClientFacing(err) => match err {
ClientFacingError::BadRequest(msg) => (StatusCode::BAD_REQUEST, msg),
ClientFacingError::Unauthorized(msg) => (StatusCode::UNAUTHORIZED, msg),
ClientFacingError::BillingLimit => (StatusCode::PAYMENT_REQUIRED, "Billing limit reached. Please upgrade your plan.".to_string()),
ClientFacingError::RateLimited => (StatusCode::TOO_MANY_REQUESTS, "Rate limit exceeded. Please reduce your request frequency and try again later.".to_string()),
ClientFacingError::ServiceUnavailable => (StatusCode::SERVICE_UNAVAILABLE, "Service is currently unavailable. Please try again later.".to_string()),
},
Expand Down Expand Up @@ -176,7 +179,7 @@ impl From<CustomRedisError> for FlagError {
match e {
CustomRedisError::NotFound => FlagError::TokenValidationError,
CustomRedisError::PickleError(e) => {
tracing::error!("failed to fetch data: {}", e);
tracing::error!("failed to fetch data from redis: {}", e);
FlagError::RedisDataParsingError
}
CustomRedisError::Timeout(_) => FlagError::TimeoutError,
Expand Down
3 changes: 2 additions & 1 deletion rust/feature-flags/src/api/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
pub mod endpoint;
pub mod errors;
pub mod handler;
pub mod request_handler;
pub mod test_endpoint;
pub mod types;
Loading
Loading