open-telemetry / open-telemetry/opentelemetry-python-contrib

opamp: do not enable heartbeats after the initial connection job fails

Open
#5,053 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
1.1k
Forks
1.1k
Avg merge
4d 15h
Merged PRs (30d)
16

Description

OpAMP agents whose initial connection exhausts retries start sending heartbeat-only messages without having established the initial full state. They also execute caller-provided completion callbacks after transport failures or shutdown interruption. Successful connections are unaffected.

At opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/agent.py:212:

The worker invokes if job.callback is not None: job.callback() after its retry loop regardless of whether send succeeded. On retry exhaustion, lines 196-198 break with message still None. start supplies _enable_scheduler as the initial full-state job callback at lines 116-122, so this failure path sets _schedule = True at line 103. This contradicts the explicit invariant at line 97, 'start scheduling only after connection with server has been established'. Subsequent scheduler iterations enqueue heartbeats even though the initial full-state message was never delivered. Root cause: completion of a failed job is treated as successful completion for its success callback.

Proposed fix

In agent.py, invoke the job success callback only after a successful send, preserving task_done for success, exhaustion and cancellation. Keep the fix confined to callback gating rather than changing retry or reconnect policy. Add deterministic worker tests to tests/opamp/test_agent.py covering exhaustion, interrupted retries and successful completion. Include a package-local fixed changelog fragment after the issue and implementation prerequisites are resolved.

How to see it

Unexecuted, deterministic code trace and regression plan. Construct an agent with max_retries=0 and a fake client whose send sets agent._stop and raises ConnectionError. Put a _Job with the full-state payload and callback=agent._enable_scheduler directly into agent._queue, then invoke agent._run_worker synchronously. The current code catches the exception, exhausts retries, invokes the callback, completes queue bookkeeping, and exits. Assert agent._schedule is False and agent._queue.unfinished_tasks is 0. The first assertion fails today because _schedule becomes True. Add a success control where send sets the stop event and returns a real empty ServerToAgent message; the callback must run once. Add a retries-exhausted case and an interrupted-backoff case, with no sleeps or real network calls.

If this looks right I can push fix/opamp-failed-connection-callback on anxkhn/opentelemetry-python-contrib instead of opening a pull request first.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/agent.py, reading the worker retry loop, _Job callback handling, and _enable_scheduler. Add deterministic tests in tests/opamp/test_agent.py for retry exhaustion, interrupted backoff, and successful completion; done means success callbacks run only after successful sends, queue bookkeeping remains complete, and failed initial connections leave scheduling disabled. Add the required package-local changelog fragment.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.