Skip to content

Conversation

@rahulpr22
Copy link
Contributor

@rahulpr22 rahulpr22 commented Oct 8, 2025

Summary by CodeRabbit

  • New Features

    • Added a DISABLE_NEW_RELIC configuration to turn off New Relic reporting.
    • Made New Relic distributed tracing configurable.
    • Improved error logging when New Relic setup runs.
  • Refactor

    • Minor cleanup in OpenTelemetry fallback (no behavior change).
  • Chores

    • Streamlined indirect dependencies and updated tooling-related packages.

@rahulpr22 rahulpr22 requested a review from ankurs October 8, 2025 06:18
@coderabbitai
Copy link

coderabbitai bot commented Oct 8, 2025

Walkthrough

Added a Config flag to disable New Relic, gated New Relic setup in startup (errors now logged), threaded a distributed-tracing flag into New Relic initialization, and adjusted indirect dependencies in go.mod.

Changes

Cohort / File(s) Summary
Config flag + core startup
config/config.go, core.go
Added DisableNewRelic bool (DISABLE_NEW_RELIC) to Config. Startup now guards SetupNewRelic with this flag and logs any SetupNewRelic error; minor formatting tweak in New Relic/OpenTelemetry fallback.
New Relic tracing init
initializers.go
SetupNewRelic now accepts/uses a tracing parameter via newrelic.ConfigDistributedTracerEnabled(tracing) to enable distributed tracing when requested.
Dependencies
go.mod
Removed several indirect dependencies (e.g., bugsnag, backoff, go-kit, otel/internal/retry, genproto); added indirect github.com/newrelic/csec-go-agent v0.3.0 and github.com/smarty/assertions v1.15.1; cleaned/reordered require block.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant App as App Startup
    participant Env as Env / Config
    participant NR as New Relic (SetupNewRelic)
    participant OTel as OpenTelemetry Init
    participant Log as Logger

    App->>Env: load config (DISABLE_NEW_RELIC, tracing)
    alt DISABLE_NEW_RELIC == false
        App->>NR: call SetupNewRelic(tracing)
        alt Setup succeeds
            Note right of NR: New Relic configured\n(distributed tracing honored if tracing==true)
        else Setup fails
            NR-->>Log: log error from SetupNewRelic
            App->>OTel: initialize OpenTelemetry fallback
        end
    else DISABLE_NEW_RELIC == true
        Note right of App: New Relic setup skipped
        App->>OTel: initialize OpenTelemetry (or other telemetry)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Custom otel support #32 — modifies OpenTelemetry/New Relic initialization paths and relates to the guarded SetupNewRelic/OTel fallback changes.

Suggested reviewers

  • svetha-cvl

Poem

I twitch my nose and check the flag,
If relic sleeps I skip the drag.
If tracing wakes, I bind the trail,
Else OTel keeps the ship full sail.
I prune the deps and hop—huzzah! 🐰✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly and concisely describes the primary change of making NewRelic setup respect configuration, directly reflecting the addition of the DisableNewRelic guard. It is specific enough for a reviewer to understand the intent without extraneous details. It does not include vague terms or unrelated content.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/nr-config

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0076015 and 956c433.

📒 Files selected for processing (1)
  • core.go (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
core.go (1)
initializers.go (2)
  • SetupNewRelic (46-66)
  • SetupNROpenTelemetry (293-309)
🔇 Additional comments (1)
core.go (1)

97-102: LGTM! Critical logic fix applied.

The inverted logic issue has been correctly resolved. NewRelic is now initialized only when DisableNewRelic is false, and error handling has been properly added.


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

Comment @coderabbitai help to get the list of available commands and usage tips.

@rahulpr22 rahulpr22 requested a review from svetha-cvl October 8, 2025 06:18
@rahulpr22 rahulpr22 changed the title Fix: Setup NewRelic based on NewRelicDistributedTracing config Fix: Setup NewRelic based on config Oct 8, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 67432d3 and 0076015.

📒 Files selected for processing (3)
  • config/config.go (1 hunks)
  • core.go (2 hunks)
  • initializers.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
core.go (1)
initializers.go (2)
  • SetupNewRelic (46-66)
  • SetupNROpenTelemetry (293-309)
🔇 Additional comments (3)
initializers.go (1)

57-57: LGTM!

The change correctly enables distributed tracing based on the tracing parameter rather than a hardcoded value, providing runtime configurability.

config/config.go (1)

33-34: LGTM!

The new configuration field is properly documented and follows the existing config pattern.

core.go (1)

142-150: LGTM!

Good practice to capture and log the error from SetupNROpenTelemetry for proper error visibility.

@rahulpr22 rahulpr22 requested a review from svetha-cvl October 8, 2025 06:43
@rahulpr22 rahulpr22 merged commit 4af0471 into main Oct 8, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants