feat: Refactor API query params to accept array using bracket notation & add array support for more params - #732
Merged
Conversation
…n & add array support for more params
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
alexluong
reviewed
Mar 6, 2026
alexbouchardd
force-pushed
the
feat/api-query-arrays-refactor
branch
from
March 6, 2026 15:02
1fca061 to
9c8f9f7
Compare
Cover 36 adversarial tenant IDs (separators, query syntax chars, TAG structural chars, quoting, whitespace, injection attempts) to verify DIALECT 2 quoted-tag escaping in ListTenant ID filtering. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Verify that tenant_id query params are ignored on single-resource retrieve endpoints (events, attempts) and that ACL is enforced via JWT tenant at the DB level (returning 404 not 403). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…case) Comma is the default RediSearch TAG separator, so IDs containing commas get split into multiple tags at index time. Fixing requires a Redis migration to change the index SEPARATOR to \x00 (FT.DROPINDEX + recreate). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
alexluong
approved these changes
Mar 10, 2026
alexluong
added a commit
that referenced
this pull request
Mar 10, 2026
Merge main (PR #732) and update metrics handlers to use ParseArrayQueryParam and resolveTenantIDsFilter. Update OpenAPI spec filters to oneOf string/array schema with bracket notation. Update test query strings to indexed bracket format. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
alexluong
added a commit
that referenced
this pull request
Mar 10, 2026
Merge main (PR #732) and update metrics handlers to use ParseArrayQueryParam and resolveTenantIDsFilter. Update OpenAPI spec filters to oneOf string/array schema with bracket notation. Update test query strings to indexed bracket format. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
alexluong
added a commit
that referenced
this pull request
Mar 11, 2026
Replace brittle positional parameter indices with name-based filter expressions (e.g., `parameters[?@.name == 'dir']` instead of `parameters[2]`). The indices broke when #732 added new query params, causing overrides to misapply (e.g., `direction` applied to `next` instead of `dir`). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
4 tasks
alexluong
added a commit
that referenced
this pull request
Mar 13, 2026
…DK union types (#744) * fix(sdks): use JSONPath filter expressions in pagination overlay Replace brittle positional parameter indices with name-based filter expressions (e.g., `parameters[?@.name == 'dir']` instead of `parameters[2]`). The indices broke when #732 added new query params, causing overrides to misapply (e.g., `direction` applied to `next` instead of `dir`). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(sdks/go): add overlay to replace oneOf unions with plain arrays Add Go-specific Speakeasy overlay that converts all oneOf [string, array] query params to plain array types, eliminating verbose tagged union constructors in the generated Go SDK. Affects 16 params across 8 endpoints. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
alexluong
added a commit
that referenced
this pull request
Mar 13, 2026
* feat: add Metrics interface to logstore driver Split LogStore into Records + Metrics sub-interfaces. LogStore is now the combined interface so all existing consumers are unaffected. Typed responses per endpoint (EventMetricsResponse, AttemptMetricsResponse) with all fields as pointers. Stub implementations for CH, PG, and mem drivers return errNotImplemented. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: add metrics conformance tests with 300-row dataset Add a comprehensive metrics test dataset spanning January 2000 with 300 tenant-1 events (50 sparse across 5 days + 250 dense bell-curve on Jan 15) and 5 tenant-2 events for isolation. All dimension cycling produces round numbers (100/topic, 150/dest, 180 success, 120 failed, error_rate=0.4, etc). Covers all granularities (1m, 1h, 1d, 1w, 1M), dimensions, filters, and measures for both event and attempt metrics. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: implement Postgres logstore metrics queries Dynamic SQL builder for event and attempt metrics with parameterized queries, time bucketing (date_bin/date_trunc), dimension grouping, conditional aggregates (FILTER WHERE), 30s query timeout fallback, and row limit enforcement with truncation flag. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: implement ClickHouse logstore metrics queries Naive implementation using uniqExact/uniqExactIf for dedup-safe aggregation over ReplacingMergeTree without FINAL. Includes 30s query timeout fallback and 100k row limit with truncation detection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: remove unimplemented check from metrics conformance tests All three drivers (mem, pg, ch) now implement the metrics interface. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: add metrics API endpoints for events and attempts Wire up QueryEventMetrics and QueryAttemptMetrics as GET /api/v1/metrics/events and GET /api/v1/metrics/attempts with query param parsing, allowlist validation, JWT tenant scoping, and response transformation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style: gofmt metrics handlers Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: remove duplicate derefIntFromIntPtr helper Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: add missing tenant_id and attempt_number dimensions to memlogstore, add metrics OpenAPI spec memlogstore QueryEventMetrics was missing tenant_id dimension causing empty results when grouping by tenant. QueryAttemptMetrics was missing both tenant_id and attempt_number dimensions. Also adds MetricsResponse schemas and /metrics/events, /metrics/attempts paths to the OpenAPI spec. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: add metrics time-series characteristics conformance suite Split metrics drivertest into DataCorrectness (existing value assertions) and Characteristics (structural contract tests for dense bucket filling, ordering, alignment, deterministic count, zero measures, no-data ranges, and dimension × time filling). Shared dataset setup, single provisioning. Characteristics tests will fail until bucket filling is implemented. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: bump Go from 1.24 to 1.26 Enables new(expr) syntax for cleaner pointer initialization. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: add post-query bucket filling for metrics time series Sparse time-series responses caused dashboard charts to render fat bars instead of slim bars across all time slots. This adds a shared bucket filling layer (internal/logstore/bucket/) called by all 3 backends after query, producing dense responses with zero-filled gaps. - Extract TruncateTime into shared bucket package - FillEventBuckets / FillAttemptBuckets with dimension-aware filling - Update drivertest assertions for dense bucket counts - All characteristics tests now pass Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: wire missing attempt metric filters (code, manual, attempt_number) The API allowlist accepted these filters but all three query builders (pg, ch, mem) silently ignored them, causing filters like attempt_number=0 to return unfiltered results. Add WHERE clauses in all drivers and conformance tests to prevent regression. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: rename date_range to time in metrics API query params and DateRange to TimeRange in Go code Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: remove eligible_for_retry from event metrics dimensions and filters Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: adopt bracket notation array params for metrics API Merge main (PR #732) and update metrics handlers to use ParseArrayQueryParam and resolveTenantIDsFilter. Update OpenAPI spec filters to oneOf string/array schema with bracket notation. Update test query strings to indexed bracket format. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: add rate throughput measures to metrics API Add per-second throughput rate measures computed as count / bucket_duration_seconds. Events endpoint gets `rate`, attempts gets `rate`, `successful_rate`, `failed_rate`. Rate computation lives in shared driver/rate.go, called by each driver after bucket filling. Dependency measures are auto-enriched (e.g. requesting `rate` without `count` internally adds `count` for SQL but omits it from API response). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: add rate measure coverage to drivertest Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: add ClickHouse SETTINGS constraints to metrics queries Adds server-side resource limits (max_execution_time, max_rows_to_group_by) to CH metrics queries and surfaces overflow errors as 400 instead of 500. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: validate time range in metrics queries to prevent div-by-zero in rate computation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: add missing tenant_id and attempt_number dimensions to CH and PG metrics queries Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: remove dead MetricsMetadata.Granularity field No driver ever sets this field — the handler already has the request granularity. Removes the dead if-branch in buildAPIMetricsResponse. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: move tenant scoping from TenantID field to filters[tenant_id] Replace MetricsRequest.TenantID with filters[tenant_id] across all 3 drivers so tenant scoping uses the same filter mechanism as other dimensions. Handler now validates/injects filters[tenant_id] for JWT callers instead of using resolveTenantIDsFilter. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: add defensive guards and validation to metrics time bucketing - Add forward-progress guard in GenerateTimeBuckets to prevent infinite loops if AdvanceTime doesn't advance - Cap bucket count at 100k to prevent OOM; returns ErrTooManyBuckets wrapped as ErrResourceLimit (→ 400 "query too broad") - Validate granularity value ranges per spec (s:1-60, m:1-60, h:1-24, d:1-31, w:1-4, M:1-12) - Document Granularity alignment behavior for calendar units (d/w/M) - Update GenerateTimeBuckets and Fill*Buckets signatures to return error - Update all driver callers (ch, pg, mem) to propagate bucket errors - Add drivertest characteristics for too-many-buckets → ErrResourceLimit - Add handler tests for query-too-broad and granularity-out-of-range Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: add failing test for 2d granularity data loss 2d granularity silently drops events on non-slot days. SQL groups by individual day but bucket fill generates every-2-day slots, so data on days that don't align with a slot boundary is orphaned. Expected 300, actual 280 (Jan 22 and Jan 28 events lost). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: use epoch-anchored alignment for multi-value d/w/M granularities TruncateTime, CH SQL, and PG SQL all ignored g.Value for d/w/M units, always truncating to the natural period (day/week/month). This caused FillEventBuckets to silently drop data on non-slot days when Value > 1 (e.g. 2d granularity lost ~7% of events). Fix: epoch-anchored N-interval alignment for all three backends: - d: anchor to 1970-01-01, align to N-day boundaries - w: anchor to 1970-01-04 (Sunday), align to N*7-day boundaries - M: align month number to nearest multiple of N from Jan 1970 - Value=1 retains existing behavior (no functional change) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(portal): add metrics dashboard to destination overview 6-chart metrics dashboard: delivery volume (stacked bar), error rate (line, 0-100%), retries (multi-line), avg attempt number, status code breakdown, and topic breakdown. Shared timeframe selector (1h/24h/7d/30d), all charts use /metrics/attempts endpoint. Includes dataviz CSS vars for info (blue) and warning (orange) themes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(portal): replace ResponsiveContainer to fix -1px dimension warnings Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(portal): redesign metrics dashboard layout and add event count chart Restructure the metrics grid to 3 rows: events/deliveries, error breakdown (3-col), and retry pressure. Add new "Events / count" chart using attempt_number=0 filter. Support title/subtitle pattern with muted subtitles and add filters param to useMetrics hook. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(portal): add events 24h sparkline column to destinations list Add sparkline + event count to each destination row using 4h granularity with attempt_number=0 filter. Includes Sparkline component with stacked success/failed bars, empty-bar rendering, and granularity override for useMetrics hook. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(portal): clean up import order and simplify useMetrics URL building Move Loading import to top of MetricsChart.tsx. Use arrays/objects directly in useMetrics instead of serializing to strings and re-splitting. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: Fix metrics table histogram display * chore: Destination page metrics improvements * chore: Update time range picker styling * fix: Fix metric breakdown height and dimension ellipsis * chore: add metrics seed & QA scripts - seed_metrics.sh: generates realistic event→attempt chains with configurable error rates, retry chains, and time distribution - qa_metrics.sh: 11 named scenarios (healthy, failing, spike, empty, single, all-fail, all-success, recent, many-topics, many-codes, retry-heavy) with verification checklists - README documenting usage and scenarios Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(portal): exclude manual retries from event count metrics Add manual=false filter alongside attempt_number=0 to prevent manual retries (which also start at attempt_number=0) from inflating event counts in the destination metrics chart and destinations list sparkline. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: exclude manual retries from first_attempt_count measure Manual retries start a new chain at attempt_number=0, inflating first_attempt_count. Add AND NOT manual to CH, PG, and memlogstore queries. Add FIXME for test dataset which assigns manual and attempt_number independently. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * bench: add metrics benchmarking suite for ClickHouse and PostgreSQL Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * bench: update metrics bench for new API and add coverage for rate, granularity, filters Migrate bench suite to current metrics API (TimeRange, tenant via filters) and add bench cases for rate measures, multi-value granularities (2d/w/M), new dimensions (code, attempt_number), and new filters (code, manual, attempt_number, multi-filter). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: align metrics with 1-based attempt_number indexing PR #740 changed attempt_number from 0-based to 1-based. Update all metrics query logic, test data, seed scripts, and bench seeds to match. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Alexandre Bouchard <alexbouchardd@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.