databuddy-analytics / databuddy-analytics/Databuddy

links health check no longer exercises Kafka delivery

Open
#719 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
1.2k
Forks
216
Avg merge
14h 53m
Merged PRs (30d)
154

Description

Raised by cubic on the release PR (#714) at apps/links/src/lib/producer.ts:113. Accepted rather than fixed so the release could ship; filing so it is not lost.

What happens

f16085061 ("always use TLS to redpanda and delete the ssl flag") hardcoded ssl: true on the links producer:

// apps/links/src/lib/producer.ts:113
ssl: true,

.github/workflows/health-check.yml still stands up a plaintext Redpanda for the links smoke run:

--advertise-kafka-addr localhost:9092
-e REDPANDA_BROKER=localhost:9092

A TLS client against a plaintext listener fails to connect. await candidate.connect() throws, and the catch swallows it:

} else {
    setAttributes({ kafka_health_connect_failed: true });
}
...
producer = null;

The health path sets an attribute, nulls the producer, and returns false. Nothing propagates, so Links Health Check goes green while the Kafka path is never exercised. The check has been passing on every PR in exactly this state, including the release that just shipped.

This is a CI coverage gap, not a production bug. Production talks TLS to Redpanda and is fine.

Why it matters

The smoke check exists to catch link-visit delivery regressions. Right now it would pass with the producer entirely broken. Any regression in that path ships unnoticed.

Options

  1. Give the CI Redpanda a TLS listener so the smoke run matches production. Highest fidelity, most setup.
  2. Make the broker protocol configurable again for test environments only. Reintroduces the flag f16085061 deliberately removed, so probably not.
  3. Fail the health check when kafka_health_connect_failed is set, so the gap is at least loud instead of silent. Cheapest, and strictly better than today even on its own.

Option 3 is worth doing regardless of which of the others is chosen, since it converts a silent pass into a visible failure.

Reproduce

Run the links health check and inspect the emitted attributes: kafka_health_connect_failed: true is present on a passing run.

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 with apps/links/src/lib/producer.ts:113 and .github/workflows/health-check.yml, then run the links health check and inspect its emitted attributes. Decide whether the CI broker should use TLS or the check should fail on kafka_health_connect_failed. Done means the smoke run exercises Kafka delivery and cannot pass while the producer connection fails.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions, kafka, typescript
Domain
backend, ci-cd, testing-qa
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.