Repository navigation
feat: batch delivery off the request path and release automatically - #7
Conversation
szymeo
left a comment
There was a problem hiding this comment.
Lead review: logdash moved from services to domains, so the smoke test must use the domain model instead of a project.
LOGDASH_API_KEYbecomes the ingest key of one shared domain, "SDK smoke tests" (its Settings page, "Ingest key": one write-only key for every app on the domain). The owner adds it, replacing this repo's old per-service key.- Send with the SDK's
appoption set tophp-sdk, so each SDK shows up as its own app on that domain. LOGDASH_PERSONAL_API_KEYis a read-only key (Logs read, Metrics read) restricted to that domain.GET https://api.logdash.io/personal-api-keys/whoamireturnsaccess: { kind: "clusters", ids: [domainId] }. Fail with a clear message unless the kind isclustersand there is an id.- Read logs from
GET /clusters/{domainId}/logs?apps=php-sdk(narrowed to this run's seed) and metrics fromGET /clusters/{domainId}/metrics(each entry hasapp,name,value: match onappandname), both with the same bearer. Drop/projects/{id}/.... - Never print an API key, not even one CI masks.
Check paths, query params and response shapes against logdash-io/logdash.io: apps/backend/src/log/core/log-core.controller.ts (readClusterLogs, ReadClusterLogsQuery in log/core/dto/cluster-logs.query.ts) and apps/backend/src/metric/core/metric-core.controller.ts (readClusterMetrics). The check program needs the app option, so it runs against the version this PR releases; before that, a manual run on this branch is not expected to pass.
… release automatically…
szymeo
left a comment
There was a problem hiding this comment.
Lead review: three blocking code findings and several minor ones. Payloads match the backend DTOs and the release computes 1.1.0, but each blocker below makes this release lose data that 1.0.6 delivered.
BLOCKING
src/Sync/HttpLogSync.php~13-15, 30: cutting messages at 16 KB does not keep 100 logs under the API's 2 MB body limit (backendmain.ts~20), because JSON escapes quotes and slashes. Logging 100 arrays sent a 2,129,210-byte batch; the API answers 413, which is not retried, so all 100 logs are lost. InRequestQueue::log()add upstrlen(json_encode($log, FLAGS))and flush before about 1.5 MB, and cut messages to the 4096 characters the API keeps.src/Queue/RequestQueue.php~122-126: age is counted from the oldest buffered item, so a single log in a long-running worker waits for the next log or for exit and is lost on SIGKILL or OOM ("job 1 failed" had not reached the API after 3 s idle). Use #4's$lastFlushAt, set in the constructor and in flush(), and flush whenhrtime(true) - $lastFlushAt >= 1 s.src/Queue/RequestQueue.php~39-50: the httpClient is no longer injectable, as it was in #4. Add?ClientInterface $httpClient = nullto RequestQueue and anhttpClientoption toLogdash::create, and sendproject-api-keyandContent-Typeper request instead of as client defaults.
MINOR
RequestQueue.php~103-108, 168: timeouts are retried, so one log against an API that never answers blocks for 10.2 s (1.0.6: 5 s), and a retry can duplicate a batch the API already stored. Retry only onServerException.RequestQueue.php~111-120: afterpcntl_fork, the child's shutdown flush sends the parent's buffer again (a buffered mutate applies twice). Storegetmypid()and drop the buffer when it changes.check-deployed-package/check.php~52-55: set and mutate merge into oneset, so the smoke test never sends achange. Call flush() between them.- Release re-runnable:
gh release view "v$V" >/dev/null 2>&1 || gh release create ..., then the Packagist ping, so a rerun after a failed step finishes. pr-title.yml:permissions: {}abovejobs:.scripts/version.ts~4: the comment still names tony's Info.plist; say Packagist reads the tag.
Add a test for each blocking fix. LOGDASH_PERSONAL_API_KEY is coming from the owner.
… release automatically…
Summary
feat: batch delivery off the request path and release automatically
Publish
malini/01J2242A9F1CC2C42A9A073AAAagainstmain..github/workflows/ci.yml.github/workflows/pr-title.ymlREADME.mdcheck-deployed-package/check.phpscripts/version.tssrc/Logdash.phpsrc/Queue/RequestQueue.phpsrc/Sync/HttpLogSync.phptests/LogdashTest.phpValidation