feat(http-send): add OpenObserve provider support for remote logging

[precious-ndamati-stream4Tech]

The library previously only supported sending logs to legacy ExceptionHandler-style endpoints. This change introduces pluggable provider support, enabling integration with modern observability platforms like OpenObserve while maintaining backward compatibility. Extracted common field extraction into a shared utility and implemented provider-specific builders for both ExceptionHandler (legacy) and OpenObserve formats. OpenObserve support includes configurable JSON wire formats (ndjson for _multi endpoint, json for _json endpoint), HTTP Basic authentication, and static metadata tags (app, environment, appVersion) stamped onto every record. Updated documentation with configuration examples and detailed parameter descriptions.

[durlabhjain]

@copilot review this thoroughly as transport for Pino logging. As there can be a lot of logging, every optimization we can squeeze in, scalability, reliability, flexibility, matters.

[Copilot]

@copilot review this thoroughly as transport for Pino logging. As there can be a lot of logging, every optimization we can squeeze in, scalability, reliability, flexibility, matters.

Addressed in f2d9162. The transport now precomputes OpenObserve auth/metadata once, supports configurable batching via batchSize to reduce per-request overhead under heavy logging, validates bodyType, and surfaces non-2xx HTTP responses instead of silently ignoring them. I also added focused provider tests for batching and payload formatting.

[Copilot]

🟡 Not ready to approve

There is a confirmed batching bug in flushBatch() that can pass an array to buildRequest() for non-batching providers, producing malformed requests.

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

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Introduces a pluggable provider architecture for the pino-http-send transport so remote logging can target both the legacy ExceptionHandler-style endpoint and modern OpenObserve ingestion endpoints, while extracting shared log-field parsing into a common utility.

Changes:

• Refactors the HTTP transport to select a provider module (exceptionHandler / openobserve) and adds optional batching.
• Adds shared field extraction/utilities plus provider-specific request builders for ExceptionHandler (form-urlencoded) and OpenObserve (JSON/NDJSON + Basic auth).
• Updates documentation and adds a focused provider unit test for OpenObserve formatting/auth and NDJSON/JSON payload modes.

File summaries

File	Description
lib/pino-http-send.mjs	Adds provider selection, batching, and improved error surfacing for non-2xx responses.
lib/pino-http-send-providers/shared.mjs	Centralizes common log-field extraction and shared helpers/constants.
lib/pino-http-send-providers/exception-handler.mjs	Implements the legacy ExceptionHandler form-urlencoded request builder as a provider.
lib/pino-http-send-providers/openobserve.mjs	Implements OpenObserve JSON/NDJSON request building with Basic auth and metadata stamping.
tests/pino-http-send.test.mjs	Adds test coverage for OpenObserve provider option validation and body construction.
README.md	Documents provider configuration and adds an OpenObserve example.
docs/LOGGER_MIGRATION.md	Updates reliability claims to reflect new HTTP failure surfacing and batching support.
package.json	Bumps library version to 3.3.8.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 4
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Not ready to approve

The OpenObserve provider has a validation hole for falsy bodyType values and emits a type-unstable stack_trace, and the HTTP/2 path buffers response bodies on success unnecessarily.

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

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (3)

lib/pino-http-send-providers/openobserve.mjs:17

• validateOptions skips validation when bodyType is an empty string (or other falsy non-undefined value), because the check uses if (options.bodyType && …). That allows invalid values through and can lead to unexpected request formatting (e.g. bodyType: "" produces single-record JSON instead of failing fast).

const validateOptions = (options) => {
  if (!options.username || !options.password) {
    throw new Error(`[pino-http-send] The "${NAME}" provider requires "username" and "password" options`);
  }
  if (options.bodyType && !SUPPORTED_BODY_TYPES.has(options.bodyType)) {
    throw new Error(`[pino-http-send] The "${NAME}" provider only supports bodyType values: ${Array.from(SUPPORTED_BODY_TYPES).join(", ")}`);
  }

lib/pino-http-send-providers/openobserve.mjs:56

• stack_trace can be either an object (when err is present) or a string (when it isn't). This type instability can cause ingestion/schema issues downstream; it's safer to always emit a consistent type (typically a stringified JSON payload).

    app_version: context.appVersion,
    message: message || msg || (err && err.message) || "",
    stack_trace: (err && { stack: err.stack, query: extra.query || "" }) || JSON.stringify(extra),
    machine_name: hostname || MACHINE_NAME,
    date_time: time,

lib/pino-http-send.mjs:68

• In the HTTP/2 path, the response body is buffered for all requests (responseBody += chunk) even when the status is 2xx. If the server returns a large response body on success, this can create unnecessary memory pressure; buffering only when the status is non-2xx avoids that overhead while still surfacing error details.

          statusCode = Number(responseHeaders[http2.constants.HTTP2_HEADER_STATUS] || 0);
        });
        stream.on("data", (chunk) => {
          responseBody += chunk;
        });

• Files reviewed: 8/8 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Changes recommended

The current batching implementation can silently drop log records for providers without buildBatchRequest (data loss risk).

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

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

README.md:507

• The README documents the new openobserve provider options but omits batchSize, which is now supported by the transport (lib/pino-http-send.mjs) and is called out in LOGGER_MIGRATION. Adding it here makes the remote logging configuration section complete and consistent with the implementation.

- `provider` (string): Which backend to format/send logs for — `"exceptionHandler"` (default, legacy `ExceptionHandler.ashx`-style form post) or `"openobserve"` ([OpenObserve](https://openobserve.ai/) JSON ingest)
- `url` (string): Full ingest URL, including org/stream/endpoint suffix for OpenObserve (e.g. `.../api/<org-token>/<stream>/_multi`)
- `username`, `password` (string): Basic auth credentials — **required** when `provider` is `"openobserve"`
- `bodyType` (string, `"openobserve"` only): `"ndjson"` (default) sends newline-delimited JSON (one JSON object per line) for OpenObserve's `_multi` endpoint; `"json"` wraps records in a JSON array for the `_json` endpoint
- `app`, `environment`, `appVersion` (string, `"openobserve"` only): static tags stamped onto every record (e.g. `app: "playbook-backend"`, `environment: "prod"`)

• Files reviewed: 8/8 changed files
• Comments generated: 1
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[durlabhjain]

@copilot fix the observations. We should never drop the logs incorrectly. IF a provider doesn't support batching, we should make the requests in a loop or similar?

Also, we should optimize for connection keep-alive which may be critical for http-send

[Copilot]

🟡 Changes recommended

The OpenObserve payload currently emits an inconsistent level field type/value and the new flush test risks leaking a patched fetch without a try/finally restore.

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

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (2)

lib/pino-http-send-providers/openobserve.mjs:49

• level is sometimes a numeric string (when level is present) and sometimes the literal text "error" (when missing). This makes the field type/shape inconsistent across records and can break indexing/querying in OpenObserve; keep it a consistent numeric/string representation and default to an error-level number if missing.

  return {
    level: level !== undefined ? String(level) : "error",
    app: context.app,

tests/pino-http-send-flush.test.mjs:56

• globalThis.fetch is restored only on the happy path; if writeLinesAndClose rejects or an unexpected error is thrown before line 56, the patched fetch can leak into later tests/process state. Wrap the patching in a try/finally so restoration is guaranteed.

  const calls = [];
  const originalFetch = globalThis.fetch;
  globalThis.fetch = async (url, init) => {
    calls.push(init.body);
    return { ok: true, text: async () => '' };
  };

• Files reviewed: 9/9 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Changes recommended

The OpenObserve provider currently JSON-stringifies structured fields (e.g., parameters), which contradicts the new tests and undermines structured ingestion.

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

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (2)

lib/pino-http-send-providers/openobserve.mjs:62

• parameters (and query_string) are being JSON-stringified, but the provider is otherwise emitting structured JSON; this also contradicts the new test expectation that parameters is an object. Keeping these as objects preserves structure for OpenObserve indexing and makes the tests pass without extra parsing.

    query_string: JSON.stringify(req.query || {}),
    parameters: JSON.stringify(paramsObj),

tests/pino-http-send-flush.test.mjs:58

• globalThis.fetch is restored in a finally, but the createWriteStream(...) call happens before the try block. If createWriteStream throws (e.g., future validation changes), the stubbed fetch will leak into other tests/process code.

  const originalFetch = globalThis.fetch;
  globalThis.fetch = async (url, init) => {
    calls.push(init.body);
    return { ok: true, text: async () => '' };
  };

  const stream = createWriteStream({
    url: 'http://example.com/ExceptionHandler.ashx',
    provider: 'exceptionHandler',
    batchSize: 3,
  });

• Files reviewed: 9/9 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.