Skip to content

Production connector readiness, finance query filters/pagination, and observability - #15

Merged
charles2ke merged 3 commits into
mainfrom
copilot/graph-connector-readiness-filter-pagination-observ
Sep 5, 2026
Merged

Production connector readiness, finance query filters/pagination, and observability#15
charles2ke merged 3 commits into
mainfrom
copilot/graph-connector-readiness-filter-pagination-observ

Conversation

Copilot AI commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Makes the finance cluster integration production-ready: real HTTP connectors, query filtering/pagination, and observability.

Production connector readiness

  • New src/connectors/httpClient.js: bearer auth, per-request timeout via AbortController, bounded retries with exponential backoff (429/5xx/network only, never on 4xx), typed UpstreamHttpError, and a health probe.
  • HTTP connectors for OpenTrading, Portfolio-Watcher, and tax-break that mirror the existing mock contracts exactly, so the GraphQL schema and service are unchanged.
  • src/connectors/index.js selects the adapter by endpoint: mock:// keeps the mock adapter (service still runs from a clean checkout), any other endpoint uses the HTTP client.
  • New config: FINANCE_HTTP_TIMEOUT_MS, FINANCE_HTTP_MAX_RETRIES, FINANCE_DEFAULT_PAGE_SIZE, FINANCE_MAX_PAGE_SIZE, LOG_LEVEL.

Filtering and pagination

  • portfolioOverview(accountId, from, to, limit, offset) — date bounds apply to performance snapshots; positions are paginated.
  • tradeHistory(accountId, symbol, side, status, from, to, limit, offset) — trades/orders filtered then paginated; returned tax events match the current page of trades.
  • taxEstimate(taxYear, accountId, symbol, from, to, limit, offset) — totals always computed over every matching event; limit/offset only page the returned events.
  • All finance payloads return pageInfo { totalCount limit offset hasNextPage hasPreviousPage }; requested limits are clamped to FINANCE_MAX_PAGE_SIZE.

Observability

  • Structured JSON-lines logger with credential redaction.
  • In-process metrics registry (counters + latency summaries, Prometheus text output) covering GraphQL operations, per-source connector calls, upstream retry failures, and cache hit/miss/coalesced.
  • Apollo plugin emitting one log line and latency/outcome metrics per GraphQL operation.
  • Endpoints: GET /health (liveness), GET /ready (per-upstream status, 503 when degraded), GET /metrics.

Testing

  • npm test: 33 tests passing (new suites for connectors and observability, plus filter/pagination coverage at the domain and GraphQL levels).
  • CodeQL: 0 alerts.

Note: the repository has no Playwright suite, so no Playwright screenshots are attached.

Co-authored-by: charles2ke <6725706+charles2ke@users.noreply.github.com>
@charles2ke
charles2ke marked this pull request as ready for review September 5, 2026 01:54
@charles2ke
charles2ke requested a lite review from Copilot September 5, 2026 01:55

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

There are a few correctness and operational issues (notably numeric env parsing producing NaN, and pagination still triggering full upstream tax-event mapping) that should be addressed before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR makes the finance “cluster” integration production-ready by adding real HTTP connectors, adding filter/pagination semantics to finance queries (including pageInfo), and introducing lightweight structured logging + in-process metrics with health/readiness endpoints.

Changes:

  • Added production HTTP connector stack (shared HTTP client with auth/timeout/retry + connector selection based on mock:// vs real endpoints).
  • Extended finance domain/service + GraphQL schema to support filtering and offset pagination with pageInfo.
  • Introduced observability primitives (structured JSON logger, metrics registry, Apollo plugin) and exposed /health, /ready, and /metrics endpoints, plus tests/docs.
File summaries
File Description
test/observability.test.js Adds unit coverage for logger redaction/levels, metrics registry, and the Apollo observability plugin behavior.
test/graphql.test.js Adds GraphQL-level coverage for finance filtering + pagination + pageInfo.
test/finance.test.js Adds domain/service-level tests for filtering, pagination, and aggregated health behavior.
test/connectors.test.js Adds tests for HTTP client behaviors (auth, retries, timeouts, health) and connector selection logic.
src/services/financeService.js Integrates connector selection + caching metrics, adds filtering/pagination, adds finance readiness aggregation.
src/server.js Installs observability plugin into Apollo server creation.
src/schema.js Adds PageInfo type and extends finance query arguments + return shapes with pagination metadata.
src/observability/metrics.js Implements an in-process metrics registry with Prometheus text rendering.
src/observability/logger.js Implements a structured JSON-lines logger with basic credential redaction.
src/observability/apolloPlugin.js Adds Apollo plugin to emit per-operation logs and metrics.
src/index.js Wires logger/metrics into the server context and exposes /ready + /metrics endpoints.
src/domain/finance.js Adds filter helpers, inclusive date-range filtering, and offset/limit pagination helper.
src/connectors/tax-break/mockConnector.js Adds a health probe to the mock tax-break connector.
src/connectors/tax-break/httpConnector.js Adds production HTTP tax-break connector.
src/connectors/portfolio-watcher/mockConnector.js Adds a health probe to the mock Portfolio-Watcher connector.
src/connectors/portfolio-watcher/httpConnector.js Adds production HTTP Portfolio-Watcher connector.
src/connectors/opentrading/mockConnector.js Adds a health probe to the mock OpenTrading connector.
src/connectors/opentrading/httpConnector.js Adds production HTTP OpenTrading connector.
src/connectors/index.js Implements connector selection and shared wiring (logger/metrics/fetch) across upstreams.
src/connectors/httpClient.js Adds shared HTTP client with bearer auth, timeout, bounded retries, metrics, and readiness probe.
src/config/finance.js Adds env-driven config for HTTP client settings and pagination limits.
README.md Documents new endpoints, configuration, filtering/pagination, and observability.
Review details

Suppressed comments (1)

src/config/finance.js:27

  • cacheTtlMs/defaultPageSize/maxPageSize are parsed with Number(...), which can produce NaN and later surface as invalid GraphQL Ints (e.g., pageInfo.limit) or unexpected caching behavior. These should be validated/coerced to safe integers, with sensible minimums.
    cacheTtlMs: Number(env.FINANCE_CACHE_TTL_MS ?? 1000),
    defaultPageSize: Number(env.FINANCE_DEFAULT_PAGE_SIZE ?? 25),
    maxPageSize: Number(env.FINANCE_MAX_PAGE_SIZE ?? 100),
  • Files reviewed: 22/22 changed files
  • Comments generated: 6
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/config/finance.js Outdated
Comment thread src/domain/finance.js
Comment thread src/server.js Outdated
Comment thread src/services/financeService.js Outdated
Comment thread src/connectors/httpClient.js Outdated
Comment thread src/services/financeService.js Outdated
Co-authored-by: charles2ke <6725706+charles2ke@users.noreply.github.com>
Co-authored-by: charles2ke <6725706+charles2ke@users.noreply.github.com>
@charles2ke
charles2ke merged commit a46ff95 into main Sep 5, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants