Skip to content

[P2][Review] Delete redundant SDK telemetry transport if unused; otherwise stop its thread and blocking flush #105

Description

@karlwaldman

Review-only / approval gate

No implementation, PR merge, package publication or production mutation until the owner approves the full sprint plan.

Evidence

Reviewed main 31b8d80686495c5d6dcac0846d6e5a2c8d4bce8b.

oilpriceapi/telemetry.py track_request calls synchronous _flush on the caller every 10 events; _flush performs httpx.post(timeout=5) despite its non-blocking comment. AsyncOilPriceAPI invokes it on the event loop. close() only flushes and never sets enabled=False/stops the daemon; configure_telemetry replaces global instances without stopping prior threads. Disabled-by-default status limits affected users; enabled consumers are not yet inventoried.

Preferred smallest change

First establish whether the opt-in external collector has a maintained consumer and useful data unavailable from existing request attribution. Prefer deleting that collector/thread with a documented compatibility/deprecation decision if not. If a real consumer requires it, minimally bound nonblocking delivery and stop lifecycle correctly; no new telemetry platform. Do not silently remove an advertised option.

Acceptance / negative-path tests

Decision evidence plus tests: disabled mode no thread/network; enabled ten calls cannot synchronously block API/event loop; close is idempotent and stops background activity; replacing global config leaves no prior thread; telemetry failure cannot alter request results.

Risk and PR gate

Low/medium risk unless public option removal requires a breaking release.
One focused issue PR; independent adversarial review must challenge deletion safety, public compatibility, failure recovery and whether a smaller deletion suffices. Include red/green test evidence and packed/installable artifact proof for any published SDK/MCP change. Merge only in the approved priority order after reviewer objections are resolved. Do not publish a breaking change as a patch. If already fixed at execution time, record evidence and skip without a cosmetic PR.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions