feat(publisher): degrade PubSubPublisher to noop when GCP credentials are absent

[julietshen]

First PR for #343 — making Osprey runnable without GCP credentials. Tracked by #410, and now incorporates the async-worker changes originally split into #411.

What this changes

Both Pub/Sub publishers now degrade to a noop instead of failing when GCP credentials can't be resolved (the default docker compose up experience, or any adopter env without GCP), or when the DISABLE_GCP_PUBSUB env var is set. In noop mode no underlying client is built and publish()/stop() return immediately.

• Shared credential detection — osprey/worker/lib/gcp_credentials.py exposes gcp_credentials_available() (cached + lock-guarded, so google.auth.default() is probed at most once per process) and gcp_pubsub_disabled() (the DISABLE_GCP_PUBSUB env opt-out). Both publishers share this one implementation.
• Sync PubSubPublisher (osprey_worker/.../lib/publisher.py) — auto-detects credentials at construction, honors the env opt-out (short-circuiting before the credential probe), logs a one-time warning, and short-circuits publish()/stop().
• Async AsyncPubSubPublisher (osprey_async_worker/.../lib/publisher.py) — the same behavior for the experimental asyncio worker.

Observability

• Startup warning — one parsable log line per publisher when it enters noop mode, naming the inert project and topic.
• configuration.errors metric — emitted once at startup when credentials are missing (tagged with project, topic, and reason:gcp_credentials_missing). The deliberate DISABLE_GCP_PUBSUB opt-out stays silent, since it isn't a misconfiguration. There is no per-send metric: the state only changes at startup, and credentials revoked mid-run surface as publish.failure spikes rather than a disabled state.

Not in this PR

Broader GCP-eager modules (secret_manager.py, bigquery.py, access_audit_log.py) remain tracked under #343.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TEcfpqhza3dPepZUVWw36X

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

PubSubPublisher now checks GCP credentials at construction, caches the result, and switches to noop mode when credentials are unavailable. In noop mode it logs a warning, skips client creation, and short-circuits publish() and stop(), while tests, fixtures, and release notes were updated to cover both paths.

Changes

PubSubPublisher noop mode

Layer / File(s)	Summary
Credential detection and init wiring osprey_worker/src/osprey/worker/lib/publisher.py	Adds Google Auth imports, a module logger, noop-mode docstring updates, cached credential probing, and _enabled initialization with warning logging and conditional client creation.
Noop publish and stop behavior osprey_worker/src/osprey/worker/lib/publisher.py	Updates publish() to increment the noop metric before returning when disabled, and stop() to return immediately when disabled.
Tests, fixture, and changelog osprey_worker/src/osprey/worker/lib/tests/test_publisher.py, osprey_worker/src/osprey/worker/lib/data_exporters/test/conftest.py, CHANGELOG.md	Adds cache-reset setup, credential probe tests, enabled and disabled publisher tests, disabled publish()/stop() tests, a test fixture override for credential checks, and a changelog entry for the new behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

• Pub/Sub publishers should degrade to noop without GCP credentials (sync + async workers) #410: Describes the same PubSubPublisher noop behavior for missing GCP credentials, including cached credential checks, short-circuiting, metrics, and tests.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 7.14% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: PubSubPublisher now falls back to noop when GCP credentials are missing.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch julietshen/noop-publisher-fallback

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

osprey_worker/src/osprey/worker/lib/tests/test_publisher.py (1)

59-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert noop metric tags too.

The implementation sends publisher tags with the noop metric; checking only the name leaves the dashboard-slicing contract unprotected.

Proposed test tightening

-    metric_names = [call.args[0] for call in metrics_mock.increment.call_args_list]
-    assert metric_names == ['PubSubPublisher.publisher.noop']
+    metrics_mock.increment.assert_called_once_with(
+        'PubSubPublisher.publisher.noop',
+        tags=['project:proj', 'topic:projects/proj/topics/topic'],
+    )

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@osprey_worker/src/osprey/worker/lib/tests/test_publisher.py` around lines 59
- 67, The noop metric test only verifies the metric name in
test_publish_short_circuits_and_emits_noop_metric_when_disabled, but not the
publisher tags that the implementation emits. Update this test to also assert
the tags passed to metrics.increment when PubSubPublisher.publish
short-circuits, using the existing publisher.metrics mock and the
PubSubPublisher publisher path so the dashboard-slicing contract stays covered.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@osprey_worker/src/osprey/worker/lib/publisher.py`:
- Around line 135-146: The lazy GCP credential cache in _check_gcp_credentials
can still race when multiple PubSubPublisher instances are created concurrently.
Add a small module-level lock around the first google.auth.default() lookup so
only one thread can populate _gcp_credentials_available, and keep the cached
return path unchanged. Use the existing _gcp_credentials_available global and
_check_gcp_credentials function to locate the fix.

---

Nitpick comments:
In `@osprey_worker/src/osprey/worker/lib/tests/test_publisher.py`:
- Around line 59-67: The noop metric test only verifies the metric name in
test_publish_short_circuits_and_emits_noop_metric_when_disabled, but not the
publisher tags that the implementation emits. Update this test to also assert
the tags passed to metrics.increment when PubSubPublisher.publish
short-circuits, using the existing publisher.metrics mock and the
PubSubPublisher publisher path so the dashboard-slicing contract stays covered.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 82dfa609-db16-4dd3-b7f3-3dce5ac305d7

📥 Commits

Reviewing files that changed from the base of the PR and between 1075b30 and 05c0cdc.

📒 Files selected for processing (2)

• osprey_worker/src/osprey/worker/lib/publisher.py
• osprey_worker/src/osprey/worker/lib/tests/test_publisher.py

[chimosky]

I'm wondering if a better way to do this would be to check for "Google" in /sys/class/dmi/id/product_name, so we can avoid the cost of this network call.

Seeing as we actually aren't using the credentials, only checking for them.

[julietshen]

oooh good catch. you're right that we only need to check whether creds exist here, not actually use them. I looked into what that network call really costs:

google.auth.default() looks for credentials in three places, in order: an environment variable pointing at a key file, then a local gcloud login file, and it only calls to the network if both of those come up empty. So the network call only happens when there are no creds anywhere else, which is the exact case we're trying to catch.

it shouldn't slows down a normal startup that has creds, and I only run the check once per process and remember the answer, so it doesn't happen on every publish.

The reason I'd hold off on reading /sys/class/dmi/id/product_name: that tells you whether the machine is a Google Cloud box, which isn't quite the same as "do we have google credentials." Those two usually match, but not always: for example, someone running Osprey outside Google Cloud (say on AWS or their own servers) but still sending to Pub/Sub with a key file has credentials, even though the machine isn't a Google box.

In that case, google.auth.default() still finds the key and publishing works. But if we switched to reading the machine type, it would see "not a Google box," assume there are no credentials, and shut publishing off without any error. Since the whole point of #343 is to stop GCP from failing quietly, I'd rather not add another way for that to happen.

If that startup check does turn out to slow down docker compose up, I think the better fix is to just give it a short time limit so it can't get stuck, vs switch to the hardware check and lose the ability to find key-file creds. Happy to do that if you think it's worth it! What do you think?

[chimosky]

The reason I'd hold off on reading /sys/class/dmi/id/product_name: that tells you whether the machine is a Google Cloud box, which isn't quite the same as "do we have google credentials." Those two usually match, but not always: for example, someone running Osprey outside Google Cloud (say on AWS or their own servers) but still sending to Pub/Sub with a key file has credentials, even though the machine isn't a Google box.

Yeah, this makes sense.

In that case, google.auth.default() still finds the key and publishing works. But if we switched to reading the machine type, it would see "not a Google box," assume there are no credentials, and shut publishing off without any error. Since the whole point of #343 is to stop GCP from failing quietly, I'd rather not add another way for that to happen.

If that startup check does turn out to slow down docker compose up, I think the better fix is to just give it a short time limit so it can't get stuck, vs switch to the hardware check and lose the ability to find key-file creds. Happy to do that if you think it's worth it! What do you think?

The network cost might be something we need not worry about tbh, putting a limit sounds like a good idea,

[ThisIsMissEm]

The only thing I'm not sure on is the usage of a metric for this state. I think you'd probably want just a config flag fed back into the system. So like allow setup of the publisher to fail, and report it back as a configuration error rather than a metric (one parsable log line vs metric per send), since this is only changing at startup.

It'd also be an idea to add a DISABLE_GCP_PUBSUB boolean environment variable that completely opts out of this codepath.

[ThisIsMissEm]

Not sure on the usage of another metric here, and I think the dashboard could probably just surface this as a warning banner or something "Publishing to GCP PubSub is currently disabled due to invalid credentials"

[julietshen]

ooooh good idea! I'll work that in

[julietshen]

i'm actually going to add that separately, since it's a UI change and i want to keep this and #411 focused on the publisher changes

[ThisIsMissEm]

@julietshen this gets close to what I was suggesting; here's something you could feed back into claude to adjust the approach, which will lift it up one layer:

1. Reuse the existing noop publisher instead of building disable logic into PubSubPublisher

There's already a NullPublisher class (osprey_worker/.../lib/publisher.py:34) that
does nothing when you publish to it — it exists for exactly this "I don't want to send
anything" case.

Right now the PR teaches PubSubPublisher to disable itself: it probes for GCP
credentials and flips into a silent mode internally. That gives the transport class a
second job (deciding whether it should run) on top of its real job (sending messages).

The cleaner shape is to make that decision at the point where the publisher is created,
and hand back a NullPublisher when Pub/Sub is off. Since it's constructed in ~5 places
(sinks.py:266/307/311, data_exporters/validation_result_exporter.py:59,
ui_api/osprey/singletons.py:14), the tidy way is a small factory — e.g.
make_publisher(project, topic) — that returns NullPublisher() when disabled or when
credentials are absent, and PubSubPublisher(...) otherwise. Everything routes through
that one function.

Trade-off worth naming: the current self-disabling approach automatically covers any
future call site too, whereas the factory needs new sites to go through it. I think the
factory is still the better call (keeps the sending class simple, reuses code we already
have), but it's a judgment call, not a hard rule.

The credential probe could actually throw and be caught higher up and then the publisher class returned is actually NullPublisher because gcp isn't configured correctly. We're essentially lifting the logic a layer up the stack, rather than focusing on the PubSubPublisher itself.

2. Read the flag through the existing config system, not os.environ directly

The codebase has a config helper for reading settings like this:
config.get_bool('DISABLE_GCP_PUBSUB', False). It already understands "true"/"false"
values. The PR currently reads the raw environment variable by hand
(os.environ.get('DISABLE_GCP_PUBSUB', '').lower() == 'true' in gcp_credentials.py),
which re-implements what the helper does for free and sidesteps the project's normal way
of handling config. Switching to get_bool (from osprey_worker/src/osprey/worker/lib/config/__init__.py) keeps it consistent with the rest of the app. There's also environ_bool in osprey_worker/src/osprey/worker/lib/osprey_logging/__init__.py oddly.

You could potentially even derive this flag from OSPREY_GCP_PROJECT_ID being set or not, though there's also PUBSUB_OSPREY_PROJECT_ID and PUBSUB_DATA_PROJECT_ID so not exactly a consistently named thing.

3. Minor: flag naming

DISABLE_GCP_PUBSUB reads fine, but note every other flag in the codebase is phrased
positively (DEBUG, TESTING, RUN_PROFILER) — there's no DISABLE_* precedent. A
positive form like PUBSUB_ENABLED (defaulting to true) avoids the "disable = false =
double negative" confusion. Not a blocker either way. There is a OSPREY_DISABLE_VALIDATION_EXPORTER but that seems to be the only case of a "disable", I think, not 100% sure.

Also worth keeping in mind: the broader goal (#343) is running Osprey without any GCP,
not just Pub/Sub. A Pub/Sub-specific name is fine for this PR, just flagging so we don't
lock ourselves out of a broader DISABLE_GCP-style switch later, though I think it may be independently controlled via two PUBSUB_ENABLED and BIGTABLE_ENABLED or something, though both depend on GCP credentials.

[ThisIsMissEm]

oh, also, there's only ever GCP PUBSUB, so you could drop the GCP_ in the env var name.

[chimosky]

This should be in osprey_worker/src/ospery/worker/lib/utils.

[julietshen]

during the refactor based on feedback from the working group, this is resolved! the shared credential-probe module is gone entirely now. Instead of probing up front, make_publisher just constructs the PubSubPublisher and catches the DefaultCredentialsError, so there's no gcp_credentials module left to relocate.

[julietshen]

Thank you @chimosky and @ThisIsMissEm for the reviews and suggestions! I'm addressing them in my next commit.

FYI, I'm going to focus on sync since AsyncPubSubPublisher has zero production callers right now. I'll follow up on it later

[ThisIsMissEm]

To retain what was previously here:

Suggested change

	return PubSubPublisher(project_id, topic_id)
	return PubSubPublisher(
	project_id,
	topic_id,
	raise_on_error=raise_on_error,
	max_bytes=max_bytes,
	max_messages=max_messages,
	max_latency=max_latency,
	)

Though these perhaps have default values?

[julietshen]

ty! They do have defaults, but no reason to drop the passthrough. Done in f0b8f6e

[ThisIsMissEm]

Suggested change

	logger.warning('GCP credentials not detected, publishing disabled (project=%s, topic=%s)', project_id, topic_id)
	logger.warning('GCP credentials errored, publishing disabled (project=%s, topic=%s)', project_id, topic_id)

[ThisIsMissEm]

Current behavior would be defaulting to True, I think, if I understand/remember this method correctly:

Suggested change

	if not CONFIG.instance().get_bool('OSPREY_PUBSUB_ENABLED', False):
	if not CONFIG.instance().get_bool('OSPREY_PUBSUB_ENABLED', True):

[julietshen]

agreed: since the construction-catch already returns a NullPublisher when credentials can't be resolved, a no-GCP setup still runs either way. defaulting to True avoids silently disabling publishing for deployments that already have creds. Done in f0b8f6e.