[meshkit] fix: disable tracing when no endpoint configured, suppress repeated export-failure logs

[uzairhameed]

Fixes: #894

Summary

This PR makes two related fixes to tracing/tracing.go:

1. Treats a missing/empty endpoint as implicitly disabled tracing: Previously, InitTracer returned a hard error if Endpoint was empty, even when tracing was never intended to be used. Now, if no endpoint is configured (or tracing is explicitly disabled via a new Enabled *bool field), InitTracer returns nil, nil — a clean no-op, not an error.

2. Suppresses repeated identical export-failure logs : when an endpoint is configured but unreachable (e.g. collector down, wrong address, not yet started), the OTLP exporter retries indefinitely, and previously logged a failure on every retry (~every 10-15s). This adds an error handler (via otel.SetErrorHandler) that logs the first failure once, then suppresses further identical failures, without affecting the underlying retry behavior itself.Any previously registered OTel error handler is preserved and still invoked, so this doesn't silently override unrelated error handling that may already be in place.

Why

Meshery Server was logging repeated "connection refused" errors when no trace collector was configured, because it kept trying to reach a default.
OTLP address. Investigation showed this had two separate causes:

• The original reported scenario (nothing configured at all) was independently fixed in meshery's main.go/Makefile defaults, but the same protection was never added at the meshkit layer, so any other meshkit consumer remained exposed to it.
• A related, still-open case: a user (or a Helm deployment, once feat: expose OTEL_CONFIG in Helm chart values.yaml, disabled by default meshery#20898 lands) explicitly configuring an endpoint that's unreachable still produces the same noisy log loop today.

Known limitation

• The suppression does not currently reset on recovery, if the collector comes back online and later goes down again, the second failure streak won't log again, since otel.SetErrorHandler only exposes a failure hook, not a success hook. This still resolves the reported issue (no more infinite log spam) but is noted here as a possible future improvement (e.g. wrapping the exporter directly to detect successful exports).
• The suppression currently applies to all OTel errors process-wide, not specifically to export failures, it doesn't distinguish "collector unreachable" from other kinds of OTel errors that might occur. A more precise fix would inspect the error type or wrap the exporter directly instead of using the global handler, which felt like a larger change than this PR's scope.

Testing

• Added unit tests covering: explicit disable, missing endpoint (no-op, not an error), and explicit enable with no endpoint (still a no-op).

• Verified live, against a real collector:

  1. Confirmed make server with no OTEL_CONFIG already logs cleanly ("tracing disabled"), unaffected by this change.

meshery.server.mp4

2. Demonstrated the still-open problem this PR fixes: with an endpoint configured but unreachable, the exporter retries and logs a failure on every attempt, continuously (~every 10-15s), before this fix.

tracing.logs.mp4

3. With the fix applied: configured the same unreachable endpoint and confirmed the failure now logs only once, not on every retry. Then started a local Jaeger instance (OTLP on 4317) and confirmed traces from meshery-server appear correctly in Jaeger's UI. Then stopped Jaeger again and confirmed no renewed log spam occurs (consistent with the known limitation noted above, suppression persists rather than resetting on recovery).

tracing.fix.mp4

Related

Companion Helm chart change (exposes OTEL_CONFIG in values.yaml):
meshery/meshery#20898

Signed commits

• Yes, I signed my commits.

Summary by CodeRabbit

Summary by CodeRabbit

• New Features

  • Added an enabled option to explicitly turn tracing on or off via YAML/JSON configuration.

• Bug Fixes

  • Tracing now behaves as a safe no-op when tracing is disabled or when no endpoint is configured, instead of failing to start.
  • Prevented repeated tracing export failures from repeatedly flooding logs.

• Tests

  • Expanded tracer initialization tests to verify nil/no-op behavior and correct shutdown handling when tracing is disabled or omitted.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 11c55baf-aea1-43d9-b65f-5946cf4c06e2

📥 Commits

Reviewing files that changed from the base of the PR and between eb3e30e and d13464b.

📒 Files selected for processing (2)

• tracing/tracing.go
• tracing/tracing_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• tracing/tracing_test.go
• tracing/tracing.go

---

📝 Walkthrough

Walkthrough

Tracing configuration adds explicit disabling, treats an empty endpoint as a no-op, and suppresses repeated OpenTelemetry export failures. Tests verify nil-provider and error behavior across enabled and disabled configurations.

Changes

Tracing configuration

Layer / File(s)	Summary
Tracing controls and error suppression tracing/tracing.go	Adds the optional Config.Enabled flag, synchronized suppression of repeated OpenTelemetry export errors, and global error-handler chaining.
Initialization behavior and validation tracing/tracing.go, tracing/tracing_test.go	InitTracer returns nil, nil when disabled or when no endpoint is configured; tests cover provider nil-ness and error expectations across configurations.

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

Possibly related issues

• Scope tracing export-error deduplication to OTLP exporter failures #1071 — Refines the OpenTelemetry export-error deduplication behavior introduced in this change.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	Only meshkit tracing changes are present; #894 also requires Meshery env, Helm, and docs updates that are not shown here.	Add the Meshery-side config/env wiring, expose it in Helm values, and update docs so tracing can be disabled end-to-end.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title matches the main change: disabling tracing when no endpoint is configured and suppressing repeated export-failure logs.
Out of Scope Changes check	✅ Passed	The changes stay focused on tracing configuration and tests, with no obvious unrelated code paths introduced.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Actionable comments posted: 2

🤖 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 `@tracing/tracing_test.go`:
- Around line 20-24: Extend the tracing test table with an enabled configuration
using Enabled: &trueVal and a nonempty Endpoint, expecting no error and a
non-nil provider. Update the test execution logic to shut down the returned
provider for this case while preserving existing assertions for other
configurations.

In `@tracing/tracing.go`:
- Around line 40-50: The global suppressRepeatedExportErrors handler must only
deduplicate OTLP exporter failures and must not replace or suppress unrelated
OpenTelemetry errors. Update the SetErrorHandler integration around
suppressRepeatedExportErrors to retain and delegate to the previously configured
handler for non-export errors, while preserving first-error-only logging for the
exporter path.

🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6d3c729c-3b73-4657-b14d-92726441f35f

📥 Commits

Reviewing files that changed from the base of the PR and between 78679d1 and c0c37ad.

📒 Files selected for processing (2)

• tracing/tracing.go
• tracing/tracing_test.go

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@tracing/tracing_test.go`:
- Around line 77-99: Update the InitTracer test setup to capture the existing
global tracer provider, error handler, and text map propagator before calling
InitTracer, then register t.Cleanup to restore all three values after each test
case. Keep the existing provider assertions and shutdown behavior unchanged.

🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3defec3e-c008-4ca7-ab8f-8ab6b46f522f

📥 Commits

Reviewing files that changed from the base of the PR and between c0c37ad and eb3e30e.

📒 Files selected for processing (2)

• tracing/tracing.go
• tracing/tracing_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• tracing/tracing.go

[ritzorama]

@uzairhameed there is an earlier PR on this same topic. Please reconcile.

[uzairhameed]

@uzairhameed there is an earlier PR on this same topic. Please reconcile.

Yes, I reviewed the earlier PR and shared my feedback during the development meeting on July 14. You can refer to the meeting recording from 51:00–58:00.

Lee gave me the go-ahead to work on this and suggested that I coordinate with the original PR author. However, as you can see, they haven't been active on this issue since January, so I proceeded with the implementation.

[uzairhameed]

Closing this in favor of a re-scoped PR, per feedback to keep this fix
narrowly focused on the endpoint/enabled check: #1088 .